This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] MIPS/GAS: Fix DWARF-2 with branch swapping for MIPS16 code


On Wed, 29 Jun 2011, Richard Sandiford wrote:

> "Maciej W. Rozycki" <macro@codesourcery.com> writes:
> > 2011-02-23  Maciej W. Rozycki  <macro@codesourcery.com>
> >
> > 	gas/
> > 	* config/tc-mips.c (append_insn): Make sure DWARF-2 location
> > 	information is properly adjusted for branches that get swapped.
> >
> > 	gas/testsuite/
> > 	* gas/mips/loc-swap.d: New test case for DWARF-2 location with
> > 	branch swapping.
> > 	* gas/mips/loc-swap-dis.d: Likewise.
> > 	* gas/mips/mips16\@loc-swap.d: Likewise, MIPS16 version.
> > 	* gas/mips/mips16\@loc-swap-dis.d: Likewise.
> > 	* gas/mips/loc-swap.s: Source for the new tests.
> > 	* gas/mips/mips.exp: Run the new tests.
> 
> Sorry, I'd somehow got the idea that this patch applied on top of
> the microMIPS changes.  I now realise that it's the other way around.

 Well, I've got some remains of sanity still in place. ;)

 Speaking of which, I plan to get back to the microMIPS changes, well... 
sometime.  Hopefully this month.  Sorry about the delays.

> I've updated the patch after the changes I made today, as below.
> I think this is OK to commit.  However:
> 
> - loc-swap-dis.d was missing from the patch

 Oops, sorry about that.  I sent an outdated version it would seem as my 
current one did have this file included.

> - this:
> 
> > +  Advance PC by 16 to 0x5c
> 
> depends on the target alignment of .text; it passes for GNU/Linux
> targets but fails on ELF ones.  I think we should stub out the
> exact numbers, since they're incidental to the test.

 Hmm, does it?  I did verify it both on a GNU/Linux and an ELF target.  It 
still passes for me, except that:

+  Advance PC by 24 to 0x64

is required instead as with my intended-to-be-sent-originally change.  
Likewise:

+  Advance PC by 23 to 0x40

for the MIPS16 dump.  Which target is failing for you?  Perhaps the cause 
was merely the outdated version of the change, sigh...

> I've included the second change in the patch.  If you want to
> apply it along with loc-swap-dis.d, please feel free.  Otherwise
> send loc-swap-dis.d to me and I'll test and commit it.

 I have regenerated and revalidated the change now and I'm ready to push 
it, but I'd prefer the final DWARF-2 instructions to be matched exactly.  
I made the effort to make the source alignment-agnostic and I'd prefer to 
keep the final offsets to be verified literally so that any unexpected 
discrepancies are caught.

 Here's a version of the change I'd like to push.  If that still fails on 
any of your pet targets, then I'll be happy to reintroduce the wildcard 
match.  Otherwise I'll insist on keeping it as is.

 And BTW, thanks a lot for the register mask, etc. rewrites!  They make 
this piece of code much easier to follow.

2011-07-02  Maciej W. Rozycki  <macro@codesourcery.com>

	gas/
	* config/tc-mips.c (append_insn): Make sure DWARF-2 location
	information is properly adjusted for branches that get swapped.

	gas/testsuite/
	* gas/mips/loc-swap.d: New test case for DWARF-2 location with
	branch swapping.
	* gas/mips/loc-swap-dis.d: Likewise.
	* gas/mips/mips16\@loc-swap.d: Likewise, MIPS16 version.
	* gas/mips/mips16\@loc-swap-dis.d: Likewise.
	* gas/mips/loc-swap.s: Source for the new tests.
	* gas/mips/mips.exp: Run the new tests.

  Maciej

binutils-gas-mips-branch-swap-dwarf2.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c	2011-07-02 00:40:03.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c	2011-07-02 00:53:52.000000000 +0100
@@ -3454,10 +3454,20 @@ append_insn (struct mips_cl_insn *ip, ex
 #ifdef OBJ_ELF
   /* The value passed to dwarf2_emit_insn is the distance between
      the beginning of the current instruction and the address that
-     should be recorded in the debug tables.  For MIPS16 debug info
-     we want to use ISA-encoded addresses, so we pass -1 for an
-     address higher by one than the current.  */
-  dwarf2_emit_insn (mips_opts.mips16 ? -1 : 0);
+     should be recorded in the debug tables.  This is normally the
+     current address.
+
+     For MIPS16 debug info we want to use ISA-encoded addresses,
+     so we use -1 for an address higher by one than the current one.
+
+     If the instruction produced is a branch that we will swap with
+     the preceding instruction, then we add the displacement by which
+     the branch will be moved backwards.  This is more appropriate
+     and for MIPS16 code also prevents a debugger from placing a
+     breakpoint in the middle of the branch (and corrupting code if
+     software breakpoints are used).  */
+  dwarf2_emit_insn ((mips_opts.mips16 ? -1 : 0)
+		    + (method == APPEND_SWAP ? insn_length (history) : 0));
 #endif
 
   if (address_expr
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap.d	2011-07-02 00:53:52.000000000 +0100
@@ -0,0 +1,61 @@
+#PROG: readelf
+#readelf: -wl
+#name: MIPS DWARF-2 location information with branch swapping
+#as: -32
+#source: loc-swap.s
+
+# Verify that DWARF-2 location information for instructions reordered
+# into a branch delay slot is updated to point to the branch instead.
+
+Raw dump of debug contents of section \.debug_line:
+
+  Offset:                      0x0
+  Length:                      67
+  DWARF Version:               2
+  Prologue Length:             33
+  Minimum Instruction Length:  1
+  Initial value of 'is_stmt':  1
+  Line Base:                   -5
+  Line Range:                  14
+  Opcode Base:                 13
+
+ Opcodes:
+  Opcode 1 has 0 args
+  Opcode 2 has 1 args
+  Opcode 3 has 1 args
+  Opcode 4 has 1 args
+  Opcode 5 has 1 args
+  Opcode 6 has 0 args
+  Opcode 7 has 0 args
+  Opcode 8 has 0 args
+  Opcode 9 has 1 args
+  Opcode 10 has 0 args
+  Opcode 11 has 0 args
+  Opcode 12 has 1 args
+
+ The Directory Table is empty\.
+
+ The File Name Table:
+  Entry	Dir	Time	Size	Name
+  1	0	0	0	loc-swap\.s
+
+ Line Number Statements:
+  Extended opcode 2: set Address to 0x0
+  Special opcode 11: advance Address by 0 to 0x0 and Line by 6 to 7
+  Special opcode 63: advance Address by 4 to 0x4 and Line by 2 to 9
+  Special opcode 120: advance Address by 8 to 0xc and Line by 3 to 12
+  Special opcode 7: advance Address by 0 to 0xc and Line by 2 to 14
+  Special opcode 120: advance Address by 8 to 0x14 and Line by 3 to 17
+  Special opcode 7: advance Address by 0 to 0x14 and Line by 2 to 19
+  Special opcode 120: advance Address by 8 to 0x1c and Line by 3 to 22
+  Special opcode 63: advance Address by 4 to 0x20 and Line by 2 to 24
+  Special opcode 120: advance Address by 8 to 0x28 and Line by 3 to 27
+  Special opcode 63: advance Address by 4 to 0x2c and Line by 2 to 29
+  Special opcode 120: advance Address by 8 to 0x34 and Line by 3 to 32
+  Special opcode 63: advance Address by 4 to 0x38 and Line by 2 to 34
+  Special opcode 120: advance Address by 8 to 0x40 and Line by 3 to 37
+  Special opcode 7: advance Address by 0 to 0x40 and Line by 2 to 39
+  Special opcode 120: advance Address by 8 to 0x48 and Line by 3 to 42
+  Special opcode 63: advance Address by 4 to 0x4c and Line by 2 to 44
+  Advance PC by 24 to 0x64
+  Extended opcode 1: End of Sequence
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap.s
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap.s	2011-07-02 00:53:52.000000000 +0100
@@ -0,0 +1,48 @@
+# Source file to test DWARF-2 location information with branch swapping.
+
+	.file	1 "loc-swap.s"
+	.text
+foo:
+	.loc	1 7
+	move	$4, $16
+	.loc	1 9
+	jr	$4
+
+	.loc	1 12
+	move	$31, $16
+	.loc	1 14
+	jr	$4
+
+	.loc	1 17
+	move	$4, $16
+	.loc	1 19
+	jr	$31
+
+	.loc	1 22
+	move	$31, $16
+	.loc	1 24
+	jr	$31
+
+	.loc	1 27
+	move	$4, $16
+	.loc	1 29
+	jalr	$4
+
+	.loc	1 32
+	move	$31, $16
+	.loc	1 34
+	jalr	$4
+
+	.loc	1 37
+	move	$4, $16
+	.loc	1 39
+	jal	bar
+
+	.loc	1 42
+	move	$31, $16
+	.loc	1 44
+	jal	bar
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+	.align	2
+	.space	16
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips.exp	2011-07-02 00:52:11.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp	2011-07-02 00:53:52.000000000 +0100
@@ -854,6 +854,10 @@ if { [istarget mips*-*-vxworks*] } {
 					[mips_arch_list_matching mips1]
 	run_dump_test_arches "branch-misc-4-64" \
 					[mips_arch_list_matching mips3]
+
+	run_dump_test_arches "loc-swap"	[mips_arch_list_all]
+	run_dump_test_arches "loc-swap-dis" \
+					[mips_arch_list_all]
     }
 
     if $has_newabi {
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@loc-swap-dis.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@loc-swap-dis.d	2011-07-02 00:53:52.000000000 +0100
@@ -0,0 +1,35 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS DWARF-2 location information with branch swapping disassembly
+#as: -32
+#source: loc-swap.s
+
+# Check branch swapping with DWARF-2 location information (MIPS16).
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 6790      	move	a0,s0
+[0-9a-f]+ <[^>]*> ec00      	jr	a0
+[0-9a-f]+ <[^>]*> 6500      	nop
+[0-9a-f]+ <[^>]*> ec00      	jr	a0
+[0-9a-f]+ <[^>]*> 65f8      	move	ra,s0
+[0-9a-f]+ <[^>]*> e820      	jr	ra
+[0-9a-f]+ <[^>]*> 6790      	move	a0,s0
+[0-9a-f]+ <[^>]*> 65f8      	move	ra,s0
+[0-9a-f]+ <[^>]*> e820      	jr	ra
+[0-9a-f]+ <[^>]*> 6500      	nop
+[0-9a-f]+ <[^>]*> 6790      	move	a0,s0
+[0-9a-f]+ <[^>]*> ec40      	jalr	a0
+[0-9a-f]+ <[^>]*> 6500      	nop
+[0-9a-f]+ <[^>]*> 65f8      	move	ra,s0
+[0-9a-f]+ <[^>]*> ec40      	jalr	a0
+[0-9a-f]+ <[^>]*> 6500      	nop
+[0-9a-f]+ <[^>]*> 1800 0000 	jal	0+0000 <foo>
+[ 	]*[0-9a-f]+: R_MIPS16_26	bar
+[0-9a-f]+ <[^>]*> 6790      	move	a0,s0
+[0-9a-f]+ <[^>]*> 65f8      	move	ra,s0
+[0-9a-f]+ <[^>]*> 1800 0000 	jal	0+0000 <foo>
+[ 	]*[0-9a-f]+: R_MIPS16_26	bar
+[0-9a-f]+ <[^>]*> 6500      	nop
+[0-9a-f]+ <[^>]*> 6500      	nop
+	\.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@loc-swap.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@loc-swap.d	2011-07-02 00:53:52.000000000 +0100
@@ -0,0 +1,61 @@
+#PROG: readelf
+#readelf: -wl
+#name: MIPS DWARF-2 location information with branch swapping
+#as: -32
+#source: loc-swap.s
+
+# Verify that DWARF-2 location information for instructions reordered
+# into a branch delay slot is updated to point to the branch instead.
+
+Raw dump of debug contents of section \.debug_line:
+
+  Offset:                      0x0
+  Length:                      67
+  DWARF Version:               2
+  Prologue Length:             33
+  Minimum Instruction Length:  1
+  Initial value of 'is_stmt':  1
+  Line Base:                   -5
+  Line Range:                  14
+  Opcode Base:                 13
+
+ Opcodes:
+  Opcode 1 has 0 args
+  Opcode 2 has 1 args
+  Opcode 3 has 1 args
+  Opcode 4 has 1 args
+  Opcode 5 has 1 args
+  Opcode 6 has 0 args
+  Opcode 7 has 0 args
+  Opcode 8 has 0 args
+  Opcode 9 has 1 args
+  Opcode 10 has 0 args
+  Opcode 11 has 0 args
+  Opcode 12 has 1 args
+
+ The Directory Table is empty\.
+
+ The File Name Table:
+  Entry	Dir	Time	Size	Name
+  1	0	0	0	loc-swap\.s
+
+ Line Number Statements:
+  Extended opcode 2: set Address to 0x1
+  Special opcode 11: advance Address by 0 to 0x1 and Line by 6 to 7
+  Special opcode 35: advance Address by 2 to 0x3 and Line by 2 to 9
+  Special opcode 64: advance Address by 4 to 0x7 and Line by 3 to 12
+  Special opcode 7: advance Address by 0 to 0x7 and Line by 2 to 14
+  Special opcode 64: advance Address by 4 to 0xb and Line by 3 to 17
+  Special opcode 7: advance Address by 0 to 0xb and Line by 2 to 19
+  Special opcode 64: advance Address by 4 to 0xf and Line by 3 to 22
+  Special opcode 35: advance Address by 2 to 0x11 and Line by 2 to 24
+  Special opcode 64: advance Address by 4 to 0x15 and Line by 3 to 27
+  Special opcode 35: advance Address by 2 to 0x17 and Line by 2 to 29
+  Special opcode 64: advance Address by 4 to 0x1b and Line by 3 to 32
+  Special opcode 35: advance Address by 2 to 0x1d and Line by 2 to 34
+  Special opcode 64: advance Address by 4 to 0x21 and Line by 3 to 37
+  Special opcode 7: advance Address by 0 to 0x21 and Line by 2 to 39
+  Special opcode 92: advance Address by 6 to 0x27 and Line by 3 to 42
+  Special opcode 35: advance Address by 2 to 0x29 and Line by 2 to 44
+  Advance PC by 23 to 0x40
+  Extended opcode 1: End of Sequence
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap-dis.d
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/loc-swap-dis.d	2011-07-02 00:56:31.000000000 +0100
@@ -0,0 +1,34 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS DWARF-2 location information with branch swapping disassembly
+#as: -32
+#source: loc-swap.s
+
+# Check branch swapping with DWARF-2 location information.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 02002021 	move	a0,s0
+[0-9a-f]+ <[^>]*> 00800008 	jr	a0
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 00800008 	jr	a0
+[0-9a-f]+ <[^>]*> 0200f821 	move	ra,s0
+[0-9a-f]+ <[^>]*> 03e00008 	jr	ra
+[0-9a-f]+ <[^>]*> 02002021 	move	a0,s0
+[0-9a-f]+ <[^>]*> 0200f821 	move	ra,s0
+[0-9a-f]+ <[^>]*> 03e00008 	jr	ra
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 02002021 	move	a0,s0
+[0-9a-f]+ <[^>]*> 0080f809 	jalr	a0
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 0200f821 	move	ra,s0
+[0-9a-f]+ <[^>]*> 0080f809 	jalr	a0
+[0-9a-f]+ <[^>]*> 00000000 	nop
+[0-9a-f]+ <[^>]*> 0c000000 	jal	0+0000 <foo>
+[ 	]*[0-9a-f]+: R_MIPS_26	bar
+[0-9a-f]+ <[^>]*> 02002021 	move	a0,s0
+[0-9a-f]+ <[^>]*> 0200f821 	move	ra,s0
+[0-9a-f]+ <[^>]*> 0c000000 	jal	0+0000 <foo>
+[ 	]*[0-9a-f]+: R_MIPS_26	bar
+[0-9a-f]+ <[^>]*> 00000000 	nop
+	\.\.\.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]