[patch] gas dwarf2 debug emission

[patch] gas dwarf2 debug emission

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

Hi,
this patch fixes a problem with dwarf2 .debug_line emission.  The logic
in dwarf2_finish is incorrect and fails for empty C source files.

GCC will emit an empty .debug_info section for an empty source file,
as it needs to create a label in that section before it knows whether
the section will be empty or not.  This can happen for some files in
libgcc2 (in my case it was the clz file).  Gas would emit a .debug_line
section for the empty file, but not emit the associated .debug_info and
other sections.

* The check for the .debug_info section should verify that the section
is non-empty.

* It is not necessary to test for debug_type == DEBUG_DWARF2, as all_segs
will be non-null if there is any line information to emit (either implicitly
generated or explicitly via .loc/.file directives).

* I think the 'files_in_use == 0' check is an incorrect attempt to fix the
problem I observed.

Built and tested on i686-pc-linux-gnu, and on an unreleased port where I
observed the problem.  ok?

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk


[-- Attachment #2: debug.patch --]
[-- Type: text/plain, Size: 4839 bytes --]

2005-01-22  Nathan Sidwell  <nathan@codesourcery.com>

	* dwarf2dbg.c (dwarf2_finish): Correct logic for determining when
	to emit .debug_line and other debug sections.
	* as.h (subseg_empty_p): Declare.
	* subsegs.c (subseg_empty_p): New predicate.

Index: gas/as.h
===================================================================
RCS file: /cvs/src/src/gas/as.h,v
retrieving revision 1.40
diff -c -3 -p -r1.40 as.h
*** gas/as.h	21 Jan 2005 05:54:38 -0000	1.40
--- gas/as.h	22 Jan 2005 17:27:22 -0000
*************** segT   subseg_new (const char *, subsegT
*** 577,582 ****
--- 577,583 ----
  segT   subseg_force_new (const char *, subsegT);
  void   subseg_set (segT, subsegT);
  int    subseg_text_p (segT);
+ int    subseg_empty_p (segT);
  void   start_dependencies (char *);
  void   register_dependency (char *);
  void   print_dependencies (void);
Index: gas/dwarf2dbg.c
===================================================================
RCS file: /cvs/src/src/gas/dwarf2dbg.c,v
retrieving revision 1.72
diff -c -3 -p -r1.72 dwarf2dbg.c
*** gas/dwarf2dbg.c	22 Nov 2004 16:29:33 -0000	1.72
--- gas/dwarf2dbg.c	22 Jan 2005 17:27:25 -0000
*************** out_debug_info (segT info_seg, segT abbr
*** 1349,1369 ****
    symbol_set_value_now (info_end);
  }
  
  void
  dwarf2_finish (void)
  {
    segT line_seg;
    struct line_seg *s;
  
!   /* We don't need to do anything unless:
!      - Some debug information was recorded via .file/.loc or
!        generated by GAS (--gdwarf2)
!      - or, there is a user-provided .debug_info section which could
!        reference the file table in the .debug_line section we generate
!        below.  */
!   if (all_segs == NULL
!       && (bfd_get_section_by_name (stdoutput, ".debug_info") == NULL
! 	  || files_in_use == 0))
      return;
  
    /* Calculate the size of an address for the target machine.  */
--- 1349,1376 ----
    symbol_set_value_now (info_end);
  }
  
+ /* Finish the dwarf2 debug sections.  We emit .debug.line if there
+    were any .file/.loc directives, or --gdwarf2 was given, or if the
+    file has a non-empty .debug_info section.  If we emit .debug_line,
+    and the .debug_info section is empty, we also emit .debug_info,
+    .debug_aranges and .debug_abbrev.  ALL_SEGS will be non-null if
+    there were any .file/.loc directives, or --gdwarf2 was given and
+    there were any located instructions emitted.  */
+ 
  void
  dwarf2_finish (void)
  {
    segT line_seg;
    struct line_seg *s;
+   segT info_seg;
+   int emit_other_sections = 0;
+ 
+   info_seg = bfd_get_section_by_name (stdoutput, ".debug_info");
+   emit_other_sections = !info_seg || subseg_empty_p (info_seg);
  
!   if (!all_segs && emit_other_sections)
!     /* There is no line information and no non-empty .debug_info
!        section.  */
      return;
  
    /* Calculate the size of an address for the target machine.  */
*************** dwarf2_finish (void)
*** 1388,1401 ****
  
    out_debug_line (line_seg);
  
!   /* If this is assembler generated line info, we need .debug_info
!      and .debug_abbrev sections as well.  */
!   if (all_segs != NULL && debug_type == DEBUG_DWARF2)
      {
        segT abbrev_seg;
-       segT info_seg;
        segT aranges_seg;
  
        info_seg = subseg_new (".debug_info", 0);
        abbrev_seg = subseg_new (".debug_abbrev", 0);
        aranges_seg = subseg_new (".debug_aranges", 0);
--- 1395,1410 ----
  
    out_debug_line (line_seg);
  
!   /* If this is assembler generated line info, and there is no
!      debug_info already, we need .debug_info and .debug_abbrev
!      sections as well.  */
!   if (emit_other_sections)
      {
        segT abbrev_seg;
        segT aranges_seg;
  
+       assert (all_segs);
+       
        info_seg = subseg_new (".debug_info", 0);
        abbrev_seg = subseg_new (".debug_abbrev", 0);
        aranges_seg = subseg_new (".debug_aranges", 0);
Index: gas/subsegs.c
===================================================================
RCS file: /cvs/src/src/gas/subsegs.c,v
retrieving revision 1.18
diff -c -3 -p -r1.18 subsegs.c
*** gas/subsegs.c	21 Jan 2005 05:54:38 -0000	1.18
--- gas/subsegs.c	22 Jan 2005 17:27:26 -0000
*************** subseg_text_p (segT sec)
*** 595,600 ****
--- 595,622 ----
  #endif /* ! BFD_ASSEMBLER */
  }
  
+ /* Return nonzero if SEC has at least one byte of data.  */
+ 
+ int
+ subseg_empty_p (segT sec)
+ {
+   segment_info_type *seginfo = seg_info (sec);
+   frchainS *chain;
+   fragS *frag;
+ 
+   if (!seginfo)
+     return 1;
+   
+   chain = seginfo->frchainP;
+   for (frag = chain->frch_root; frag; frag = frag->fr_next)
+     if (frag->fr_fix)
+       return 0;
+   if (obstack_next_free (&chain->frch_obstack)
+       == chain->frch_last->fr_literal)
+     return 1;
+   return 0;
+ }
+ 
  void
  subsegs_print_statistics (FILE *file)
  {

Re: [patch] gas dwarf2 debug emission

[Alan Modra]

On Sat, Jan 22, 2005 at 05:42:08PM +0000, Nathan Sidwell wrote:
> +   emit_other_sections = !info_seg || subseg_empty_p (info_seg);

The convention in binutils is to compare non-boolean values against NULL
or 0, so

  emit_other_sections = info_seg == NULL || subseg_empty_p (info_seg);

> + /* Return nonzero if SEC has at least one byte of data.  */
> + 
> + int
> + subseg_empty_p (segT sec)

Comment doesn't match function return value.  More seriously

o  You're using the function to test whether a section is non-empty,
   but the code only looks at the first sub-section.  The function ought
   to run down frch_next, and be called seg_empty_p (or section_empty_p,
   gas nomenclature is a little confusing).

o  The function doesn't handle a number of frag types that might have
   fr_fix zero but still contribute to section size.  Correcting this
   isn't easy, and unfortunately I think it will be possible to
   construct cases where it's impossible to guess the frag size at this
   stage of assembly.  So I'll let this pass if you add a comment.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] gas dwarf2 debug emission

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

Alan Modra wrote:
> On Sat, Jan 22, 2005 at 05:42:08PM +0000, Nathan Sidwell wrote:

> o  You're using the function to test whether a section is non-empty,
>    but the code only looks at the first sub-section.  The function ought
>    to run down frch_next, and be called seg_empty_p (or section_empty_p,
>    gas nomenclature is a little confusing).
> 
> o  The function doesn't handle a number of frag types that might have
>    fr_fix zero but still contribute to section size.  Correcting this
>    isn't easy, and unfortunately I think it will be possible to
>    construct cases where it's impossible to guess the frag size at this
>    stage of assembly.  So I'll let this pass if you add a comment.

Like this?  I chose 'seg_not_empty_p' to emphasize that a false result
might not indicate actual emptiness.

built & tested on i686-pc-linux-gnu.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::     CodeSourcery LLC
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk


[-- Attachment #2: debug.patch --]
[-- Type: text/plain, Size: 5973 bytes --]

2005-01-23  Nathan Sidwell  <nathan@codesourcery.com>

	* dwarf2dbg.c (dwarf2_finish): Correct logic for determining when
	to emit .debug_line and other debug sections.
	* as.h (seg_not_empty_p): Declare.
	* subsegs.c (seg_not_empty_p): New predicate.

Index: gas/as.h
===================================================================
RCS file: /cvs/src/src/gas/as.h,v
retrieving revision 1.40
diff -c -3 -p -r1.40 as.h
*** gas/as.h	21 Jan 2005 05:54:38 -0000	1.40
--- gas/as.h	24 Jan 2005 16:54:32 -0000
*************** void   input_scrub_close (void);
*** 572,589 ****
  void   input_scrub_end (void);
  int    new_logical_line (char *, int);
  void   subsegs_begin (void);
- void   subseg_change (segT, int);
  segT   subseg_new (const char *, subsegT);
  segT   subseg_force_new (const char *, subsegT);
  void   subseg_set (segT, subsegT);
  int    subseg_text_p (segT);
  void   start_dependencies (char *);
  void   register_dependency (char *);
  void   print_dependencies (void);
  #ifdef BFD_ASSEMBLER
  segT   subseg_get (const char *, int);
  #endif
! 
  
  struct expressionS;
  struct fix;
--- 572,589 ----
  void   input_scrub_end (void);
  int    new_logical_line (char *, int);
  void   subsegs_begin (void);
  segT   subseg_new (const char *, subsegT);
  segT   subseg_force_new (const char *, subsegT);
  void   subseg_set (segT, subsegT);
  int    subseg_text_p (segT);
+ int    seg_not_empty_p (segT);
  void   start_dependencies (char *);
  void   register_dependency (char *);
  void   print_dependencies (void);
  #ifdef BFD_ASSEMBLER
  segT   subseg_get (const char *, int);
  #endif
! void   seg_change (segT, int);
  
  struct expressionS;
  struct fix;
Index: gas/dwarf2dbg.c
===================================================================
RCS file: /cvs/src/src/gas/dwarf2dbg.c,v
retrieving revision 1.72
diff -c -3 -p -r1.72 dwarf2dbg.c
*** gas/dwarf2dbg.c	22 Nov 2004 16:29:33 -0000	1.72
--- gas/dwarf2dbg.c	24 Jan 2005 16:54:33 -0000
*************** out_debug_info (segT info_seg, segT abbr
*** 1349,1369 ****
    symbol_set_value_now (info_end);
  }
  
  void
  dwarf2_finish (void)
  {
    segT line_seg;
    struct line_seg *s;
  
!   /* We don't need to do anything unless:
!      - Some debug information was recorded via .file/.loc or
!        generated by GAS (--gdwarf2)
!      - or, there is a user-provided .debug_info section which could
!        reference the file table in the .debug_line section we generate
!        below.  */
!   if (all_segs == NULL
!       && (bfd_get_section_by_name (stdoutput, ".debug_info") == NULL
! 	  || files_in_use == 0))
      return;
  
    /* Calculate the size of an address for the target machine.  */
--- 1349,1376 ----
    symbol_set_value_now (info_end);
  }
  
+ /* Finish the dwarf2 debug sections.  We emit .debug.line if there
+    were any .file/.loc directives, or --gdwarf2 was given, or if the
+    file has a non-empty .debug_info section.  If we emit .debug_line,
+    and the .debug_info section is empty, we also emit .debug_info,
+    .debug_aranges and .debug_abbrev.  ALL_SEGS will be non-null if
+    there were any .file/.loc directives, or --gdwarf2 was given and
+    there were any located instructions emitted.  */
+ 
  void
  dwarf2_finish (void)
  {
    segT line_seg;
    struct line_seg *s;
+   segT info_seg;
+   int emit_other_sections = 0;
+ 
+   info_seg = bfd_get_section_by_name (stdoutput, ".debug_info");
+   emit_other_sections = info_seg == NULL || seg_not_empty_p (info_seg) == 0;
  
!   if (!all_segs && emit_other_sections)
!     /* There is no line information and no non-empty .debug_info
!        section.  */
      return;
  
    /* Calculate the size of an address for the target machine.  */
*************** dwarf2_finish (void)
*** 1388,1401 ****
  
    out_debug_line (line_seg);
  
!   /* If this is assembler generated line info, we need .debug_info
!      and .debug_abbrev sections as well.  */
!   if (all_segs != NULL && debug_type == DEBUG_DWARF2)
      {
        segT abbrev_seg;
-       segT info_seg;
        segT aranges_seg;
  
        info_seg = subseg_new (".debug_info", 0);
        abbrev_seg = subseg_new (".debug_abbrev", 0);
        aranges_seg = subseg_new (".debug_aranges", 0);
--- 1395,1410 ----
  
    out_debug_line (line_seg);
  
!   /* If this is assembler generated line info, and there is no
!      debug_info already, we need .debug_info and .debug_abbrev
!      sections as well.  */
!   if (emit_other_sections)
      {
        segT abbrev_seg;
        segT aranges_seg;
  
+       assert (all_segs);
+       
        info_seg = subseg_new (".debug_info", 0);
        abbrev_seg = subseg_new (".debug_abbrev", 0);
        aranges_seg = subseg_new (".debug_aranges", 0);
Index: gas/subsegs.c
===================================================================
RCS file: /cvs/src/src/gas/subsegs.c,v
retrieving revision 1.18
diff -c -3 -p -r1.18 subsegs.c
*** gas/subsegs.c	21 Jan 2005 05:54:38 -0000	1.18
--- gas/subsegs.c	24 Jan 2005 16:54:34 -0000
*************** subseg_text_p (segT sec)
*** 595,600 ****
--- 595,628 ----
  #endif /* ! BFD_ASSEMBLER */
  }
  
+ /* Return non zero if SEC has at least one byte of data.  It is
+    possible that we'll return zero even on a non-empty section because
+    we don't know all the fragment types, and it is possible that an
+    fr_fix == 0 one still contributes data.  Think of this as
+    seg_definitely_not_empty_p.  */
+ 
+ int
+ seg_not_empty_p (segT sec)
+ {
+   segment_info_type *seginfo = seg_info (sec);
+   frchainS *chain;
+   fragS *frag;
+ 
+   if (!seginfo)
+     return 0;
+   
+   for (chain = seginfo->frchainP; chain; chain = chain->frch_next)
+     {
+       for (frag = chain->frch_root; frag; frag = frag->fr_next)
+ 	if (frag->fr_fix)
+ 	  return 1;
+       if (obstack_next_free (&chain->frch_obstack)
+ 	  != chain->frch_last->fr_literal)
+ 	return 1;
+     }
+   return 0;
+ }
+ 
  void
  subsegs_print_statistics (FILE *file)
  {

Re: [patch] gas dwarf2 debug emission

[Alan Modra]

On Tue, Jan 25, 2005 at 09:42:39AM +0000, Nathan Sidwell wrote:
> 	* dwarf2dbg.c (dwarf2_finish): Correct logic for determining when
> 	to emit .debug_line and other debug sections.
> 	* as.h (seg_not_empty_p): Declare.
> 	* subsegs.c (seg_not_empty_p): New predicate.

Hmm.

> Index: gas/as.h
[snip]
> - void   subseg_change (segT, int);
[snip]
> ! void   seg_change (segT, int);

Please don't commit this one.  Looks like some other patch of yours
crept in unannounced here.

> +   emit_other_sections = info_seg == NULL || seg_not_empty_p (info_seg) == 0;

Style issue here.  seg_not_empty_p is obviously a predicate.  It's
preferable to use '!' rather than comparing against zero in this case
(but leave info_seg == NULL test as is).

> + int
> + seg_not_empty_p (segT sec)

And in a similar vein, the return type is better as bfd_boolean.
OK with those changes.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre