This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PR gas/13024: internal error with branch swapping and double .locs
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- Cc: binutils at sourceware dot org
- Date: Mon, 10 Jun 2013 20:29:42 +0100
- Subject: Re: PR gas/13024: internal error with branch swapping and double .locs
- References: <87zkiieqyr dot fsf at firetop dot home> <201306071515 dot r57FFFnu006535 at d06av02 dot portsmouth dot uk dot ibm dot com> <87zjuyj6vd dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com>
Richard Sandiford <rsandifo@linux.vnet.ibm.com> writes:
> Index: gas/config/tc-mips.c
> ===================================================================
> --- gas/config/tc-mips.c 2013-06-10 16:31:04.501559312 +0100
> +++ gas/config/tc-mips.c 2013-06-10 16:55:43.148936439 +0100
> @@ -4370,22 +4370,18 @@ append_insn (struct mips_cl_insn *ip, ex
> branch_disp = method == APPEND_SWAP ? insn_length (history) : 0;
>
> #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. This is normally the
> - current address.
> + dwarf2_emit_insn (0);
> + /* We want MIPS16 and microMIPS debug info to use ISA-encoded addresses,
> + so "move" the instruction accordingly.
>
> - For MIPS16/microMIPS 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/microMIPS 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 ((HAVE_CODE_COMPRESSION ? -1 : 0) + branch_disp);
> + Also, it doesn't seem appropriate for the assembler to reorder
> + .loc entries. If this instruction is a branch that we are going to
> + swap with the previous instruction, the two instructions should be
> + treated as a unit, and the debug information for both instructions
> + should refer to the start of the branch sequence. This also stops
> + a debugger from corrupting the code by trying to insert a 32-bit
> + breakpoint instruction into a 16-bit MIPS16 or microMIPS delay slot. */
> + dwarf2_move_insn ((HAVE_CODE_COMPRESSION ? 1 : 0) - branch_disp);
> #endif
So this rewording was written at work without access to the manuals.
Should be "Using the current position is certainly wrong when swapping
a 32-bit branch and a 16-bit delay slot, since the current position
would then be in the middle of a branch."
Richard