[powerpc] powerpc-linux-gnu accesses memory below the stack

[Quuxplusone]

Bugzilla Link	PR22819
Status	NEW
Importance	P normal
Reported by	Markus F.X.J. Oberhumer (markus@oberhumer.com)
Reported on	2015-03-06 06:36:04 -0800
Last modified on	2016-08-30 14:00:17 -0700
Version	3.6
Hardware	PC Linux
CC	chmeeedalf@gmail.com, hfinkel@anl.gov, llvm-bugs@lists.llvm.org, rdivacky@freebsd.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

powerpc-linux-gnu accesses memory below the stack - please see "-4(1)" below.

This is a nasty one as it only shows up under rare conditions - it was
quite hard to find the real cause of the problem.

$ cat test.c

int foo(int *);

int test(int *a) {
    return foo(a) + 1;
}

// EOF

$ clang-3.6.0 -target powerpc-linux-gnu -O2 -Wall -save-temps -c test.c

$ cat test.s

        .text
        .file   "test.bc"
        .globl  test
        .align  2
        .type   test,@function
test:                                   # @test
# BB#0:                                 # %entry
        mflr 0
        stw 31, -4(1)
        stw 0, 4(1)
        stwu 1, -16(1)
        mr 31, 1
        bl foo
        addi 3, 3, 1
        addi 1, 1, 16
        lwz 0, 4(1)
        lwz 31, -4(1)
        mtlr 0
        blr
.Ltmp0:
        .size   test, .Ltmp0-test
        .ident  "clang version 3.6.0 (tags/RELEASE_360/final)"
        .section        ".note.GNU-stack","",@progbits

[Quuxplusone]

Indeed; this is a known issue. The frame-lowering code assumes the availability of a red zone on all targets. However, this is actually only legal on PPC64/Linux and on Darwin. It is not legal on PPC/Linux.

[Quuxplusone]

I've just noticed that there is an option -disable-red-zone (but -mno-red-zone
is not an allowed option??), and in fact

   clang-3.6.0 -target powerpc-apple-darwin -Xclang -disable-red-zone

also accesses memory below the stack.

Is there a specific reason why the stack pointer is not adjusted first?

[Quuxplusone]

(In reply to comment #2)
> I've just noticed that there is an option -disable-red-zone (but
> -mno-red-zone
> is not an allowed option??), and in fact
>
>    clang-3.6.0 -target powerpc-apple-darwin -Xclang -disable-red-zone
>
> also accesses memory below the stack.
>
> Is there a specific reason why the stack pointer is not adjusted first?

No, and it really should be adjusted first for ppc32. Patches welcome.

[Quuxplusone]

Please note that I do consider this a critical bug rendering
llvm ppc32-elf completely unusable for production code.

In the meantime I've also looked into this issue a little bit deeper and
have found several "FIXME" in PPCFrameLowering.cpp, including one
from SVN rev 157133 ;-)

Unfortunately I personally don't know enough about PPC to be able to fix this.

[Quuxplusone]

It seems that in our scenario we can work around spilling r31 by using
-fomit-frame-pointer, so I have a question about r30.

Consider this code:

$ cat test2.c

extern int foo;
int *test(void) { return &foo; }

// EOF

$ clang-3.6.0 -target powerpc-linux-gnu -fPIC -O2 -fomit-frame-pointer -Wall -
save-temps -c test2.c

$ cat test2.s

[...]

test:
# BB#0:                                 # %entry
        mflr 0
        stw 30, -8(1)        <<<<<
        stw 0, 4(1)
        stwu 1, -16(1)
        stw 30, 8(1)                    # 4-byte Folded Spill
        bl .L0$pb
.L0$pb:
        mflr 30
        lwz 3, .L0$poff-.L0$pb(30)
        add 30, 3, 30
        lwz 3, .LC1-.LTOC(30)
        lwz 30, 8(1)                    # 4-byte Folded Reload
        addi 1, 1, 16
        lwz 0, 4(1)
        lwz 30, -8(1)        <<<<<
        mtlr 0
        blr

[...]

It seems that extra prologue/epilogue spilling of r30 ("<<<<<") is
actually not needed at all because R30 is included in CalleeSavedRegs.

So would it be safe to simply disable the test "FI->usesPICBase()"
in PPCFrameLowering.cpp lines 711 and 1089 ?

Thanks,
Markus

[Quuxplusone]

(In reply to comment #5)
...
>
> It seems that extra prologue/epilogue spilling of r30 ("<<<<<") is
> actually not needed at all because R30 is included in CalleeSavedRegs.
>
> So would it be safe to simply disable the test "FI->usesPICBase()"
> in PPCFrameLowering.cpp lines 711 and 1089 ?

[+Justin who implemented the PIC support]

>
> Thanks,
> Markus

[Quuxplusone]

I added in the explicit save/restore of R30 in the prologue/epilogue because during my testing there were cases where not doing so would not save $r30, and I could not determine why, so punted. Unfortunately, I don't have time right now to test if this has changed.

[Quuxplusone]

(In reply to comment #5)
>
> It seems that extra prologue/epilogue spilling of r30 ("<<<<<") is
> actually not needed at all because R30 is included in CalleeSavedRegs.
>
> So would it be safe to simply disable the test "FI->usesPICBase()"
> in PPCFrameLowering.cpp lines 711 and 1089 ?
>
> Thanks,
> Markus

Do you mean to remove just the check, and unconditionally save/restore?  Or do
you mean to remove the whole block in each?