Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536

[Pedro Alves <palves@redhat.com>]

On 12/11/2018 06:58 AM, Jakub Jelinek wrote:
> On Mon, Dec 10, 2018 at 05:33:19PM -0700, Jeff Law wrote:
>>>> where di.num_comps is just strlen (mangled) * 2.  Without any analysis
>>>> whatsoever, bumping the "recursion" limit will just mean we can process 1.5
>>>> times long names.  Either we need more precise analysis on what we are
>>>> looking for (how big arrays we'll need) or it needs to be an independent
>>>> limit and certainly should allow say 10KB symbols too if they are
>>>> reasonable.
>>>
>>> If the problem is alloca, we could avoid using alloca if the size
>>> passes a threshold.  Perhaps even use a better data structure than a
>>> preallocated array based on a guess about the number of components...
>> Actually I would strongly suggest avoiding alloca completely.  This
>> isn't particularly performance sensitive code and alloca can be abused
>> in all kinds of interesting ways.
> 
> We can't use malloc, 

I noticed that the comment on top of __cxa_demangle says:

  "If OUTPUT_BUFFER is not long enough, it is expanded using realloc."

and __cxa_demangle calls 'free'.

And d_demangle, seemingly the workhorse for __cxa_demangle says:

/* Entry point for the demangler.  If MANGLED is a g++ v3 ABI mangled
   name, return a buffer allocated with malloc holding the demangled
   name.  OPTIONS is the usual libiberty demangler options.  On
   success, this sets *PALC to the allocated size of the returned
   buffer.  On failure, this sets *PALC to 0 for a bad name, or 1 for
   a memory allocation failure, and returns NULL.  */

cplus_demangle, the entry point that gdb uses, also relies on malloc.

Ian earlier mentioned that we've wanted to avoid malloc because some
programs call the demangler from a signal handler, but it seems like
we already do, these functions already aren't safe to use from
signal handlers as is.  Where does the "we can't use malloc" idea
come from?  Is there some entry point that avoids
the malloc/realloc/free calls?

Not advocating for adding to the number of malloc calls willy-nilly BTW.
I'd prefer to reduce the number of mallocs/rellocs calls within the
demangler and also within the bfd_demangle wrapper, for performance
reasons, for example by making it possible to reuse a growing buffer
across demangling invocations.

> therefore on some targets alloca (or VLAs) are the only
> option, and for small sizes even if mmap is available using it is too
> costly.
> 
> Though, I like Jason's suggestion of just adding a maxinum of the number
> of components and number of substitutions and failing if we need more.
> 
> 	Jakub

Thanks,
Pedro Alves