This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536
- From: Pedro Alves <palves at redhat dot com>
- To: Jakub Jelinek <jakub at redhat dot com>, Jeff Law <law at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>, Michael Matz <matz at suse dot de>, Nick Clifton <nickc at redhat dot com>, Ian Lance Taylor <iant at google dot com>, "H.J. Lu" <hjl dot tools at gmail dot com>, Richard Biener <richard dot guenther at gmail dot com>, Scott Gayou <sgayou at redhat dot com>, Tom Tromey <tom at tromey dot com>, gcc-patches List <gcc-patches at gcc dot gnu dot org>, Binutils <binutils at sourceware dot org>
- Date: Tue, 11 Dec 2018 11:05:25 +0000
- Subject: Re: [PATCH] Set DEMANGLE_RECURSION_LIMIT to 1536
- References: <c7c959ca-b8bf-bd3e-a65d-bb274a3118d3@redhat.com> <fca558b7-9ed3-76d0-176c-03f64790e3f1@redhat.com> <2f4c983b-494f-93ba-d6c6-1fe0a9730a76@redhat.com> <CAKOQZ8y=B6beozokJ2tdAAkVDVue08ogehMP7TAXvrPzdz9MuQ@mail.gmail.com> <CAMe9rOomd2E3C03CxTXyTRkq6HG32OX+rbMPS3y6dcEWmwaMYg@mail.gmail.com> <CAMe9rOokMpaAUFk0rcYTTUQTQhEMn-VQetXfiDTDXYdTXZEJTA@mail.gmail.com> <alpine.LSU.2.21.1812101442470.5354@wotan.suse.de> <20181210151020.GA12380@tucnak> <CADzB+2nX8UO94EjRiSegxFXBKwb7qmYLLpSjbJmXkZNo20M7Lg@mail.gmail.com> <2b97a775-ed9a-9bd2-e574-52b679f464c7@redhat.com> <20181211065804.GH12380@tucnak>
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