[RFA/dwarf2] Small problem scanning line table for included files

[RFA/dwarf2] Small problem scanning line table for included files

[Joel Brobecker]

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

Hello,

I noticed that gdb.base/sep.exp fails with GCC 3.4. The attached
patch fixes it (the comment should explain how).

For the record, here is the line table generated by 3.4 for sep.c:

> The File Name Table:
>  Entry Dir     Time    Size    Name
>  1     0       0       0       sep-proc.c
>  2     0       0       0       sep.c
>
> Line Number Statements:
>  Extended opcode 2: set Address to 0x0
>  Advance Line by 23 to 24
>  Copy
>  Special opcode 90: advance Address by 6 to 0x6 and Line by 1 to 25
>  Special opcode 174: advance Address by 12 to 0x12 and Line by 1 to 26
>  Set File Name to entry 2 in the File Name Table
>  Special opcode 38: advance Address by 2 to 0x14 and Line by 5 to 31
>  Advance PC by constant 17 to 0x25
>  Special opcode 160: advance Address by 11 to 0x30 and Line by 1 to 32
>  Special opcode 76: advance Address by 5 to 0x35 and Line by 1 to 33
>  Special opcode 76: advance Address by 5 to 0x3a and Line by 1 to 34
>  Advance PC by 2 to 3c
>  Extended opcode 1: End of Sequence

And here is the line table generated by 3.2:

> The File Name Table:
>  Entry Dir     Time    Size    Name
>  1     0       0       0       sep.c
>  2     1       0       0       types.h
>  3     2       0       0       wchar.h
>  4     3       0       0       stddef.h
>  5     2       0       0       _G_config.h
>  6     2       0       0       gconv.h
>  7     2       0       0       libio.h
>  8     3       0       0       stdio.h
>  9     0       0       0       sep-proc.c
>  10    3       0       0       stdarg.h
>
> Line Number Statements:
>  Set File Name to entry 9 in the File Name Table
>  Extended opcode 2: set Address to 0x0
>  Advance Line by 23 to 24
>  Copy
>  Special opcode 90: advance Address by 6 to 0x6 and Line by 1 to 25
>  Special opcode 174: advance Address by 12 to 0x12 and Line by 1 to 26
>  Set File Name to entry 1 in the File Name Table
>  Special opcode 38: advance Address by 2 to 0x14 and Line by 5 to 31
>  Special opcode 230: advance Address by 16 to 0x24 and Line by 1 to 32
>  Special opcode 76: advance Address by 5 to 0x29 and Line by 1 to 33
>  Special opcode 76: advance Address by 5 to 0x2e and Line by 1 to 34
>  Advance PC by 2 to 30
>  Extended opcode 1: End of Sequence

The new line info with GCC 3.4 is much better, but tricks current GDB.

2004-07-25  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf_decode_lines): Handle the case when a set_file
        instruction is not emitted for the first file, as it is implicit
        at the begining of the line number program.
        Fixes [D721-012].

Tested on x86-linux, fixes the 2 regressions in sep.exp.
OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: dwarf2read.c.diff --]
[-- Type: text/plain, Size: 4251 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.156
diff -u -p -r1.156 dwarf2read.c
--- dwarf2read.c	6 Jul 2004 19:29:30 -0000	1.156
+++ dwarf2read.c	25 Jul 2004 15:02:45 -0000
@@ -5926,6 +5926,18 @@ dwarf_decode_lines (struct line_header *
   CORE_ADDR baseaddr;
   struct objfile *objfile = cu->objfile;
   const int decode_for_pst_p = (pst != NULL);
+  
+  /* Normally, we detect the list of files that are really included by
+     scanning the LNP for DW_LNS_set_file.  However, the initial value
+     for the file register is 1, which means that the program is allowed
+     to omit the set_file instruction at the begining if the first
+     instructions refer to that file.  In order to detect that case,
+     we use this FIRST_SET_FILE_FOUND boolean that will be set as soon
+     as we encounter the a set_file instruction.  If, in the meantime,
+     we find any instruction that does change the address, line or
+     column registers, then it means that file 1 was indeed included.  */
+  int first_set_file_found = 0;
+  int file_one_included_p = 0;
 
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
@@ -5977,6 +5989,8 @@ dwarf_decode_lines (struct line_header *
 	          record_line (current_subfile, line, 
 	                       check_cu_functions (address, cu));
                 }
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      basic_block = 1;
 	    }
 	  else switch (op_code)
@@ -5997,6 +6011,8 @@ dwarf_decode_lines (struct line_header *
 		  address = read_address (abfd, line_ptr, cu, &bytes_read);
 		  line_ptr += bytes_read;
 		  address += baseaddr;
+                 if (!first_set_file_found)
+                   file_one_included_p = 1;
 		  break;
 		case DW_LNE_define_file:
                   {
@@ -6028,15 +6044,21 @@ dwarf_decode_lines (struct line_header *
 	        record_line (current_subfile, line, 
 	                     check_cu_functions (address, cu));
 	      basic_block = 0;
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    case DW_LNS_advance_pc:
 	      address += lh->minimum_instruction_length
 		* read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 	      line_ptr += bytes_read;
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    case DW_LNS_advance_line:
 	      line += read_signed_leb128 (abfd, line_ptr, &bytes_read);
 	      line_ptr += bytes_read;
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    case DW_LNS_set_file:
               {
@@ -6055,11 +6077,14 @@ dwarf_decode_lines (struct line_header *
                   dir = comp_dir;
                 if (!decode_for_pst_p)
                   dwarf2_start_subfile (fe->name, dir);
+                first_set_file_found = 1;
               }
 	      break;
 	    case DW_LNS_set_column:
 	      column = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
 	      line_ptr += bytes_read;
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    case DW_LNS_negate_stmt:
 	      is_stmt = (!is_stmt);
@@ -6075,10 +6100,14 @@ dwarf_decode_lines (struct line_header *
 	    case DW_LNS_const_add_pc:
 	      address += (lh->minimum_instruction_length
 			  * ((255 - lh->opcode_base) / lh->line_range));
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    case DW_LNS_fixed_advance_pc:
 	      address += read_2_bytes (abfd, line_ptr);
 	      line_ptr += 2;
+             if (!first_set_file_found)
+               file_one_included_p = 1;
 	      break;
 	    default:
 	      {  /* Unknown standard opcode, ignore it.  */
@@ -6097,6 +6126,9 @@ dwarf_decode_lines (struct line_header *
     {
       int file_index;
 
+      if (file_one_included_p)
+        lh->file_names[0].included_p = 1;
+
       /* Now that we're done scanning the Line Header Program, we can
          create the psymtab of each included file.  */
       for (file_index = 0; file_index < lh->num_file_names; file_index++)

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Michael Chastain]

Now that I look, I see this happening in my test bed, too.
gdb HEAD suite HEAD -gdwarf-2 has PASS with gcc 3.3.4 and
FAIL with gcc 3.4.1.

I had better take another look at my gcc 3.3.4 versus gcc 3.4.1
tables and see if I overlooked anything else in there!

  http://www.shout.net/~mec/sunday/current/Compare-by-gcc-334-341.html

The regressions I know about are:

  gcc 3.4.1 versus gcc 3.3.4
    gcc/12267 gcc/13708 gcc/13956
    gdb/1537 gdb/1540

Michael C

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Jim Blandy]

This is getting a bit hairy.  It would be simpler to simply delete the
"!decode_for_pst_p && " towards the top of dwarf2_decode_lines, and
always create a psymtab for the first file.  This could conceivably
result in creating psymtabs for source files that have no line number
entries in them, but that's probably not going to be a big deal.

(To be picky, we can make that mistake now anyway: consider what would
happen if the line number program contained two consecutive
DW_LNS_set_file instructions: the first file gets a psymtab, even
though it's never used.)

To get everything exactly right, we would need to delay setting a file
table entry's included_p flag until just before we actually call
record_line.

What do you think?

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Joel Brobecker]

Hello Jim,

Thanks for your careful review.

> This is getting a bit hairy.

I can't agree more. I was just fixated on having an exact fix, as
opposed to an approximate fix where we assume that the compiler is
not going to do anything meaningless.

> It would be simpler to simply delete the "!decode_for_pst_p && "
> towards the top of dwarf2_decode_lines, and always create a psymtab
> for the first file.  This could conceivably result in creating
> psymtabs for source files that have no line number entries in them,
> but that's probably not going to be a big deal.
> 
> (To be picky, we can make that mistake now anyway: consider what would
> happen if the line number program contained two consecutive
> DW_LNS_set_file instructions: the first file gets a psymtab, even
> though it's never used.)
> 
> To get everything exactly right, we would need to delay setting a file
> table entry's included_p flag until just before we actually call
> record_line.

I think you are right. Looking for DW_LNS_set_file was a mistake, since
it is not fullproof as you demonstrated above. I looked at the dwarf3
(draft7) reference again, and setting that flag everytime we need to
record a line seems to be the perfect place to do (it's a bit
complicated to explain my resoning, I was concerned mostly because
I was a bit fuzzy on certain details which were cleared by rereding
the dwarf3 reference document).

I shall give this a try, and report back.

-- 
Joel

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Joel Brobecker]

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

Hello Jim,

> I think you are right. Looking for DW_LNS_set_file was a mistake, since
> it is not fullproof as you demonstrated above. I looked at the dwarf3
> (draft7) reference again, and setting that flag everytime we need to
> record a line seems to be the perfect place to do (it's a bit
> complicated to explain my resoning, I was concerned mostly because
> I was a bit fuzzy on certain details which were cleared by rereding
> the dwarf3 reference document).
> 
> I shall give this a try, and report back.

This worked beautifully, as expected. Here is a new patch, much simpler.

2004-07-30  Joel Brobecker  <brobecker@gnat.com>

        * dwarf2read.c (dwarf_decode_lines): Do not consider the current
        file as included until we record the first line in the linetable.

Tested on x86-linux with a GCC-3.4 based compiler, fixes the following
two regressions:

        sep.exp: list using location inside included file
        sep.exp: breakpoint inside included file

I also verified that this test still passes with GCC 3.2.
OK to commit?

Thanks,
-- 
Joel

[-- Attachment #2: dwarf2read.c.diff --]
[-- Type: text/plain, Size: 1643 bytes --]

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.157
diff -u -p -r1.157 dwarf2read.c
--- dwarf2read.c	30 Jul 2004 12:22:27 -0000	1.157
+++ dwarf2read.c	30 Jul 2004 21:12:10 -0000
@@ -5977,6 +5977,7 @@ dwarf_decode_lines (struct line_header *
 	      address += (adj_opcode / lh->line_range)
 		* lh->minimum_instruction_length;
 	      line += lh->line_base + (adj_opcode % lh->line_range);
+              lh->file_names[file - 1].included_p = 1;
               if (!decode_for_pst_p)
                 {
 	          /* append row to matrix using current values */
@@ -5996,6 +5997,7 @@ dwarf_decode_lines (struct line_header *
 		{
 		case DW_LNE_end_sequence:
 		  end_sequence = 1;
+                  lh->file_names[file - 1].included_p = 1;
                   if (!decode_for_pst_p)
 		    record_line (current_subfile, 0, address);
 		  break;
@@ -6030,6 +6032,7 @@ dwarf_decode_lines (struct line_header *
 		}
 	      break;
 	    case DW_LNS_copy:
+              lh->file_names[file - 1].included_p = 1;
               if (!decode_for_pst_p)
 	        record_line (current_subfile, line, 
 	                     check_cu_functions (address, cu));
@@ -6054,7 +6057,6 @@ dwarf_decode_lines (struct line_header *
                 file = read_unsigned_leb128 (abfd, line_ptr, &bytes_read);
                 line_ptr += bytes_read;
                 fe = &lh->file_names[file - 1];
-                fe->included_p = 1;
                 if (fe->dir_index)
                   dir = lh->include_dirs[fe->dir_index - 1];
                 else

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Jim Blandy]

Joel Brobecker <brobecker@gnat.com> writes:
> > I think you are right. Looking for DW_LNS_set_file was a mistake, since
> > it is not fullproof as you demonstrated above. I looked at the dwarf3
> > (draft7) reference again, and setting that flag everytime we need to
> > record a line seems to be the perfect place to do (it's a bit
> > complicated to explain my resoning, I was concerned mostly because
> > I was a bit fuzzy on certain details which were cleared by rereding
> > the dwarf3 reference document).
> > 
> > I shall give this a try, and report back.
> 
> This worked beautifully, as expected. Here is a new patch, much simpler.
> 
> 2004-07-30  Joel Brobecker  <brobecker@gnat.com>
> 
>         * dwarf2read.c (dwarf_decode_lines): Do not consider the current
>         file as included until we record the first line in the linetable.
> 
> Tested on x86-linux with a GCC-3.4 based compiler, fixes the following
> two regressions:
> 
>         sep.exp: list using location inside included file
>         sep.exp: breakpoint inside included file
> 
> I also verified that this test still passes with GCC 3.2.
> OK to commit?

Yes, please do.  Thanks!

Re: [RFA/dwarf2] Small problem scanning line table for included files

[Joel Brobecker]

> > 2004-07-30  Joel Brobecker  <brobecker@gnat.com>
> > 
> >         * dwarf2read.c (dwarf_decode_lines): Do not consider the current
> >         file as included until we record the first line in the linetable.
> > 
> 
> Yes, please do.  Thanks!

Thank you! Checked in.

-- 
Joel