This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] PR python/18565 - make Frame.function work for inline frames
- From: Pedro Alves <palves at redhat dot com>
- To: Tom Tromey <tom at tromey dot com>
- Cc: Yao Qi <qiyaoltc at gmail dot com>, "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Tue, 26 Jul 2016 12:14:23 +0100
- Subject: Re: [RFA] PR python/18565 - make Frame.function work for inline frames
- Authentication-results: sourceware.org; auth=none
- References: <1466439050-11330-1-git-send-email-tom@tromey.com> <86ziqfq6sz.fsf@gmail.com> <8737o5kqtv.fsf@tromey.com> <CAH=s-PMf2Encmm7JFLBDdrP6e07fEddCiVZGEfU+rkrunbSm-A@mail.gmail.com> <d861e87d-a94d-5ae9-5d51-3f7f8ad0ccc1@redhat.com> <87bn1lu5h9.fsf@tromey.com>
On 07/25/2016 04:01 PM, Tom Tromey wrote:
> *funname = xstrdup (SYMBOL_PRINT_NAME (func));
> [...]
> if (*funlang == language_cplus)
> {
> /* It seems appropriate to use SYMBOL_PRINT_NAME() here,
> to display the demangled name that we already have
> stored in the symbol table, but we stored a version
> with DMGL_PARAMS turned on, and here we don't want to
> display parameters. So remove the parameters. */
> char *func_only = cp_remove_params (*funname);
>
> I'm not 100% sure that cp_remove_params cannot throw.
> However, it's
> simple to deal with this by adding a cleanup in find_frame_funname. I'm
> happy to do this if desired.
>
> Another approach might be to have a free_current_contents cleanup at the
> start of find_frame_funname and discard it at the exit. This would
> maybe make it a bit safer in the face of future changes.
Yet another approach would be to push the xstrdup call to after the
cp_remove_params call, and remove the xfree call, something like:
if (*funlang == language_cplus)
{
char *func_only = cp_remove_params (SYMBOL_PRINT_NAME (func));
if (func_only)
*funname = func_only;
else
*funname = xstrdup (SYMBOL_PRINT_NAME (func));
}
else
*funname = xstrdup (SYMBOL_PRINT_NAME (func));
}
In any case, IMO this would be the subject of a separate patch.
> Alternatively, if we need a try/catch in the caller to possibly free the
> function name, then several other callers are incorrect (ada-lang.c and
> stack.c).
Yeah. I think that if a function has such a requirement, then it needs
to be clearly documented as that being part of its API contract.
Otherwise, it's too easy for the called function to change in a way that
makes the caller try to free a dangling output pointer.
Thanks,
Pedro Alves