Miscompilation: register smashed on ppc

[Quuxplusone]

Bugzilla Link	PR2464
Status	RESOLVED FIXED
Importance	P normal
Reported by	Gary Benson (gbenson@redhat.com)
Reported on	2008-06-16 03:35:14 -0700
Last modified on	2014-12-09 08:23:00 -0800
Version	trunk
Hardware	Macintosh Linux
CC	baldrick@free.fr, evan.cheng@apple.com, fang@csl.cornell.edu, hfinkel@anl.gov, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments	test.bc (63192 bytes, application/octet-stream) test.s (109883 bytes, application/octet-stream) test.ll (453976 bytes, application/octet-stream) func5.bc (42900 bytes, application/octet-stream) test-a.bc (11924 bytes, application/octet-stream) test-b.bc (11484 bytes, application/octet-stream)
Blocks
Blocked by
See also

func5 in the attached test.bc gets miscompiled on 32-bit ppc Linux.  The
failing section is instrumented with calls to trace_bytecode and print_value,
which product the following output:

  630: iload
  local_4_113 = 0
  632: iload
  local_5_114 = 57
  local_4_113 = 0
  634: if_icmplt
  132: aload_3
  133: iload
  135: caload
  136: istore
  138: iload
  140: sipush
  143: if_icmpge
  146: iload
  148: iload
  150: if_icmplt
  185: aload
  187: iload
  189: iinc
  192: iload
  194: i2b
  195: bastore
  196: iinc
  199: goto
  630: iload
  local_4_419 = 1
  632: iload
  local_5_420 = 261095424
  local_4_419 = 1

The variable local_5_* is used to see when to exit a loop; it's "sl" in this:

  int sp = 0;
  int sl = whatever;
  while (sp < sl) {
    // do stuff
  }

In the attached test.s (and test.ll):

  lines 2646-2648 (line 3993 in test.ll) print the first "632: iload"
  lines 2649-2652 (line 3994 in test.ll) print "local_5_114 = 57" (correct)

From line 2651 you can see that the 57 came from r26.

At the bottom:

  lines 4901-4903 (7791 in test.ll) print the second "632: iload"
  lines 4904-4907 (7792 in test.ll) print "local_5_420 = 261095424" (junk)

From line 4906 you can see that the 261095424 also came from r26. Looking at
what happens to r26 in the meantime it seems it's being used to hold temporary
values:

 lines 2684 and 2685 (4048 and 4049 in test.ll calculate an offset into an array which is then used in line 2687.
 line 2703 (the top half of the 261101980 in lines 4074 and 4075 in test.ll) stores the high word of a pair of inlined pointers, used in lines 2704 and 2711.

That last one is where the 261095424 comes from.

This is all with svn revision 52213 BTW, but the miscompilation was also
present in 49380.

[Quuxplusone]

Attached test.bc (63192 bytes, application/octet-stream): Bitcode

[Quuxplusone]

Attached test.s (109883 bytes, application/octet-stream): llc test.bc -o test.s

[Quuxplusone]

Attached test.ll (453976 bytes, application/octet-stream): llvm-dis test.bc

[Quuxplusone]

http://gbenson.net/llc.debug.gz
llc -debug test.bc -o test.s >llc.debug 2>&1

[Quuxplusone]

In bci_630_private_copy, this line
        call void @print_value( i32 ptrtoint ([12 x i8]* @dump56 to i32), i32 %local_5_114 )
prints properly: %local_5_114 has the right value.  After ppc32 codegen
the value is in r26.

Later in bci_630, this line
        call void @print_value( i32 ptrtoint ([12 x i8]* @dump59 to i32), i32 %local_5_420 )
prints wrongly: %local_5_420 has the wrong value.  This value is also in
r26.

What happened to r26 between these two basic blocks?  The wrong value it
gets comes from bci_146:

bci_146: 0x10e5b700, LLVM BB @0x10e1d818, ID#91:
Live Ins: %r28 %r29 %r30 %r23 %r24 %r20 %r15 %r16 %r17 %r18 %r27 %r19 %r25 %r22
%r14
    Predecessors according to CFG: 0x10e5b660 (#90)
        %r26<def> = LIS 3984
        %r21<def> = ORI %r26, 6556

Note that r26 is not live-in here.

However the value was already wacked in the predecessor of bci_146, namely
in_bounds:

in_bounds: 0x10e5b660, LLVM BB @0x10e1d7f0, ID#90:
Live Ins: %r28 %r29 %r30 %r23 %r24 %r20 %r15 %r16 %r17 %r18 %r27 %r19 %r25 %r22
    Predecessors according to CFG: 0x10ebde30 (#88)
        %r26<def> = RLWINM %r27, 1, 0, 30

Note that r26 is not live-in here.

The predecessor of in_bounds is not_null547, and here r26 is live-in:

not_null547: 0x10ebde30, LLVM BB @0x10e1d7a0, ID#88:
Live Ins: %r28 %r29 %r30 %r23 %r24 %r20 %r15 %r16 %r17 %r18 %r27 %r26 %r19 %r25
%r22
    Predecessors according to CFG: 0x10ebd9d0 (#86)
        %r3<def> = LWZ 8, %r18, Mem:LD(4,4) [0x10e31c4c + 0]
        %cr0<def> = CMPW %r27, %r3<kill>
        BCC 12, %cr0<kill>, mbb<in_bounds,0x10e5b660>
    Successors according to CFG: 0x10e5b660 (#90) 0x10e5b5c0 (#89)

It seems suspicious that r26 is live-in on not_null547 but not on
in_bounds even though nothing happens to r26 in not_null547.

[Quuxplusone]

Duncan, I don't really follow your diagnosis:

>bci_146: 0x10e5b700, LLVM BB @0x10e1d818, ID#91:
>Live Ins: %r28 %r29 %r30 %r23 %r24 %r20 %r15 %r16 %r17 %r18 %r27 %r19 %r25 %r22
>%r14
>    Predecessors according to CFG: 0x10e5b660 (#90)
>        %r26<def> = LIS 3984
>        %r21<def> = ORI %r26, 6556
>
>Note that r26 is not live-in here.
>
>However the value was already wacked in the predecessor of bci_146, namely
>in_bounds:

r26 is not livein to bci_146. It is computed with the LIS + ORI instruction.
Why do you think it's already wrong in in_bounds?

>
>in_bounds: 0x10e5b660, LLVM BB @0x10e1d7f0, ID#90:
>Live Ins: %r28 %r29 %r30 %r23 %r24 %r20 %r15 %r16 %r17 %r18 %r27 %r19 %r25 %r22
>    Predecessors according to CFG: 0x10ebde30 (#88)
>        %r26<def> = RLWINM %r27, 1, 0, 30
>
>Note that r26 is not live-in here.

The code is too large to try to debug by hand. Why not use bugpoint to reduce
it down?

[Quuxplusone]

Hi Evan, thanks for taking a look.  I think you misunderstood what
I was saying.  The value for %local_5_420 printed in bci_630 is wrong.
The value is held in r26:

        ADJCALLSTACKDOWN 16, %r1<imp-def>, %r1<imp-use>
        %r3<def> = LA %r14<kill>, <ga:dump59>
        %r4<def> = OR %r26, %r26
        BL_ELF <ga:print_value>, %r3<kill>, %r4<kill>, %r0<imp-def>, %r2<imp-def>, %r3<imp-def,dead>, %r4<imp-def,dead>, %r5<imp-def,dead>, %r6<imp-def,dead>, %r7<imp-def,dead>, %r8<imp-def,dead>, %r9<imp-def,dead>, %r10<imp-def,dead>, %r11<imp-def,dead>, %r12<imp-def,dead>, %f0<imp-def,dead>, %f1<imp-def,dead>, %f2<imp-def,dead>, %f3<imp-def,dead>, %f4<imp-def,dead>, %f5<imp-def,dead>, %f6<imp-def,dead>, %f7<imp-def,dead>, %f8<imp-def,dead>, %v0<imp-def,dead>, %v1<imp-def,dead>, %v2<imp-def,dead>, %v3<imp-def,dead>, %v4<imp-def,dead>, %v5<imp-def,dead>, %v6<imp-def,dead>, %v7<imp-def,dead>, %v8<imp-def,dead>, %v9<imp-def,dead>, %v10<imp-def,dead>, %v11<imp-def,dead>, %v12<imp-def,dead>, %v13<imp-def,dead>, %v14<imp-def,dead>, %v15<imp-def,dead>, %v16<imp-def,dead>, %v17<imp-def,dead>, %v18<imp-def,dead>, %v19<imp-def,dead>, %lr<imp-def>, %ctr<imp-def,dead>, %cr0<imp-def,dead>, %cr1<imp-def,dead>, %cr5<imp-def,dead>, %cr6<imp-def,dead>, %cr7<imp-def,dead>, %0<imp-def,dead>, %1<imp-def,dead>, %2<imp-def,dead>, %3<imp-def,dead>, %4<imp-def,dead>, %5<imp-def,dead>, %6<imp-def,dead>, %7<imp-def,dead>, %20<imp-def,dead>, %21<imp-def,dead>, %22<imp-def,dead>, %23<imp-def,dead>, %24<imp-def,dead>, %25<imp-def,dead>, %26<imp-def,dead>, %27<imp-def,dead>, %28<imp-def,dead>, %29<imp-def,dead>, %30<imp-def,dead>, %31<imp-def,dead>
        ADJCALLSTACKUP 16, 0, %r1<imp-def>, %r1<imp-use>

In an earlier basic block bci_630_private_copy the right value for
%local_5_114 is printed.  This is also held in r26:

        ADJCALLSTACKDOWN 16, %r1<imp-def>, %r1<imp-use>
        %r3<def> = LA %r14<kill>, <ga:dump56>
        %r4<def> = OR %r26, %r26
        BL_ELF <ga:print_value>, %r3<kill>, %r4<kill>, %r0<imp-def>, %r2<imp-def>, %r3<imp-def,dead>, %r4<imp-def,dead>, %r5<imp-def,dead>, %r6<imp-def,dead>, %r7<imp-def,dead>, %r8<imp-def,dead>, %r9<imp-def,dead>, %r10<imp-def,dead>, %r11<imp-def,dead>, %r12<imp-def,dead>, %f0<imp-def,dead>, %f1<imp-def,dead>, %f2<imp-def,dead>, %f3<imp-def,dead>, %f4<imp-def,dead>, %f5<imp-def,dead>, %f6<imp-def,dead>, %f7<imp-def,dead>, %f8<imp-def,dead>, %v0<imp-def,dead>, %v1<imp-def,dead>, %v2<imp-def,dead>, %v3<imp-def,dead>, %v4<imp-def,dead>, %v5<imp-def,dead>, %v6<imp-def,dead>, %v7<imp-def,dead>, %v8<imp-def,dead>, %v9<imp-def,dead>, %v10<imp-def,dead>, %v11<imp-def,dead>, %v12<imp-def,dead>, %v13<imp-def,dead>, %v14<imp-def,dead>, %v15<imp-def,dead>, %v16<imp-def,dead>, %v17<imp-def,dead>, %v18<imp-def,dead>, %v19<imp-def,dead>, %lr<imp-def>, %ctr<imp-def,dead>, %cr0<imp-def,dead>, %cr1<imp-def,dead>, %cr5<imp-def,dead>, %cr6<imp-def,dead>, %cr7<imp-def,dead>, %0<imp-def,dead>, %1<imp-def,dead>, %2<imp-def,dead>, %3<imp-def,dead>, %4<imp-def,dead>, %5<imp-def,dead>, %6<imp-def,dead>, %7<imp-def,dead>, %20<imp-def,dead>, %21<imp-def,dead>, %22<imp-def,dead>, %23<imp-def,dead>, %24<imp-def,dead>, %25<imp-def,dead>, %26<imp-def,dead>, %27<imp-def,dead>, %28<imp-def,dead>, %29<imp-def,dead>, %30<imp-def,dead>, %31<imp-def,dead>
        ADJCALLSTACKUP 16, 0, %r1<imp-def>, %r1<imp-use>

Tracing through the basic blocks in the .ll you can see that the value
printed in bci_630 should be the same as that in bci_630_private_copy,
since it is the same variable:

bci_630_private_copy            %local_5_114    #1216
safepointed                     %local_5_114    #1216
safepointed.bci_132_crit_edge   %local_5_114 (not used in BB)
bci_132                         %local_5_124 via phi node       #1226
not_null547                     %local_5_124 (not used in BB)
in_bounds                       %local_5_124 (not used in BB)
bci_146                         %local_5_134 via degenerate phi node    #1237
bci_185                         %local_5_166 via degenerate phi node    #1302
not_null600                    %local_5_166 (not used in BB)
in_bounds602                   %local_5_166 (not used in BB)
bci_630                        %local_5_420 via phi node               #1724

So where did the wrong value for r26 in bci_630 come from?  r26 is marked
as live-in for bci_630: the value of r26 came from the predecessor
in_bounds602.  The value of r26 in in_bounds602 came (live-in) from the
predecessor not_null600.  Searching back in this way you discover that
the value of r26 comes from bci_146, where it is comes from a calculation
and has nothing to do with %local_5_XYZ (which is why the value is wrong).
In the predecessor in_bounds of bci_146 you can see r26 also being used,
getting some value also unrelated to %local_5_XYZ.  Note that r26 is not
marked as live-in on in_bounds nor bci_146.

Starting from the other end, in bci_630_private_copy the value of
local_5_114 is stored in r26.  Also, r26 is marked live-in on the
following basic blocks safepointed, safepointed.bci_132_crit_edge,
bci_132 and not_null547 (the value is only used in the first one).
Why is r26 no longer live-in on in_bounds?

It seems to me that r26 should have been considered live across
in_bounds and bci_146.  That way the value wouldn't have been
clobbered in those basic blocks, and the correct value would
have reached bci_630.

By the way I can't make a smaller testcase because it is not my testcase :)
I understand it came from jit'ing java, and was dumped from within
the execution engine.  The reporter mentioned to me on irc that if
he reduces the size further then the problem goes away.

[Quuxplusone]

Attached func5.bc (42900 bytes, application/octet-stream): all the action happens in func5; here is func5 as bitcode

[Quuxplusone]

Attached test-a.bc (11924 bytes, application/octet-stream): func5 with every unentered block bar bci_153 stripped out

[Quuxplusone]

Attached test-b.bc (11484 bytes, application/octet-stream): func5 with every unentered block including bci_153 stripped out

[Quuxplusone]

I just uploaded two stripped-down bitcode files:

 test-a.bc is func5 with every unentered block except bci_153 removed.
 test-b.bc is func5 with every unentered block including bci_153 removed.

test-a.bc miscompiles, test-b.bc does not (with svn 52391).

[Quuxplusone]

Can you try tot? I have just fixed a coalescer bug. See pr2465.

[Quuxplusone]

It is still failing with 52487.

[Quuxplusone]

Is it failing the same way?

[Quuxplusone]

Yeah, exactly the same.

[Quuxplusone]

This bug is six years old, and trunk llc can't even read the test cases any more. I'm going to assume this has been fixed.