From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22709 invoked by alias); 29 Jun 2011 21:26:29 -0000 Received: (qmail 22699 invoked by uid 22791); 29 Jun 2011 21:26:27 -0000 X-SWARE-Spam-Status: No, hits=-2.3 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-ww0-f43.google.com (HELO mail-ww0-f43.google.com) (74.125.82.43) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 29 Jun 2011 21:26:12 +0000 Received: by wwi18 with SMTP id 18so1404361wwi.12 for ; Wed, 29 Jun 2011 14:26:11 -0700 (PDT) Received: by 10.227.195.209 with SMTP id ed17mr1153775wbb.13.1309382770825; Wed, 29 Jun 2011 14:26:10 -0700 (PDT) Received: from localhost (rsandifo.gotadsl.co.uk [82.133.89.107]) by mx.google.com with ESMTPS id d19sm1179011wbh.42.2011.06.29.14.26.08 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 29 Jun 2011 14:26:09 -0700 (PDT) From: Richard Sandiford To: "Maciej W. Rozycki" Mail-Followup-To: "Maciej W. Rozycki" ,binutils@sourceware.org, rdsandiford@googlemail.com Cc: binutils@sourceware.org Subject: Re: [PATCH] MIPS/GAS: Fix DWARF-2 with branch swapping for MIPS16 code References: Date: Wed, 29 Jun 2011 21:26:00 -0000 In-Reply-To: (Maciej W. Rozycki's message of "Wed, 23 Feb 2011 13:38:39 +0000 (GMT)") Message-ID: <87boxg5nyo.fsf@firetop.home> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2011-06/txt/msg00374.txt.bz2 "Maciej W. Rozycki" writes: > 2011-02-23 Maciej W. Rozycki > > 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. 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 - 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. 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. Richard gas/ 2011-06-29 Maciej W. Rozycki * config/tc-mips.c (append_insn: Make sure DWARF-2 location information is properly adjusted for branches that get swapped. gas/testsuite/ 2011-06-29 Maciej W. Rozycki * 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. Index: gas/config/tc-mips.c =================================================================== --- gas/config/tc-mips.c 2011-06-29 22:05:03.000000000 +0100 +++ gas/config/tc-mips.c 2011-06-29 22:06:15.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: gas/testsuite/gas/mips/loc-swap.d =================================================================== --- /dev/null 2011-06-29 20:49:09.402506167 +0100 +++ gas/testsuite/gas/mips/loc-swap.d 2011-06-29 22:16:56.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 .* to 0x.* + Extended opcode 1: End of Sequence Index: gas/testsuite/gas/mips/mips16@loc-swap.d =================================================================== --- /dev/null 2011-06-29 20:49:09.402506167 +0100 +++ gas/testsuite/gas/mips/mips16@loc-swap.d 2011-06-29 22:18:41.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 .* to 0x.* + Extended opcode 1: End of Sequence Index: gas/testsuite/gas/mips/mips16@loc-swap-dis.d =================================================================== --- /dev/null 2011-06-29 20:49:09.402506167 +0100 +++ gas/testsuite/gas/mips/mips16@loc-swap-dis.d 2011-06-29 22:06:15.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 +[ ]*[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 +[ ]*[0-9a-f]+: R_MIPS16_26 bar +[0-9a-f]+ <[^>]*> 6500 nop +[0-9a-f]+ <[^>]*> 6500 nop + \.\.\. Index: gas/testsuite/gas/mips/loc-swap.s =================================================================== --- /dev/null 2011-06-29 20:49:09.402506167 +0100 +++ gas/testsuite/gas/mips/loc-swap.s 2011-06-29 22:17:43.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: gas/testsuite/gas/mips/mips.exp =================================================================== --- gas/testsuite/gas/mips/mips.exp 2011-06-29 21:38:31.000000000 +0100 +++ gas/testsuite/gas/mips/mips.exp 2011-06-29 22:06:15.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 {