[PATCH] fix DWARF for ia64

[PATCH] fix DWARF for ia64

[Bob Wilson]

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

While fixing up some DWARF line number issues for Xtensa, I discovered a problem 
with the ia64 port of GAS.  Basically, the features tested by the lns-common-1 
test are broken but no one noticed because that test doesn't run on ia64.

I've included an ia64-specific variant of the lns-common-1 test that 
demonstrates the problem.  (I'm not at all familiar with ia64 assembly code, so 
I just made this up by looking at some other ia64 tests.)

The cause of the problem is that GAS for ia64 collects the line number 
information, stores it in association with machine instructions, and then later 
calls dwarf2_gen_line_info.  It does not use dwarf2_emit_insn, which is 
responsible for clearing the loc_directive_seen flag and several other flags 
associated with the line information.

The patch fixes this by providing a new function to clear these flags, and by 
using that function in the ia64 port.  I have a patch to make the Xtensa port 
handle line numbers in the same way as ia64, and I would also like to use this 
new function there.

The patch also fixes a minor issue related to clearing the loc_directive_seen 
flag.  That flag is not cleared when debug_type is set to DEBUG_DWARF2.  Daniel 
J. recently added a check for consecutive .loc directives, and I believe that 
check will not cause a lot of extra calls to dwarf2_emit_insn with DEBUG_DWARF2, 
since the flag never gets cleared.

Is this OK?

gas/
	* dwarf2dbg.c (dwarf2_consume_line_info): New.
	(dwarf2_emit_insn): Use it here.
	(dwarf2_directive_loc): Fix check for consecutive .loc directives
	when debug_type is DEBUG_DWARF2.
	* dwarf2dbg.h (dwarf2_consume_line_info): New prototype.
	* config/tc-ia64.c (ia64_flush_insns): Call dwarf2_consume_line_info.
	(md_assemble): Likewise.

gas/testsuite/
	* gas/lns/lns.exp: Run lns-common-1 with alternate source for ia64.
	* gas/lns/lns-common-1-ia64.s: New file.

[-- Attachment #2: gas-debug-ia64.patch --]
[-- Type: text/x-diff, Size: 4782 bytes --]

Index: dwarf2dbg.c
===================================================================
RCS file: /cvs/src/src/gas/dwarf2dbg.c,v
retrieving revision 1.91
diff -u -p -r1.91 dwarf2dbg.c
--- dwarf2dbg.c	29 Aug 2007 20:03:43 -0000	1.91
+++ dwarf2dbg.c	13 Nov 2007 19:32:27 -0000
@@ -373,11 +373,6 @@ dwarf2_emit_insn (int size)
 	or the physical input file name (foo.s) and not the file name
 	specified in the most recent .loc directive (eg foo.h).  */
       loc = current;
-
-      /* Unless we generate DWARF2 debugging information for each
-	 assembler line, we only emit one line symbol for one LOC.  */
-      if (debug_type != DEBUG_DWARF2)
-	loc_directive_seen = FALSE;
     }
   else if (debug_type != DEBUG_DWARF2)
     return;
@@ -385,6 +380,21 @@ dwarf2_emit_insn (int size)
     dwarf2_where (&loc);
 
   dwarf2_gen_line_info (frag_now_fix () - size, &loc);
+  dwarf2_consume_line_info ();
+}
+
+/* Called after the current line information has been either used with
+   dwarf2_gen_line_info or saved with a machine instruction for later use.
+   This resets the state of the line number information to reflect that
+   it has been used.  */
+
+void
+dwarf2_consume_line_info (void)
+{
+  /* Unless we generate DWARF2 debugging information for each
+     assembler line, we only emit one line symbol for one LOC.  */
+  if (debug_type != DEBUG_DWARF2)
+    loc_directive_seen = FALSE;
 
   current.flags &= ~(DWARF2_FLAG_BASIC_BLOCK
 		     | DWARF2_FLAG_PROLOGUE_END
@@ -572,7 +582,7 @@ dwarf2_directive_loc (int dummy ATTRIBUT
 
   /* If we see two .loc directives in a row, force the first one to be
      output now.  */
-  if (loc_directive_seen)
+  if (loc_directive_seen && debug_type != DEBUG_DWARF2)
     dwarf2_emit_insn (0);
 
   filenum = get_absolute_expression ();
Index: dwarf2dbg.h
===================================================================
RCS file: /cvs/src/src/gas/dwarf2dbg.h,v
retrieving revision 1.19
diff -u -p -r1.19 dwarf2dbg.h
--- dwarf2dbg.h	3 Jul 2007 11:01:03 -0000	1.19
+++ dwarf2dbg.h	13 Nov 2007 19:32:27 -0000
@@ -72,6 +72,10 @@ extern void dwarf2_gen_line_info (addres
 /* Must be called for each generated instruction.  */
 extern void dwarf2_emit_insn (int);
 
+/* Reset the state of the line number information to reflect that
+   it has been used.  */
+extern void dwarf2_consume_line_info (void);
+
 /* Should be called for each code label.  */
 extern void dwarf2_emit_label (symbolS *);
 
Index: config/tc-ia64.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.c,v
retrieving revision 1.197
diff -u -p -r1.197 tc-ia64.c
--- config/tc-ia64.c	17 Oct 2007 16:45:55 -0000	1.197
+++ config/tc-ia64.c	13 Nov 2007 19:32:29 -0000
@@ -1131,6 +1131,7 @@ ia64_flush_insns ()
       dwarf2_where (&CURR_SLOT.debug_line);
       CURR_SLOT.debug_line.flags |= DWARF2_FLAG_BASIC_BLOCK;
       dwarf2_gen_line_info (frag_now_fix (), &CURR_SLOT.debug_line);
+      dwarf2_consume_line_info ();
     }
   CURR_SLOT.label_fixups = 0;
 
@@ -10967,6 +10968,7 @@ md_assemble (str)
   CURR_SLOT.idesc = idesc;
   as_where (&CURR_SLOT.src_file, &CURR_SLOT.src_line);
   dwarf2_where (&CURR_SLOT.debug_line);
+  dwarf2_consume_line_info ();
 
   /* Add unwind entries, if there are any.  */
   if (unwind.current_entry)
Index: testsuite/gas/lns/lns.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/lns/lns.exp,v
retrieving revision 1.7
diff -u -p -r1.7 lns.exp
--- testsuite/gas/lns/lns.exp	29 Aug 2007 20:03:43 -0000	1.7
+++ testsuite/gas/lns/lns.exp	13 Nov 2007 19:32:33 -0000
@@ -13,7 +13,6 @@ run_dump_test "lns-duplicate"
 # information (d10v).
 if {
         ![istarget d10v-*-*]
-     && ![istarget ia64*-*-*]
      && ![istarget i370-*-*]
      && ![istarget i960-*-*]
      && ![istarget mcore-*-*]
@@ -23,6 +22,8 @@ if {
     # Use alternate file for targets using DW_LNS_fixed_advance_pc opcodes.
     if { [istarget xtensa-*-*] } {
       run_dump_test "lns-common-1-alt"
+    } elseif { [istarget ia64*-*-*] } {
+      run_dump_test "lns-common-1" { { source "lns-common-1-ia64.s" } }
     } else {
       run_dump_test "lns-common-1"
     }
--- /dev/null	2007-04-16 22:20:02.000000000 -0700
+++ testsuite/gas/lns/lns-common-1-ia64.s	2007-11-13 10:38:21.000000000 -0800
@@ -0,0 +1,16 @@
+	.file 1 "foo.c"
+	.loc 1 1
+	.explicit
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 2 3
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 3 prologue_end
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 4 0 epilogue_begin
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 5 isa 1 basic_block
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 6 is_stmt 0
+	{ .mii; nop 0; nop 0; nop 0 ;; }
+	.loc 1 7 is_stmt 1
+	{ .mii; nop 0; nop 0; nop 0 ;; }

Re: [PATCH] fix DWARF for ia64

[Bob Wilson]

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

Here is a follow-on patch.  I noticed that dwarf2_emit_label() can also use the 
new dwarf2_consume_line_info function.

Is this OK?

I forgot to say how I tested this.  I built binutils for xtensa-elf, 
ia64-unknown-linux-gnu and i686-pc-linux-gnu targets, all hosted on 
i686-pc-linux-gnu, and ran the testsuite.  I've rerun those tests now with this 
follow-on patch.  Everything passed as expected with one exception: the 
ld-ia64/line.exp test fails but I see the same failure without any of my 
changes.  ld fails to show the source line number in an error message.  That 
does seem suspicious.  Is this a known problem?

gas/
	* dwarf2dbg.c (dwarf2_emit_label): Use dwarf2_consume_line_info.


[-- Attachment #2: gas-debug-ia64-2.patch --]
[-- Type: text/x-diff, Size: 540 bytes --]

--- dwarf2dbg.c.2	2007-11-13 12:41:33.000000000 -0800
+++ dwarf2dbg.c	2007-11-13 12:42:42.000000000 -0800
@@ -419,17 +419,11 @@
   if (debug_type == DEBUG_DWARF2)
     dwarf2_where (&loc);
   else
-    {
-      loc = current;
-      dwarf2_loc_directive_seen = FALSE;
-    }
+    loc = current;
 
   loc.flags |= DWARF2_FLAG_BASIC_BLOCK;
 
-  current.flags &= ~(DWARF2_FLAG_BASIC_BLOCK
-		     | DWARF2_FLAG_PROLOGUE_END
-		     | DWARF2_FLAG_EPILOGUE_BEGIN);
-
+  dwarf2_consume_line_info ();
   dwarf2_gen_line_info_1 (label, &loc);
 }
 

Re: [PATCH] fix DWARF for ia64

[Nick Clifton]

Hi Bob,

> Here is a follow-on patch.  I noticed that dwarf2_emit_label() can also 
> use the new dwarf2_consume_line_info function.
> 
> Is this OK?

Your first patch and this follow on one both look OK, apart from the last part 
of this second patch:

      loc.flags |= DWARF2_FLAG_BASIC_BLOCK;

   -  current.flags &= ~(DWARF2_FLAG_BASIC_BLOCK
   -		     | DWARF2_FLAG_PROLOGUE_END
   -		     | DWARF2_FLAG_EPILOGUE_BEGIN);
   -
   +  dwarf2_consume_line_info ();
      dwarf2_gen_line_info_1 (label, &loc);

Why are you deleting the update of current.flags ?


> Everything passed as expected with one 
> exception: the ld-ia64/line.exp test fails but I see the same failure 
> without any of my changes.

Hmm, I do not see that failure.  In fact I get (trimmed slightly for email):

   Running .../ld-ia64/line.exp ...
   .../gas/as-new -o tmpdir/undefined.o -x .../ld-ia64/undefined.s
   ./ld-new -e start -o tmpdir/undefined tmpdir/undefined.o
   ld-new: warning: cannot find entry symbol start; defaulting to 40000000000000b0
   tmpdir/undefined.o: In function `function':
   undefined.c:9: undefined reference to `this_function_is_not_defined'
   ld-new: warning: cannot find entry symbol start; defaulting to 40000000000000b0
   tmpdir/undefined.o: In function `function':
   undefined.c:9: undefined reference to `this_function_is_not_defined'
   ld-new: warning: cannot find entry symbol start; defaulting to 40000000000000b0
   tmpdir/undefined.o: In function `function':
   undefined.c:9: undefined reference to `this_function_is_not_defined'
   PASS: undefined line
   testcase .../ld-ia64/line.exp completed in 0 seconds

> ld fails to show the source line number in 
> an error message.  That does seem suspicious.  Is this a known problem?

Suspicious yes.  Known problem no.  The above test was run for an 
ia64-unknown-linux-gnu toolchain, and I would hope that you would see the same 
results.  Can you investigate a little more please ?

Cheers
   Nick

Re: [PATCH] fix DWARF for ia64

[Bob Wilson]

Nick Clifton wrote:
> Your first patch and this follow on one both look OK, apart from the 
> last part of this second patch:
> 
>      loc.flags |= DWARF2_FLAG_BASIC_BLOCK;
> 
>   -  current.flags &= ~(DWARF2_FLAG_BASIC_BLOCK
>   -             | DWARF2_FLAG_PROLOGUE_END
>   -             | DWARF2_FLAG_EPILOGUE_BEGIN);
>   -
>   +  dwarf2_consume_line_info ();
>      dwarf2_gen_line_info_1 (label, &loc);
> 
> Why are you deleting the update of current.flags ?

It isn't deleted, just moved into the dwarf2_consume_line_info function.


>> Everything passed as expected with one exception: the ld-ia64/line.exp 
>> test fails but I see the same failure without any of my changes.
> 
> Hmm, I do not see that failure.  In fact I get (trimmed slightly for 
> email):
> 
>   Running .../ld-ia64/line.exp ...
>   .../gas/as-new -o tmpdir/undefined.o -x .../ld-ia64/undefined.s
>   ./ld-new -e start -o tmpdir/undefined tmpdir/undefined.o
>   ld-new: warning: cannot find entry symbol start; defaulting to 
> 40000000000000b0
>   tmpdir/undefined.o: In function `function':
>   undefined.c:9: undefined reference to `this_function_is_not_defined'
>   ld-new: warning: cannot find entry symbol start; defaulting to 
> 40000000000000b0
>   tmpdir/undefined.o: In function `function':
>   undefined.c:9: undefined reference to `this_function_is_not_defined'
>   ld-new: warning: cannot find entry symbol start; defaulting to 
> 40000000000000b0
>   tmpdir/undefined.o: In function `function':
>   undefined.c:9: undefined reference to `this_function_is_not_defined'
>   PASS: undefined line
>   testcase .../ld-ia64/line.exp completed in 0 seconds
> 
>> ld fails to show the source line number in an error message.  That 
>> does seem suspicious.  Is this a known problem?
> 
> Suspicious yes.  Known problem no.  The above test was run for an 
> ia64-unknown-linux-gnu toolchain, and I would hope that you would see 
> the same results.  Can you investigate a little more please ?

It definitely not related to my change.  I have again verified that it happens 
without any of my changes.  I suspect you're using a 64-bit host, since the 
problem appears to be related to 32-bit hosts.

Here is what I get:

% ia64-unknown-linux-gnu-ld -e start -o undefined undefined.o
ia64-unknown-linux-gnu-ld: warning: cannot find entry symbol start; defaulting 
to 40000000000000b0
ia64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.18.50.20071116 assertion fail 
../../src/bfd/arange-set.c:667
undefined.o: In function `function':
undefined.c:(.text+0x12): undefined reference to `this_function_is_not_defined'

Notice the assertion failure.  The following stack trace shows the problem:

#0  splay_tree_insert (sp=0x81814e0, key=176, value=239)
     at ../../src/libiberty/splay-tree.c:299
#1  0x08093118 in arange_set_splay_tree_insert (set=0x81814ac,
     low=4611686018427388080, high=4611686018427388143, value=0)
     at ../../src/bfd/arange-set.c:455
#2  0x08093aff in arange_set_insert (set=0x81814ac, low=4611686018427388080,
     high=4611686018427388143, value=0) at ../../src/bfd/arange-set.c:660
#3  0x0808db4d in dwarf2_comp_unit_arange_add (unit=0x8181458,
     low=4611686018427388080, high=4611686018427388144)
     at ../../src/bfd/dwarf2.c:1014
#4  0x080908e5 in parse_comp_unit (stash=0x8167cc8, unit_length=76,
     info_ptr_unit=0x8181104 "L", offset_size=4) at ../../src/bfd/dwarf2.c:2353
#5  0x080920bb in find_line (abfd=0x815fb80, section=0x8161e34, offset=18,
     symbol=0x0, symbols=0x8161738, filename_ptr=0xbfc6e314,
     functionname_ptr=0xbfc6e310, linenumber_ptr=0xbfc6e30c, addr_size=4,
     pinfo=0x815ff9c) at ../../src/bfd/dwarf2.c:3303
#6  0x0809237d in _bfd_dwarf2_find_nearest_line (abfd=0x815fb80,
     section=0x8161e34, symbols=0x8161738, offset=18, filename_ptr=0xbfc6e314,
     functionname_ptr=0xbfc6e310, linenumber_ptr=0xbfc6e30c, addr_size=0,
     pinfo=0x815ff9c) at ../../src/bfd/dwarf2.c:3372
#7  0x080b5280 in _bfd_elf_find_nearest_line (abfd=0x815fb80,
     section=0x8161e34, symbols=0x8161738, offset=18, filename_ptr=0xbfc6e314,
     functionname_ptr=0xbfc6e310, line_ptr=0xbfc6e30c)
     at ../../src/bfd/elf.c:7033
#8  0x08065b48 in vfinfo (fp=0x40168560,
     fmt=0x810f30c ": undefined reference to `%T'\n",
     arg=0xbfc6e424 "?\026\b", is_warning=1) at ../../src/ld/ldmisc.c:323
#9  0x08066031 in einfo (fmt=0x810f308 "%X%C: undefined reference to `%T'\n")
     at ../../src/ld/ldmisc.c:465
#10 0x08060b3e in undefined_symbol (info=0x8147940,
     name=0x81676f2 "this_function_is_not_defined", abfd=0x815fb80,
     section=0x8161e34, address=18, error=1) at ../../src/ld/ldmain.c:1362

When arange_set_splay_tree_insert calls splay_tree_insert, the 64-bit "low" and 
"high" values are truncated to 32-bit "key" and "value".

--Bob

Re: [PATCH] fix DWARF for ia64

[Nick Clifton]

Hi Bob,

>> Why are you deleting the update of current.flags ?
> 
> It isn't deleted, just moved into the dwarf2_consume_line_info function.

Oops, I missed that.

OK the patch is approved - please apply.

> It definitely not related to my change.  I have again verified that it 
> happens without any of my changes.  I suspect you're using a 64-bit 
> host,

I am.

> since the problem appears to be related to 32-bit hosts.
> ia64-unknown-linux-gnu-ld: BFD (GNU Binutils) 2.18.50.20071116 assertion 
> fail ../../src/bfd/arange-set.c:667

Ah - this is the splay tree bug that HJ is working on.  Sorry for making you 
chase this one down.

Cheers
   Nick