Bug in relaxation in m68k binutils-2.11

[Peter Barada]

I'm in the midst of adding support to gas for the lastest version of
the ColdFire core.  See:

http://e-www.motorola.com/webapp/sps/site/prod_summary.jsp?code=COLDFIRE4E&nodeId=01M978558372849

for more details.  I plan to contribute the changes back to the FSF,
after I convince my lawyers its in Motorola's best interest to do so!

I've got a problem with relaxation in the 68k/coldfire
backend(configured for --host=i686-linux --target=m68k-elf), and since
there isn't anyone who's maintaining the 68k backend in the
binutils/MAINTAINERS list, I thought I'd send this to you, and you
could pass it on(and tell me who's looking at it so I can send them
more info).

I'll try to spare you the gory details.  Here's the listing of a
small snippet(from a much larger program which I can't prune since the
problem goes away):

 1773 07e0 4EB9 0000            jsr dbgfatal
 1773      0000 
 1774 07e6 508F                 addq.l #8,%sp
 1775                   .L309:
 1777                   .LM174:
 1778 07e8 4A87                 tst.l %d7
 1779 07ea 6612                 jbne .L318
 1780 07ec 4878 03C1            pea 961.w
 1781 07f0 4879 0000            pea .LC2
 1781      0000 
 1782 07f6 4EB9 0000            jsr dbgfatal
 1782      0000 
 1783 07fc 508F                 addq.l #8,%sp
 1784                   .L318:
 1786                   .LM175:
 1787 07fe 2044                 move.l %d4,%a0

The "jbne .L318" instruction at offset 0x07ea is not assembled
correctly.  From the objdump of the .o file produced by assembling
this same snippet I get:

 7e0:   4eb9 0000 0000  jsr 0 <parseCommentEcho>
 7e6:   508f            addql #8,%sp
 7e8:   4a87            tstl %d7
 7ea:   6600 4878       bnew 5064 <PJLisPJLLine+0x44fa>
 7ee:   03c1            bset %d1,%d1
 7f0:   4879 0000 0000  pea 0 <parseCommentEcho>
 7f6:   4eb9 0000 0000  jsr 0 <parseCommentEcho>
 7fc:   508f            addql #8,%sp
 7fe:   2044            moveal %d4,%a0

Notice that in the listing, "jbne .L318" assembles into 0x6612 at
offset 0x07ea, but in the objdump it was written as 0x6600 (which in
68k/coldfire parlance indicates that the branch consumes two words),
and the first short from the "pea .LC2" instruction is used as the
offset (0x4878). 

I turned on all the debugging in gas/write.c(by adding -DDEBUG[2-6] to
the CFLAGS definition in gas/Makefile), and added the following code
to write_contents so I could see the actual data being written in the
output from the frags:

      unsigned long fill_size;
      char *fill_literal;
      long count;

#ifdef DEBUG6
      {
        int i;
        fprintf(stderr, "frag %x offset %x length %x symbol %x\n",
                f, offset, f->fr_fix, f->fr_symbol);
        if (f->fr_fix) {
          for(i=0; i<0x1e && i<f->fr_fix; ++i)
            fprintf(stderr, "%s%02x", i? ",":"", f->fr_literal[i]&0xff);
          fprintf(stderr, "\n");
        }
      }
#endif

      assert (f->fr_type == rs_fill);
      if (f->fr_fix)
        {


And the output (just the relavent bits) is:

adjusting fixup:
fix 812d248 /tmp/c.s:1542 pcrel pcrel_adjust=-1
    size=1 frag=8127ed8 where=-1 offset=0 addnumber=0
    BFD_RELOC_8_PCREL (13)
   +<sym 8125250 .L318 resolved local .text 7fe>

adjusted fixup:
fix 812d248 /tmp/c.s:1542 pcrel pcrel_adjust=-1
    size=1 frag=8127ed8 where=-1 offset=7fe addnumber=0
    BFD_RELOC_8_PCREL (13)
   +<sym 8121030 .text resolved used-in-reloc defined .text 0>

as well as:

frag 8127a9c offset 7e0 length 6 symbol 81215a8
4e,b9,00,00,00,00
frag 8127ad8 offset 7e6 length 6 symbol 0
50,8f,4a,87,66,00
frag 8127ed8 offset 7ec length 0 symbol 8125250
frag 8127f14 offset 7ec length a symbol 8121480
48,78,03,c1,48,79,00,00,00,00


Note that the frag at 0x8127ed8 is attempting to apply a fix for
MFD_RELO_8_PCREL of size 1 to where of -1.  When I step through
md_apply_fix2, at the point that "*buf++ = val;" executes, buf is
pointing to the character before &fixP->fx_frag->fr_literal[0]:

#0  md_apply_fix_2 (fixP=0x812d248, val=18)
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:4420
(gdb) p *fixP->fx_frag
$47 = {fr_address = 2028, fr_next = 0x8127f14, fr_fix = 0, fr_var = 0, 
  fr_symbol = 0x8125250, fr_offset = 0, fr_opcode = 0x8127b0c "f", line = 0x0, 
  fr_type = rs_fill, fr_subtype = 4, fr_file = 0xbffffa62 "/tmp/c.s", 
  fr_line = 1153, fr_literal = ""}
(gdb) p buf
$48 = 0x8127f07 ""
(gdb) p &fixP->fx_frag->fr_literal[0]
$49 = 0x8127f08 ""
(gdb) s
(gdb) p *fixP->fx_frag
$50 = {fr_address = 2028, fr_next = 0x8127f14, fr_fix = 0, fr_var = 0, 
  fr_symbol = 0x8125250, fr_offset = 0, fr_opcode = 0x8127b0c "f", line = 0x0, 
  fr_type = rs_fill, fr_subtype = 4, fr_file = 0xbffffa62 "/tmp/c.s", 
  fr_line = 301991041, fr_literal = ""}
(gdb) p val
$51 = 18
(gdb)

Note that the value of 18 is the correct offset to fix, but the
place where its done is completely wrong in that it changes
fixP->fx_frag->fr_line from 1153(0x00000841) to 301991041(0x12000481),
and not the last byte int he previous frag(at least on my x86 host).

So the first question is whether or not fx_frag.fx_where can ever hold a
negative value.  From observing the code I don't believe so.  Working
under that assumption, I backed up and had gdb break on the assignment
to fx_where if its negative: 

(gdb) b fix_new_internal if where < 0
Breakpoint 16 at 0x806581b: file ../../../../binutils-2.11/gas/write.c, line 
165.
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: 
/home/pbarada/work/cvs-wavemark/cross-linux-tools/obj/m68k-elf/build-binutils/gas/as-new
 -o /tmp/c.o /tmp/c.s

Breakpoint 16, fix_new_internal (frag=0x8127ed8, where=-1, size=1, 
    add_symbol=0x8125250, sub_symbol=0x0, offset=0, pcrel=1, 
    r_type=BFD_RELOC_8_PCREL) at ../../../../binutils-2.11/gas/write.c:165
(gdb) 

And sure enough, 'where' is negative(and its frag 0x8127ed8)!  I backed up
the stack and tried to figure out why 'where' becomes negative:

(gdb) up
#1  0x08065ab2 in fix_new (frag=0x8127ed8, where=-1, size=1, 
    add_symbol=0x8125250, offset=0, pcrel=1, r_type=BFD_RELOC_8_PCREL)
    at ../../../../binutils-2.11/gas/write.c:259
(gdb) 
#2  0x080700de in md_convert_frag_1 (fragP=0x8127ed8)
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:4568
(gdb) b md_convert_frag_1 if fragP == 0x8127ed8
Breakpoint 17 at 0x806fffe: file 
../../../../binutils-2.11/gas/config/tc-m68k.c, line 4542.
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: 
/home/pbarada/work/cvs-wavemark/cross-linux-tools/obj/m68k-elf/build-binutils/gas/as-new
 -o /tmp/c.o /tmp/c.s

Breakpoint 17, md_convert_frag_1 (fragP=0x8127ed8)
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:4542
(gdb) p *fragP
$52 = {fr_address = 2028, fr_next = 0x8127f14, fr_fix = 0, fr_var = 0, 
  fr_symbol = 0x8125250, fr_offset = 0, fr_opcode = 0x8127b0c "f", line = 0x0, 
  fr_type = rs_machine_dependent, fr_subtype = 4, 
  fr_file = 0xbffffa62 "/tmp/c.s", fr_line = 1153, fr_literal = ""}
(gdb) p fragP->fr_fix
$53 = 0
(gdb) 

And since fix_new is called with fragP->fr_fix-1 (at least for a
subtype of 4), that's where the negative value of 'where' comes from.
Backing up even further to figure out why fr_fix is set to zero, and
set a breakpoint in frag_new just after:

  /* Fix up old frag's fr_fix.  */
  frag_now->fr_fix = frag_now_fix_octets () - old_frags_var_max_size;

I get the following.  You'll note that I skipped the first hit of the
breakpoint since it was an rs_align type, and didn't seem to be
causeing the problem:

Breakpoint 20 at 0x8053a59: file ../../../../binutils-2.11/gas/frags.c, line 
120.
(gdb) cond 20 frag_now->fr_fix == 0
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: 
/home/pbarada/work/cvs-wavemark/cross-linux-tools/obj/m68k-elf/build-binutils/gas/as-new
 -o /tmp/c.o /tmp/c.s

Breakpoint 19, frag_new (old_frags_var_max_size=1)
    at ../../../../binutils-2.11/gas/frags.c:120
(gdb) where 3
#0  frag_new (old_frags_var_max_size=1)
    at ../../../../binutils-2.11/gas/frags.c:120
#1  0x08053d3f in frag_var (type=rs_align, max_chars=1, var=1, subtype=0, 
    symbol=0x0, offset=2, opcode=0x0)
    at ../../../../binutils-2.11/gas/frags.c:220
#2  0x08053e68 in frag_align (alignment=2, fill_character=0, max=0)
    at ../../../../binutils-2.11/gas/frags.c:298
(More stack frames follow...)
(gdb) c
Continuing.

Breakpoint 19, frag_new (old_frags_var_max_size=10)
    at ../../../../binutils-2.11/gas/frags.c:120
(gdb) where 3
#0  frag_new (old_frags_var_max_size=10)
    at ../../../../binutils-2.11/gas/frags.c:120
#1  0x08053d3f in frag_var (type=rs_machine_dependent, max_chars=10, var=0, 
    subtype=7, symbol=0x8125250, offset=0, opcode=0x8127b0c "f")
    at ../../../../binutils-2.11/gas/frags.c:220
#2  0x0806ed4f in md_assemble (str=0x80ebb07 "jbne .L318")
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:3908
(More stack frames follow...)
(gdb) p frag_now
$63 = (fragS *) 0x8127ed8
(gdb) p *frag_now
$64 = {fr_address = 0, fr_next = 0x0, fr_fix = 0, fr_var = 0, 
  fr_symbol = 0x8125250, fr_offset = 0, fr_opcode = 0x8127b0c "f", line = 0x0, 
  fr_type = rs_machine_dependent, fr_subtype = 7, 
  fr_file = 0xbffffa62 "/tmp/c.s", fr_line = 1153, fr_literal = ""}
(gdb) 


Note that we're back at our old freind frag 0x8127ed8.  Backing up to
the md_assemble call that caused this I see:

(gdb) b md_assemble
Breakpoint 22 at 0x806e66d: file 
../../../../binutils-2.11/gas/config/tc-m68k.c, line 3748.
(gdb) ign 22 701
Will ignore next 701 crossings of breakpoint 22.
(gdb) run
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: 
/home/pbarada/work/cvs-wavemark/cross-linux-tools/obj/m68k-elf/build-binutils/gas/as-new
 -o /tmp/c.o /tmp/c.s

Breakpoint 22, md_assemble (str=0x80ebb07 "jbne .L318")
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:3748
Note: breakpoint 21 also set at pc 0x806ecfc.
Breakpoint 23 at 0x806ecfc: file 
../../../../binutils-2.11/gas/config/tc-m68k.c, line 3908.
(gdb) c
Continuing.

Breakpoint 23, md_assemble (str=0x80ebb07 "jbne .L318")
    at ../../../../binutils-2.11/gas/config/tc-m68k.c:3908
(gdb) s
frag_var (type=rs_machine_dependent, max_chars=10, var=0, subtype=7, 
    symbol=0x8125250, offset=0, opcode=0x8127b0c "f")
    at ../../../../binutils-2.11/gas/frags.c:202
(gdb) p max_chars
$66 = 10
(gdb) p frchain_now->frch_obstack
$67 = {chunk_size = 4072, chunk = 0x8126b28, 
  object_base = 0x8127b08 "P\217J\207f", next_free = 0x8127b0e "", 
  chunk_limit = 0x8127b10 "", temp = 0, alignment_mask = 3, 
  chunkfun = 0x80a8ee0 <xmalloc>, freefun = 0x8049570 <free>, extra_arg = 0x0, 
  use_extra_arg = 0, maybe_empty_object = 1, alloc_failed = 0}
(gdb) p  frchain_now->frch_obstack.chunk_limit - 
frchain_now->frch_obstack.next_free
$68 = 2
(gdb) 


Note that this is the call to frag_var for the instruction "jbne .L318", 
and the the first statement in frag_var is a call to
frag_grow(max_chars) which in this case causes the fragment to
grow(causing the first fragment holding the actual branch to be
closed, and a new fragment set up wheras the fr_fix gets set to zero).

I'm wondering if this is a *window-of-doom* where the instruction fits in
the frag, but the maximum size of the operands don't, causing the frag to
be closed, and another frag opened, but the fr_fix is zero since the
frag is closed again due to the frag_new at the bottom of frag_var,
which ultimately ends up causeing md_apply_fix_2 to attempt to update
the last byte of the *previous* fragment.

So I ask the question of whether or not in frag_var, should one
guarantee that the fragment has enough space to hold the max by making
sure that before any part of an instruction is placed in the frag,
that the frag can hold the maximum sized instruction.  Perhaps by placing a
'frag_grow(max_chars)' in md_assemble before any part of the
instruction is assembled and an assert in frag_var that it can hold
max_chars, this situation can be avoided.  Or does that create a
bigger mess elsewhere.

I hope the above is enough information to help in identifying this
problem(as well as leading to a fix).

If anybody has an questions or suggestions, please contact me.

-- 
Peter Barada                                   address@hidden
Wizard                                         781-852-2768 (direct)
WaveMark Solutions(wholly owned by Motorola)   781-270-0193 (fax)