Re: [RFA take 2] Allow setting breakpoints on inline functions (PR 10738)

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Tue, 06 Dec 2011 15:31:25 +0100, Gary Benson wrote:
> 	* gdb.opt/inline-break.exp: New file.
> 	* gdb.opt/inline-break.c: Likewise.
> 	* gdb.dwarf2/inline-break.exp: Likewise.
> 	* gdb.dwarf2/inline-break.S: Likewise.

The basenames should be different as dejagnu identifies testcases for example
for --ignore just by their basenames.


I found such a little disadvantage - but that is a bug (not regression) of
Tom's linespec patch IMO:

1	int v;
2	extern void f (void);
3	void g (void) {
4	  f ();
5	}
6	void f (void) {
7	  v++;
8	}

(gdb) b f
Breakpoint 1 at 0x0: f. (2 locations)
(gdb) info breakpoints 
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <MULTIPLE>         
1.1                         y     0x0000000000000000 in g at 1.c:7
1.2                         y     0x0000000000000010 in f at 1.c:7

The line numbers are based on .debug_line.  But DWARF knows the real line
numbers:
 <1><2d>: Abbrev Number: 2 (DW_TAG_subprogram)
    <2e>   DW_AT_name        : f
    <31>   DW_AT_decl_line   : 6
 <1><33>: Abbrev Number: 3 (DW_TAG_subprogram)
    <34>   DW_AT_name        : g
 <2><4e>: Abbrev Number: 4 (DW_TAG_inlined_subroutine)
    <64>   DW_AT_call_line   : 4

I consider this as an unrelated Bug/RFE.


> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -578,6 +578,7 @@ struct partial_die_info
>      unsigned int has_type : 1;
>      unsigned int has_specification : 1;
>      unsigned int has_pc_info : 1;
> +    unsigned int may_be_inlined : 1;
>  
>      /* Flag set if the SCOPE field of this structure has been
>         computed.  */
> @@ -4285,6 +4286,10 @@ add_partial_subprogram (struct partial_die_info *pdi,
>  				 pdi->highpc - 1 + baseaddr,
>  				 cu->per_cu->v.psymtab);
>  	    }
> +        }
> +
> +      if (pdi->has_pc_info || (!pdi->is_external && pdi->may_be_inlined))
> +	{
>            if (!pdi->is_declaration)
>  	    /* Ignore subprogram DIEs that do not have a name, they are
>  	       illegal.  Do not emit a complaint at this point, we will
> @@ -9925,6 +9930,11 @@ read_partial_die (struct partial_die_info *part_die,
>  	      language_of_main = language_fortran;
>  	    }
>  	  break;
> +	case DW_AT_inline:
> +	  if (DW_UNSND (&attr) == DW_INL_inlined
> +	      || DW_UNSND (&attr) == DW_INL_declared_inlined)
> +	    part_die->may_be_inlined = 1;
> +	  break;
>  	default:
>  	  break;
>  	}
> @@ -11800,8 +11810,7 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>  	     finish_block.  */
>  	  SYMBOL_CLASS (sym) = LOC_BLOCK;
>  	  SYMBOL_INLINED (sym) = 1;
> -	  /* Do not add the symbol to any lists.  It will be found via
> -	     BLOCK_FUNCTION from the blockvector.  */
> +	  list_to_add = &file_symbols;
>  	  break;
>  	case DW_TAG_template_value_param:
>  	  suppress_add = 1;

I do not find it completely great to mess inlined functions into the
STATIC_BLOCK symbols, I thought more about either searching all the block
vectors or to use some other index to find all the inlined instances.

But I am fine with STATIC_BLOCK as long as you catch all the exceptions.
For example search_symbols also needs an exception, the countercase:
(gdb) l
1	int v;
2	extern void f (void);
3	void g (void) {
4	  f ();
5	}
6	void f (void) {
7	  v++;
8	}
(gdb) info functions f
All functions matching regular expression "f":
File 1.c:
void f(void);
static void f(void);


Maybe also default_make_symbol_completion_list_break_on and
make_file_symbol_completion_list need an exception:
(gdb) l
1	int v;
2	static void f (void);
3	void g (void) {
4	  f ();
5	}
6	static void f (void) {
7	  v++;
8	}
(gdb) p <tab><tab>
2.c  f    g    int  v    
(gdb) p f
No symbol "f" in current context.


And also rbreak_command needs an exception:
(gdb) l
1	int v;
2	extern void f (void);
3	void g (void) {
4	  f ();
5	}
6	void f (void) {
7	  v++;
8	}
(gdb) rbreak f
Breakpoint 1 at 0x0: 1.c:f. (2 locations)
void f(void);
Note: breakpoint 1 also set at pc 0x0.
Note: breakpoint 1 also set at pc 0x10.
Breakpoint 2 at 0x0: 1.c:f. (2 locations)
static void f(void);
(gdb) info breakpoints 
Num     Type           Disp Enb Address            What
1       breakpoint     keep y   <MULTIPLE>         
1.1                         y     0x0000000000000000 in g at 1.c:7
1.2                         y     0x0000000000000010 in f at 1.c:7
2       breakpoint     keep y   <MULTIPLE>         
2.1                         y     0x0000000000000000 in g at 1.c:7
2.2                         y     0x0000000000000010 in f at 1.c:7


> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index 68b80be..275c201 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -1781,8 +1781,9 @@ lookup_block_symbol (const struct block *block, const char *name,
>  	   sym != NULL;
>  	   sym = dict_iter_name_next (name, &iter))
>  	{
> -	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
> -				     SYMBOL_DOMAIN (sym), domain))
> +	  if (!SYMBOL_INLINED (sym)
> +	      && symbol_matches_domain (SYMBOL_LANGUAGE (sym),
> +					SYMBOL_DOMAIN (sym), domain))
>  	    return sym;
>  	}
>        return NULL;

It should be also in the second part of this function.  When the current scope
is in function f which contains inlined function g then command `break g'
could again pick the inlined variant.


> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/inline-break.S
> +	.long	.LASF19	# DW_AT_comp_dir: "/home/gary/work/archer/src/gdb/testsuite/gdb.dwarf2"
> +	.string	"/home/gary/work/archer/src/gdb/testsuite/gdb.dwarf2"

Maybe you want to delete those strings but maybe not.



Thanks,
Jan