Bug in libiberty/cplus-dem.c demangle_qualified()

[Carlo Wood <carlo at runaway dot xs4all dot nl>]

Hi all,

I found a bug in demangle_qualified() in libiberty/cplus-dem.c.

This particular bug shows up in egcs-1.1.1, gdb-4.17, gdb-4.17.0.8
and `nm' (binutils-2.9.1.0.15, which is the only version I checked).
The libiberty/cplus-dem.c of egcs-1.1b does not lead to a problem
for me, but I am not sure if it doesn't contain other bugs or
problems, or that it is a coincidence(?). It should be noted however
that libiberty/cplus-dem.c was heavily changed between egcs-1.1b
and egcs-1.1.1.

This report will use the libiberty/cplus-dem.c version of egcs-1.1.1
as example, I hope that it will not be a big problem to fix this bug
in the last version (which I am sure still has that bug).

In words: While demangling a Qualified name with a template as most
inner class, demangle_qualified() uses an uninitialized variable
when demangling the constructor or destructor of that inner class.

Example:

class A {
public:
  template<class T>
  class B {
  public:
    B(void);
  };
};

will produce for the constructor of B the mangled name:

__Q21At1B1Zi

When demangling this, we enter demangle_qualified() with
"Q21At1B1Zi" and follow the following path:

Breakpoint 1, demangle_qualified (work=0xbffff48c, mangled=0xbffff4cc,
    result=0xbffff4b8, isfuncname=1, append=0) at cplus-dem.c:1920
1920      int success = 1;
(gdb) p *mangled
$1 = 0x81b2926 "Q21At1B1Zi"
(gdb) n
1925      string_init (&temp);
(gdb)
1926      switch ((*mangled)[1])
(gdb)
1957          num[0] = (*mangled)[1];
(gdb)
1958          num[1] = '\0';
(gdb)
1959          qualifiers = atoi (num);
(gdb)
1964          if ((*mangled)[2] == '_')
(gdb)
1968          (*mangled) += 2;
(gdb)
1969          break;
(gdb)
1976      if (!success)
(gdb)
1982      while (qualifiers-- > 0)
(gdb)
1984          if (*mangled[0] == '_')
(gdb)
1986          if (*mangled[0] == 't')
(gdb)
1991          else if (*mangled[0] == 'X')
(gdb)
1998              namelength = consume_count (mangled);
(gdb)
1999              if (strlen (*mangled) < namelength)
(gdb)
2005              string_appendn (&temp, *mangled, namelength);
(gdb)
2006              *mangled += namelength;
(gdb)
2008          if (qualifiers > 0)
(gdb)
2010              string_append (&temp, (work -> options & DMGL_JAVA) ? "." : "::");
(gdb)
2012        }
(gdb)
1982      while (qualifiers-- > 0)
(gdb)
1984          if (*mangled[0] == '_')
(gdb)
1986          if (*mangled[0] == 't')
(gdb)
1988              success = demangle_template(work, mangled, &temp, 0, 1);
(gdb)
1989              if (!success) break;
(gdb)
1990            }
(gdb)
2008          if (qualifiers > 0)
(gdb)
2012        }
(gdb)
1982      while (qualifiers-- > 0)
(gdb)
2019      if (isfuncname && (work->constructor & 1 || work->destructor & 1))
(gdb)
2021          string_append (&temp, (work -> options & DMGL_JAVA) ? "." : "::");
(gdb)
2022          if (work -> destructor & 1)
(gdb)
2026          string_appendn (&temp, (*mangled) - namelength, namelength);
(gdb) (gdb) p temp
$2 = {b = 0x81b4860 "A::B<int>::\bìü\032\b\034ý\032\bLý\032\bP",
  p = 0x81b486b "\bìü\032\b\034ý\032\bLý\032\bP",
  e = 0x81b4880 " H\e\b\001\001"}
(gdb) p namelength
$3 = 1
(gdb) p (*mangled)-1
$4 = 0x81b292f "i"
(gdb) p (*mangled)-12
$8 = 0x81b2924 "__Q21At1B1Zi"
(gdb)                         

The problem is obviously that in line 2026 is assumed that *mangled just
had appended the class name ("B"), which is not true when the class is
a template.  For an overview, here is the loop that runs over `qualifiers'
which is assumed to initialize (*mangled) and `namelength':

  while (qualifiers-- > 0)
    {
      if (*mangled[0] == '_')
        *mangled = *mangled + 1;
      if (*mangled[0] == 't')
        {
          success = demangle_template(work, mangled, &temp, 0, 1);	// Line 1988
          if (!success) break;
        }
      else if (*mangled[0] == 'X')
        {
          success = do_type (work, mangled, &temp);
          if (!success) break;
        }
      else
        {
          namelength = consume_count (mangled);
          if (strlen (*mangled) < namelength)
            {
              /* Simple sanity check failed */
              success = 0;
              break;
            }
          string_appendn (&temp, *mangled, namelength);
          *mangled += namelength;
        }
      if (qualifiers > 0)
        {
          string_append (&temp, (work -> options & DMGL_JAVA) ? "." : "::");
        }
    }

We only pass line 1988, and never initialize `namelength' at all.
More over, (*mangled) doesn't contain the name of the constructor/destructor
we need later.

Although the given simple example is not enough to let gdb core dump yet
(because namelenght is just '1' for some stack history reason), we can still
use the following example to see this effect in practise:

======start of example=====================================================
class A {
public:
  template<class T>
  class B {
  public:
    B(void);
  };
};

template<class T>
A::B<T>::B(void) {
}

int main(void) {
  A::B<int> b;
}
======end of example========================================================

When I compile this and then use gdb to look what it gives as demangled
name for B's constructor, we get:

~/c++/libr/src/kernel/tests>g++ -g bug.cc
~/c++/libr/src/kernel/tests>gdb a.out
GNU gdb 4.17.0.8 with Linux support
Copyright 1998 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-redhat-linux"...
(gdb) b main
Breakpoint 1 at 0x8048556: file bug.cc, line 15.
(gdb) r
Starting program: /home/carlo/c++/libr/src/kernel/tests/a.out

Breakpoint 1, main () at bug.cc:15
15        A::B<int> b;
(gdb) s
A::B<int>::i (this=0xbffff747) at bug.cc:11
11      A::B<T>::B(void) {
(gdb)

Note the "i" as name of the constructor, which should have been "B".
This "i" is thus the last part of "__Q21At1B1Zi".

Unfortunately, I do not feel qualified enough to write a bug fix
for this, as I have no overview of the workings of cplus-dem.c.
There might be other bugs like this elsewhere, or I would fix this
but break something else :).

Thanks for the good work, I am looking forwards for a fix for this :)

Regards,

-- 
 Carlo Wood  <carlo@runaway.xs4all.nl>

CC To: egcs because they broke it in the first place
   To: utils because that is what the README of libiberty says
   To: H.J. as maintainer of gdb-4.17.0.8 and because I sent
       all previous reports about this also to him.