This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/2] Report call site for inlined functions


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]