This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] Report call site for inlined functions
- From: Pedro Alves <palves at redhat dot com>
- To: Keith Seitz <keiths at redhat dot com>, gdb-patches at sourceware dot org
- Date: Tue, 18 Jul 2017 20:05:45 +0100
- Subject: Re: [PATCH 1/2] Report call site for inlined functions
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=palves at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7519A806A6
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7519A806A6
- References: <1499740601-15957-1-git-send-email-keiths@redhat.com>
On 07/11/2017 03:36 AM, Keith Seitz wrote:
> I have chosen to record the actual symbol that we found during the parse
> in the SaL. Later that information is copied into the bp_location.
At some point I think we'll just make bp_location hold a SaL. :-)
> From
> there, print_breakpoint_location can use this information to ascertain
> that the symbol is really an inlined function, and it can report the
> real symbol (foo) and inform the user of the actual call site (where the
> breakpoint is really set):
>
> (gdb) i b
> Num Type Disp Enb Address What
> 1 breakpoint keep y 0x0000000000400434 in foo at 1228556.c:3
> inlined in main
> at 1228556.c:10
>
> I have reported this information through to MI so that UI writers can inform
> their users as well:
>
> (gdb) interpreter-exec mi -break-info
> ^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1",col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="Enb"},{width="18",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_name="what",colhdr="What"}],body=[bkpt={number="1",type="breakpoint",disp="keep",enabled="y",addr="0x0000000000400434",func="foo",file="1228556.c",fullname="/home/keiths/tmp/1228556.c",line="3",call-site-func="main",call-site-file="1228556.c",call-site-fullname="/home/keiths/tmp/1228556.c",call-site-line="10",thread-groups=["i1"],times="0",original-location="foo"}]}
>
> Here you can see the new call-site-func, call-site-file, call-site-fullname,
> and call-site-line.
The non-inlined call site seems sufficient info to me, at least for
the CLI. I'm not sure whether "call-site" is sufficiently clear that
it's referring to that vs the immediate potential inlined-too
caller though. I think at least the docs need such a clarification.
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -6153,24 +6153,73 @@ print_breakpoint_location (struct breakpoint *b,
> uiout->field_string ("what", event_location_to_string (b->location.get ()));
> else if (loc && loc->symtab)
> {
> - struct symbol *sym
> - = find_pc_sect_function (loc->address, loc->section);
> - if (sym)
> + /* If a location is associated with an inlined symbol, print out
> + information about its call site, too. */
> + if (loc->symbol != NULL && SYMBOL_INLINED (loc->symbol))
> {
> + struct inlined_symbol *isym = (struct inlined_symbol *) (loc->symbol);
> + struct symbol *sym
> + = find_pc_sect_function (loc->address, loc->section);
> +
> + /* ISYM contains information about the inlined function, and
> + LOC->SYMBOL describes the call site. */
> uiout->text ("in ");
> - uiout->field_string ("func", SYMBOL_PRINT_NAME (sym));
> + uiout->field_string ("func", SYMBOL_PRINT_NAME (loc->symbol));
> uiout->text (" ");
> uiout->wrap_hint (wrap_indent_at_field (uiout, "what"));
> uiout->text ("at ");
> +
> + const char *s = symtab_to_filename_for_display (isym->decl_file);
> + uiout->field_string ("file", s);
> +
> + uiout->text (":");
> + if (uiout->is_mi_like_p ())
> + {
> + uiout->field_string ("fullname",
> + symtab_to_fullname (isym->decl_file));
> + }
Isn't there a good deal of duplication between the above and the
else branch?
> + uiout->field_int ("line", isym->decl_line);
> + uiout->text (" ");
> + uiout->wrap_hint (wrap_indent_at_field (uiout, "what"));
> + uiout->text ("inlined in ");
> + if (sym != NULL)
> + uiout->field_string ("call-site-func", SYMBOL_PRINT_NAME (sym));
> + uiout->text (" ");
> + uiout->wrap_hint (wrap_indent_at_field (uiout, "what"));
> + uiout->text ("at ");
> + s = symtab_to_filename_for_display (symbol_symtab (loc->symbol));
> + uiout->field_string ("call-site-file", s);
> +
> + uiout->text (":");
> + if (uiout->is_mi_like_p ())
> + {
> + s = symtab_to_fullname (symbol_symtab (loc->symbol));
> + uiout->field_string ("call-site-fullname", s);
> + }
> + uiout->field_int ("call-site-line", SYMBOL_LINE (loc->symbol));
> }
> - uiout->field_string ("file",
> - symtab_to_filename_for_display (loc->symtab));
> - uiout->text (":");
> + else
> + {
> + struct symbol *sym
> + = find_pc_sect_function (loc->address, loc->section);
>
I wonder whether the else branch could use loc->symbol now that you
have it, instead of looking it up.
> - if (uiout->is_mi_like_p ())
> - uiout->field_string ("fullname", symtab_to_fullname (loc->symtab));
> + if (sym)
> + {
> + uiout->text ("in ");
> + uiout->field_string ("func", SYMBOL_PRINT_NAME (sym));
> + uiout->text (" ");
> + uiout->wrap_hint (wrap_indent_at_field (uiout, "what"));
> + uiout->text ("at ");
> + }
> + uiout->field_string ("file",
> + symtab_to_filename_for_display (loc->symtab));
> + uiout->text (":");
> +
> + if (uiout->is_mi_like_p ())
> + uiout->field_string ("fullname", symtab_to_fullname (loc->symtab));
>
> - uiout->field_int ("line", loc->line_number);
> + uiout->field_int ("line", loc->line_number);
> + }
> }
> else if (loc)
> {
> @@ -8986,6 +9035,7 @@ add_location_to_breakpoint (struct breakpoint *b,
> loc->gdbarch = loc_gdbarch;
> loc->line_number = sal->line;
> loc->symtab = sal->symtab;
> + loc->symbol = sal->symbol;
>
> set_breakpoint_location_function (loc,
> sal->explicit_pc || sal->explicit_line);
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index d955184..2f10c3b 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -484,6 +484,11 @@ public:
> to find the corresponding source file name. */
>
> struct symtab *symtab = NULL;
> +
> + /* The symbol found by the location parser, if any. This may be used to
> + ascertain when an event location was set at a different location than
> + the one originally selected by parsing, e.g., inlined symbols. */
> + const struct symbol *symbol = NULL;
> };
>
> /* The possible return values for print_bpstat, print_it_normal,
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index c167a86..58cdc1a 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26755,6 +26755,14 @@ known. If not known, this field is not present.
> The line number at which this breakpoint appears, if known.
> If not known, this field is not present.
>
> +@item call-site-func
> +@item call-site-file
> +@item call-site-fullname
> +@item call-site-line
> +These fields describe the call site for a breakpoint set on an inlined function.
> +The fields are analogous to those fields of the same name for normal breakpoint
> +locations.
IMO, the non-inlined call site is sufficient info, at least for the
CLI. I'm not sure whether "call-site" is sufficiently clear that
it's referring to that vs the immediate potential inlined-too
caller though. I think at least the docs need that clarified.
> +
> @item at
> If the source file is not known, this field may be provided. If
> provided, this holds the address of the breakpoint, possibly followed
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 0fdcd42..3b3193b 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -11470,6 +11470,35 @@ inherit_abstract_dies (struct die_info *die, struct dwarf2_cu *cu)
> do_cleanups (cleanups);
> }
>
> +/* Get the symtab representing the KIND of file of DIE in CU or NULL
> + if no such file exists. KIND is any valid DWARF file attribute such as
> + DW_AT_decl_file or DW_AT_call_file. */
> +
> +static struct symtab *
> +dwarf2_file_symtab (unsigned int kind, struct die_info *die,
> + struct dwarf2_cu *cu)
> +{
> + struct attribute *attr = dwarf2_attr (die, kind, cu);
> +
> + if (attr != NULL)
> + {
> + file_name_index file_index = (file_name_index) DW_UNSND (attr);
> + struct file_entry *fe;
> +
> + if (cu->line_header != NULL)
> + fe = cu->line_header->file_name_at (file_index);
> + else
> + fe = NULL;
> +
> + if (fe == NULL)
> + complaint (&symfile_complaints, _("file index out of range"));
> + else
> + return fe->symtab;
> + }
> +
> + return NULL;
> +}
> +
> static void
> read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
> {
> @@ -11486,6 +11515,8 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
> int inlined_func = (die->tag == DW_TAG_inlined_subroutine);
> VEC (symbolp) *template_args = NULL;
> struct template_symbol *templ_func = NULL;
> + struct inlined_symbol *isym = NULL;
> + struct symbol *symbol_storage = NULL;
>
> if (inlined_func)
> {
> @@ -11540,13 +11571,28 @@ read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
> {
> templ_func = allocate_template_symbol (objfile);
> templ_func->base.is_cplus_template_function = 1;
> + symbol_storage = (struct symbol *) templ_func;
> break;
> }
> }
>
> + /* If we have an inlined symbol, we must also allocate a different
> + symbol. */
How does this work when you have an (inlined) template function?
Like e.g.,:
template<typename T>
static T inline_func (T val)
{
return val * 2;
}
int
not_inline_func (int val)
{
return inline_func (val * 2);
}
<1><31>: Abbrev Number: 2 (DW_TAG_subprogram)
<32> DW_AT_name : (indirect string, offset: 0x929): inline_func<int>
<36> DW_AT_decl_file : 1
<37> DW_AT_decl_line : 2
<38> DW_AT_type : <0x54>
<3c> DW_AT_inline : 1 (inlined)
<3d> DW_AT_sibling : <0x54>
> + if (inlined_func)
> + {
> + isym = allocate_inlined_symbol (objfile);
> + isym->decl_file = dwarf2_file_symtab (DW_AT_decl_file, die, cu);
> +
> + attr = dwarf2_attr (die, DW_AT_decl_line, cu);
> + if (attr != NULL)
> + isym->decl_line = DW_UNSND (attr);
> +
> + symbol_storage = (struct symbol *) isym;
> + }
> +
> newobj = push_context (0, lowpc);
> newobj->name = new_symbol_full (die, read_type_die (die, cu), cu,
> - (struct symbol *) templ_func);
> + symbol_storage);
>
> /* If there is a location expression for DW_AT_frame_base, record
> it. */
> @@ -18948,25 +18994,11 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
> SYMBOL_LINE (sym) = DW_UNSND (attr);
> }
>
> /* See symtab.h. */
>
> struct objfile *
> diff --git a/gdb/symtab.h b/gdb/symtab.h
> index 341deca..a70ed54 100644
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -910,6 +910,21 @@ struct template_symbol
> struct symbol **template_arguments;
> };
>
> +/* A superclass of struct symbol used to represent an inlined symbol.
> + A symbol is really of this type if SYMBOL_INLINED is true. */
> +
> +struct inlined_symbol
> +{
> + /* The base class. */
> + struct symbol base;
Couldn't this be real C++ inheritance?
struct inlined_symbol : symbol
{
...
};
I ran out of time ... more later.
Thanks,
Pedro Alves