Re: [RFA] libiberty/cplus-dem.c:demangle_template() problem?

Re: [RFA] libiberty/cplus-dem.c:demangle_template() problem?

[Joel Brobecker]

Hello,

I just committed the following change to libiberty in the GCC tree.
Apparently, the libiberty repository is not "shared" between GCC and
GDB. This fixes a SIGSEGV that occured in GDB. Is it ok to import
this change in the GDB sources?

Thanks,

On Sat, Oct 18, 2003 at 10:59:40PM -0700, Joel Brobecker wrote:
> I noticed a SIGSEGV inside GDB while reading the symbol table.
> What GDB does with each symbol is try to compute their demangled
> name. The SEGV occured because the compiler I used (GNAT) generated
> a symbol which the demangle did not like:
> 
>       _test_array__L_1__B23b___clean.6
> 
> GDB basically called cplus_demangle() with the above name, and kaboom!
> SIGSEGV inside work_stuff_copy_to_from().
> 
> What happened is that cplus_demangle() ends up trying to demangle the
> symbol using gnu_special() which tries to see if the symbol is a
> template by calling demangle_template().
> 
> The value given for parameter REMEMBER is 1, so the first thing the
> function does is registering a Btype inside the work_stuff structure.
> But as it realizes it actually is not a template, it aborts the
> execution and returns zero. However, the work->btypevec vector now
> contains a NULL entry. When the code later tries to make a copy of the
> work_stuff structure, if segfaults because it's trying to copy a NULL
> string. 
> 
> I think the right fix is to only register the Btype when we know we
> are going to store it. In the present case, the attached patch seemed
> to be the right fix. I also attached a patch for the testsuite.  The
> testdriver segfaults before I apply my patch, and runs to completion
> after. The output is unchanged.
> 
> 2003-10-19  J. Brobecker  <brobecker@gnat.com>
> 
> 	* cplus-dem.c (demangle_template): Register a new Btype only
> 	when needed.
> 	* testsuite/demangle-expected: Add a new test.
> 
> OK to apply?
> 
> Thanks,
> -- 
> Joel

Content-Description: cplus-dem.c.diff
> Index: cplus-dem.c
> ===================================================================
> RCS file: /nile.c/cvs/Dev/gdb/gdb-6.0/libiberty/cplus-dem.c,v
> retrieving revision 1.1.1.1
> diff -u -p -r1.1.1.1 cplus-dem.c
> --- cplus-dem.c	5 Oct 2003 10:40:46 -0000	1.1.1.1
> +++ cplus-dem.c	18 Oct 2003 06:42:31 -0000
> @@ -2043,13 +2043,10 @@ demangle_template (work, mangled, tname,
>    const char *start;
>    int is_java_array = 0;
>    string temp;
> -  int bindex = 0;
>  
>    (*mangled)++;
>    if (is_type)
>      {
> -      if (remember)
> -	bindex = register_Btype (work);
>        start = *mangled;
>        /* get template name */
>        if (**mangled == 'z')
> @@ -2226,7 +2223,10 @@ demangle_template (work, mangled, tname,
>      }
>  
>    if (is_type && remember)
> -    remember_Btype (work, tname->b, LEN_STRING (tname), bindex);
> +    {
> +      const int bindex = register_Btype (work);
> +      remember_Btype (work, tname->b, LEN_STRING (tname), bindex);
> +    }
>  
>    /*
>      if (work -> static_type)

> Index: demangle-expected
> ===================================================================
> RCS file: /cvs/src/src/libiberty/testsuite/demangle-expected,v
> retrieving revision 1.13
> diff -u -r1.13 demangle-expected
> --- demangle-expected	16 Oct 2003 15:22:27 -0000	1.13
> +++ demangle-expected	18 Oct 2003 19:25:46 -0000
> @@ -2864,3 +2864,9 @@
>  --format=auto
>  __CPR212____ct__Q3_3std141list__tm__128_Q2_3edm41THandle__tm__26_Q2_4emid15EMparticleChunkQ2_3std68allocator__tm__51_Q2_3edmJ37J14const_iteratorFRCQ3_3std18list__tm__7_Z1ZZ2Z8iterator
>  __CPR212____ct__Q3_3std141list__tm__128_Q2_3edm41THandle__tm__26_Q2_4emid15EMparticleChunkQ2_3std68allocator__tm__51_Q2_3edmJ37J14const_iteratorFRCQ3_3std18list__tm__7_Z1ZZ2Z8iterator
> +#
> +# This used to cause a crash. It doesn't follow the C++ encoding so
> +# the demangled name should be identical to the original symbol name.
> +--format=auto
> +_test_array__L_1__B23b___clean.6
> +_test_array__L_1__B23b___clean.6


-- 
Joel

Re: [RFA] libiberty/cplus-dem.c:demangle_template() problem?

[Andrew Cagney]

> Hello,
> 
> I just committed the following change to libiberty in the GCC tree.
> Apparently, the libiberty repository is not "shared" between GCC and
> GDB. This fixes a SIGSEGV that occured in GDB. Is it ok to import
> this change in the GDB sources?

Yes.  Any dj approved upstream change to gcc/libiberty is implicitly 
approved for src/libiberty.

Andrew

Re: [RFA] libiberty/cplus-dem.c:demangle_template() problem?

[DJ Delorie]

> Yes.  Any dj approved upstream change to gcc/libiberty is implicitly 
> approved for src/libiberty.

And if you don't do it yourself, the machine usually handles it after
a while.

Re: [RFA] libiberty/cplus-dem.c:demangle_template() problem?

[Joel Brobecker]

> >I just committed the following change to libiberty in the GCC tree.
> >Apparently, the libiberty repository is not "shared" between GCC and
> >GDB. This fixes a SIGSEGV that occured in GDB. Is it ok to import
> >this change in the GDB sources?
> 
> Yes.  Any dj approved upstream change to gcc/libiberty is implicitly 
> approved for src/libiberty.

OK, thank you both for clarifying how it works. I just checked the
change in src as well.

-- 
Joel