Re: [PATCH 1/2] Fix debug info for ignored decls at start of assembly

[Richard Biener <rguenther@suse.de>]

On Fri, 30 Jul 2021, Bernd Edlinger wrote:

> 
> 
> On 7/29/21 9:23 AM, Richard Biener wrote:
> > On Wed, 28 Jul 2021, Bernd Edlinger wrote:
> > 
> >> On 7/28/21 2:51 PM, Richard Biener wrote:
> >>> On Mon, 26 Jul 2021, Bernd Edlinger wrote:
> >>>
> >>>> Ignored functions decls that are compiled at the start of
> >>>> the assembly have bogus line numbers until the first .file
> >>>> directive, as reported in PR101575.
> >>>>
> >>>> The work around for this issue is to emit a dummy .file
> >>>> directive when the first function is DECL_IGNORED_P, when
> >>>> that is not already done, mostly for -fdwarf-4.
> >>>
> >>> I wonder if it makes sense to unconditionally announce the
> >>> TU with a .file directive at the beginning.  ISTR this is
> >>> what we now do with -gdwarf-5.
> >>>
> >>
> >> Yes, that would work, even when the file name is not guessed
> >> correctly.
> >>
> >> Initially I had "<dummy>" unconditionally here, and it did
> >> not really hurt, except that it is visible with readelf.
> > 
> > I think I'd prefer that, since if we don't announce a .file
> > before the first assembler statement but ask gas to produce
> > line info it might be tempted to create line info referencing
> > the possibly temporary filename of the assembler file which
> > is undesirable from a build reproducability point.
> > 
> 
> Yeah, I understand.
> 
> Meanwhile I found a simple C test case without ignored functions
> 
> $ cat test1.c
> asm("nop");
> int main () 
> {
>   return 0;
> }
> 
> $ gcc -g test1.c
> $ readelf --debug-dump=decodedline a.out 
> Contents of the .debug_line section:
> 
> CU: ./test1.c:
> File name                            Line number    Starting address    View    Stmt
> test1.c                                        5            0x401106               x
> test1.c                                        3            0x401107               x
> test1.c                                        4            0x40110b               x
> test1.c                                        5            0x401110               x
> test1.c                                        -            0x401112
> 
> even with the proposed patch, so I agree it is incomplete.
> 
> I tried the gdb test case and compile it with different LTO
> options, but the gen_AT_string was always valid, in some
> cases a lto debug section together with a couple .file <n>
> directives was output before the .file 0.
> So I'd like to use the file name from gen_AT_string, since
> it's most of the time accurate, and avoids unnecessary confusion
> on the readers of the produced debug info.
> 
> So I'd propose the attached patch instead.
> Is it OK for trunk?

Works for me - please give others a chance to comment though.

Thanks,
Richard.

> 
> > Richard.
> > 
> >>> Note get_AT_string (comp_unit_die (), DW_AT_name) doesn't
> >>> work with LTO, you'll get <dummy> then.
> >>>
> >>
> >> Yeah, that's why I wanted to restrict that to the case where
> >> it's absolutely necessary.
> >>
> >>> Is the dwarf assembler bug reported/fixed?  Can you include
> >>> a reference please?
> >>>
> >>
> >> I've just added a bug report, it's unlikely to be fixed IMHO:
> >> https://sourceware.org/bugzilla/show_bug.cgi?id=28149
> >>
> >> I will add that to the patch description:
> >>
> >> Ignored functions decls that are compiled at the start of
> >> the assembly have bogus line numbers until the first .file
> >> directive, as reported in PR101575.
> >>
> >> The corresponding binutils bug report is
> >> https://sourceware.org/bugzilla/show_bug.cgi?id=28149
> >>
> >> The work around for this issue is to emit a dummy .file
> >> directive when the first function is DECL_IGNORED_P, when
> >> that is not already done, mostly for -fdwarf-4.
> >>
> >>
> >> Thanks
> >> Bernd.
> >>
> >>> Thanks,
> >>> Richard.
> >>>
> >>>> 2021-07-24  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> >>>>
> >>>> 	PR ada/101575
> >>>> 	* dwarf2out.c (dwarf2out_begin_prologue): Move init
> >>>> 	of fde->ignored_debug to dwarf2out_set_ignored_loc.
> >>>> 	(dwarf2out_set_ignored_loc): This is now also called
> >>>> 	when no .loc statement is to be generated, in that case
> >>>> 	we emit a dummy .file statement when needed.
> >>>> 	* final.c (final_start_function_1,
> >>>> 	final_scan_insn_1): Call debug_hooks->set_ignored_loc
> >>>> 	for all DECL_IGNORED_P functions.
> >>>> ---
> >>>>  gcc/dwarf2out.c | 29 +++++++++++++++++++++++++----
> >>>>  gcc/final.c     |  5 ++---
> >>>>  2 files changed, 27 insertions(+), 7 deletions(-)
> >>>>
> >>>> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> >>>> index 884f1e1..8de0d6f 100644
> >>>> --- a/gcc/dwarf2out.c
> >>>> +++ b/gcc/dwarf2out.c
> >>>> @@ -1115,7 +1115,6 @@ dwarf2out_begin_prologue (unsigned int line ATTRIBUTE_UNUSED,
> >>>>    fde->dw_fde_current_label = dup_label;
> >>>>    fde->in_std_section = (fnsec == text_section
> >>>>  			 || (cold_text_section && fnsec == cold_text_section));
> >>>> -  fde->ignored_debug = DECL_IGNORED_P (current_function_decl);
> >>>>    in_text_section_p = fnsec == text_section;
> >>>>  
> >>>>    /* We only want to output line number information for the genuine dwarf2
> >>>> @@ -28546,10 +28545,32 @@ dwarf2out_set_ignored_loc (unsigned int line, unsigned int column,
> >>>>  {
> >>>>    dw_fde_ref fde = cfun->fde;
> >>>>  
> >>>> -  fde->ignored_debug = false;
> >>>> -  set_cur_line_info_table (function_section (fde->decl));
> >>>> +  if (filename)
> >>>> +    {
> >>>> +      set_cur_line_info_table (function_section (fde->decl));
> >>>> +
> >>>> +      dwarf2out_source_line (line, column, filename, 0, true);
> >>>> +    }
> >>>> +  else
> >>>> +    {
> >>>> +      fde->ignored_debug = true;
> >>>> +
> >>>> +      /* Work around for PR101575: output a dummy .file directive.  */
> >>>> +      if (in_first_function_p
> >>>> +	  && debug_info_level >= DINFO_LEVEL_TERSE
> >>>> +	  && dwarf_debuginfo_p ()
> >>>> +#if defined(HAVE_AS_GDWARF_5_DEBUG_FLAG) && defined(HAVE_AS_WORKING_DWARF_N_FLAG)
> >>>> +	  && dwarf_version <= 4
> >>>> +#endif
> >>>> +	  && output_asm_line_debug_info ())
> >>>> +	{
> >>>> +	  const char *filename0 = get_AT_string (comp_unit_die (), DW_AT_name);
> >>>>  
> >>>> -  dwarf2out_source_line (line, column, filename, 0, true);
> >>>> +	  if (filename0 == NULL)
> >>>> +	    filename0 = "<dummy>";
> >>>> +	  maybe_emit_file (lookup_filename (filename0));
> >>>> +	}
> >>>> +    }
> >>>>  }
> >>>>  
> >>>>  /* Record the beginning of a new source file.  */
> >>>> diff --git a/gcc/final.c b/gcc/final.c
> >>>> index ac6892d..82a5767 100644
> >>>> --- a/gcc/final.c
> >>>> +++ b/gcc/final.c
> >>>> @@ -1725,7 +1725,7 @@ final_start_function_1 (rtx_insn **firstp, FILE *file, int *seen,
> >>>>    if (!dwarf2_debug_info_emitted_p (current_function_decl))
> >>>>      dwarf2out_begin_prologue (0, 0, NULL);
> >>>>  
> >>>> -  if (DECL_IGNORED_P (current_function_decl) && last_linenum && last_filename)
> >>>> +  if (DECL_IGNORED_P (current_function_decl))
> >>>>      debug_hooks->set_ignored_loc (last_linenum, last_columnnum, last_filename);
> >>>>  
> >>>>  #ifdef LEAF_REG_REMAP
> >>>> @@ -2205,8 +2205,7 @@ final_scan_insn_1 (rtx_insn *insn, FILE *file, int optimize_p ATTRIBUTE_UNUSED,
> >>>>  	    }
> >>>>  	  else if (!DECL_IGNORED_P (current_function_decl))
> >>>>  	    debug_hooks->switch_text_section ();
> >>>> -	  if (DECL_IGNORED_P (current_function_decl) && last_linenum
> >>>> -	      && last_filename)
> >>>> +	  if (DECL_IGNORED_P (current_function_decl))
> >>>>  	    debug_hooks->set_ignored_loc (last_linenum, last_columnnum,
> >>>>  					  last_filename);
> >>>>  
> >>>>
> >>>
> >>
> > 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)