Bug 81996 - powerpc __builtin_return_address(0) fails with -fPIC -fstack-protector-all or -fsanitize=address
Summary: powerpc __builtin_return_address(0) fails with -fPIC -fstack-protector-all or...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.2.0
: P3 normal
Target Milestone: ---
Assignee: Alan Modra
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2017-08-27 07:34 UTC by Sergei Trofimovich
Modified: 2017-09-21 13:46 UTC (History)
4 users (show)

See Also:
Host:
Target: powerpc-unknown-linux-gnu
Build:
Known to work:
Known to fail: 8.0
Last reconfirmed: 2017-08-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2017-08-27 07:34:19 UTC
I've found a problem on glibc-2.25 where glibc crashes at startup.

Minimal reproducer does not crash the program but yields bad result:

  #include <stdio.h>

  static void * f(void) __attribute__((noinline));
  static void * f(void) {
      return __builtin_extract_return_addr (__builtin_return_address(0));
  }

  int main(void) {
    printf ("main    =%p\n", &main);
    printf ("ret_addr=%p\n", f());
    return 0;
  }

$ powerpc-unknown-linux-gnu-gcc-7.2.0 a.c -O2 -fno-PIC -o a && ./a
main    =0x100002e0
ret_addr=0x1000030c <- good!
$ powerpc-unknown-linux-gnu-gcc-7.2.0 a.c -O2 -fPIC -o a && ./a
main    =0x100002e0
ret_addr=0x4        <- bad!

[ In real example glibc crash happens at RETURN_ADDRESS(0) call here:
    https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/malloc.c;h=e3ff778113febdd0533aeea70f1a35f62259bcfd;hb=HEAD#l3061 ]

Normally gcc should use 'lr' value in both cases but for some reason
it tries to spill 'lr' into stack and then reads it from wrong location:

-fno-PIC: a frame is buit, but good (master does even better than that):

10000410 <f>:
10000410:       94 21 ff f0     stwu    r1,-16(r1)
10000414:       7c 68 02 a6     mflr    r3
10000418:       38 21 00 10     addi    r1,r1,16
1000041c:       4e 80 00 20     blr

-fPIC (

10000420 <f>:
10000420:       94 21 ff e0     stwu    r1,-32(r1)
10000424:       7c 08 02 a6     mflr    r0
10000428:       90 01 00 24     stw     r0,36(r1) ; spill 'lr' into stack
1000042c:       93 c1 00 18     stw     r30,24(r1)
10000430:       81 21 00 10     lwz     r9,16(r1) ; step1 (uninitialized garbage value)
10000434:       80 01 00 24     lwz     r0,36(r1)
10000438:       80 69 00 04     lwz     r3,4(r9) ; step2 (glibc SIGSEGVs here)
1000043c:       83 c1 00 18     lwz     r30,24(r1)
10000440:       38 21 00 20     addi    r1,r1,32
10000444:       7c 08 03 a6     mtlr    r0
10000448:       4e 80 00 20     blr
Comment 1 Sergei Trofimovich 2017-08-27 07:36:40 UTC
Compiler version:

Target: powerpc-unknown-linux-gnu
Configured with: /dev/shm/portage/sys-devel/gcc-7.2.0/work/gcc-7.2.0/configure --host=powerpc-unknown-linux-gnu --build=powerpc-unknown-linux-gnu --prefix=/usr --bindir=/usr/powerpc-unknown-linux-gnu/gcc-bin/7.2.0 --includedir=/usr/lib/gcc/powerpc-unknown-linux-gnu/7.2.0/include --datadir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/7.2.0 --mandir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/7.2.0/man --infodir=/usr/share/gcc-data/powerpc-unknown-linux-gnu/7.2.0/info --with-gxx-include-dir=/usr/lib/gcc/powerpc-unknown-linux-gnu/7.2.0/include/g++-v7 --with-python-dir=/share/gcc-data/powerpc-unknown-linux-gnu/7.2.0/python --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 7.2.0 p1.1' --disable-esp --enable-libstdcxx-time --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --disable-multilib --disable-altivec --disable-fixed-point --enable-targets=all --disable-libgcj --enable-libgomp --disable-libmudflap --disable-libssp --disable-libcilkrts --disable-libmpx --disable-vtable-verify --disable-libvtv --enable-lto --without-isl --enable-libsanitizer --disable-default-pie --enable-default-ssp
Thread model: posix
gcc version 7.2.0 (Gentoo 7.2.0 p1.1)
Comment 2 Sergei Trofimovich 2017-08-27 07:48:05 UTC
gcc master is slightly better here but is still inefficient in fPIC mode:

gcc -O2 -fno-PIC:

    f:
        mflr 3
        blr

gcc -O2 -fPIC:

    f:
        stwu 1,-16(1)
        mflr 0
        stw 0,20(1)
        stw 30,8(1)
        lwz 9,0(1)
        lwz 0,20(1)
        lwz 3,4(9)
        lwz 30,8(1)
        mtlr 0
        addi 1,1,16
        blr
Comment 3 Sergei Trofimovich 2017-08-27 11:56:10 UTC
Built vanilla gcc from head to make sure it's not local patches.
-fstack-protector-all is the trigger breaking return address:

  Target: powerpc-unknown-linux-gnu
  Configured with: ../gcc/configure --prefix=/root/gcc-installed
  Thread model: posix
  gcc version 8.0.0 20170827 (experimental) (GCC) 

$ gcc-installed/bin/gcc -O2 a.c -o a -fPIC -fstack-protector-all && ./a
main    =0x1000030c
ret_addr=0x7c641b78

$ gcc-installed/bin/gcc -O2 a.c -o a -fno-PIC -fstack-protector-all && ./a
main    =0x10000310
ret_addr=0x10000344
Comment 4 Segher Boessenkool 2017-08-28 10:51:48 UTC
Hi!

(In reply to Sergei Trofimovich from comment #2)
> gcc master is slightly better here but is still inefficient in fPIC mode:
> 
> gcc -O2 -fno-PIC:
> 
>     f:
>         mflr 3
>         blr
> 
> gcc -O2 -fPIC:
> 
>     f:
>         stwu 1,-16(1)
>         mflr 0
>         stw 0,20(1)
>         stw 30,8(1)
>         lwz 9,0(1)
>         lwz 0,20(1)
>         lwz 3,4(9)
>         lwz 30,8(1)
>         mtlr 0
>         addi 1,1,16
>         blr

This code looks fine.  But it is *without* -fstack-protector-all, and with
it indeed a problem shows up:

===
f:
        stwu 1,-48(1)
        mflr 0
        stw 0,52(1)          # save at old r1+4
        stw 30,40(1)
        lwz 9,-28680(2)
        stw 9,28(1)
        li 9,0
        lwz 9,32(1)          # tries to get the old r1; but that is at r1+48 !
        lwz 10,28(1)
        lwz 8,-28680(2)
        xor. 10,10,8
        li 8,0
        lwz 3,4(9)           # and load from old r1+4
        bne 0,.L5
        lwz 0,52(1)
        lwz 30,40(1)
        addi 1,1,48
        mtlr 0
        blr
.L5:
        bl __stack_chk_fail@plt
===

Confirmed.  (Needs -m32).
Comment 5 Mikael Pettersson 2017-09-16 11:59:21 UTC
I can reproduce this wrong-code on powerpc64-linux-gnu with -m32 -O2 -fPIC -fstack-protector-all and every gcc from 8.0 down to and including 4.1.2.
Comment 6 Alan Modra 2017-09-17 04:01:06 UTC
It looks like this is FRAME_GROWS_DOWNWARD, which rs6000.h sets true for flag_stack_protect.  And, yes, -fsanitize=address shows the same sort of error, reading an offset from r1 instead of 0(r1) to get the previous frame where lr is saved.
Comment 7 Alan Modra 2017-09-18 02:25:42 UTC
Author: amodra
Date: Mon Sep 18 02:25:10 2017
New Revision: 252901

URL: https://gcc.gnu.org/viewcvs?rev=252901&root=gcc&view=rev
Log:
[RS6000] PR81996, __builtin_return_address(0) fails

rs6000_return_addr assumes that the stack link is at frame+0, which is
true for count>0.  For count==0, rs6000_return_addr is called with
frame==frame_pointer_rtx and the stack link is *not* at frame+0 if
-fstack-protector-all or -fsanitize=address because rs6000.h sets
FRAME_GROWS_DOWNWARD for those options.

	PR target/81996
	* gcc/config/rs6000/rs6000.c (rs6000_return_addr): Use
	stack_pointer_rtx for count 0.  Update comments.  Break up
	large rtl expression.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/rs6000/rs6000.c
Comment 8 Alan Modra 2017-09-21 12:56:08 UTC
Author: amodra
Date: Thu Sep 21 12:55:37 2017
New Revision: 253067

URL: https://gcc.gnu.org/viewcvs?rev=253067&root=gcc&view=rev
Log:
PR81996, __builtin_return_address(0) fails

rs6000_return_addr assumes that the stack link is at frame+0, which is
true for count>0.  For count==0, rs6000_return_addr is called with
frame==frame_pointer_rtx and the stack link is *not* at frame+0 if
-fstack-protector-all or -fsanitize=address because rs6000.h sets
FRAME_GROWS_DOWNWARD for those options.

	PR target/81996
	* gcc/config/rs6000/rs6000.c (rs6000_return_addr): Use
	stack_pointer_rtx for count 0.  Update comments.  Break up
	large rtl expression.

Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
Comment 9 Alan Modra 2017-09-21 12:57:55 UTC
Author: amodra
Date: Thu Sep 21 12:57:24 2017
New Revision: 253068

URL: https://gcc.gnu.org/viewcvs?rev=253068&root=gcc&view=rev
Log:
PR81996, __builtin_return_address(0) fails

rs6000_return_addr assumes that the stack link is at frame+0, which is
true for count>0.  For count==0, rs6000_return_addr is called with
frame==frame_pointer_rtx and the stack link is *not* at frame+0 if
-fstack-protector-all or -fsanitize=address because rs6000.h sets
FRAME_GROWS_DOWNWARD for those options.

	PR target/81996
	* gcc/config/rs6000/rs6000.c (rs6000_return_addr): Use
	stack_pointer_rtx for count 0.  Update comments.  Break up
	large rtl expression.

Modified:
    branches/gcc-6-branch/gcc/ChangeLog
    branches/gcc-6-branch/gcc/config/rs6000/rs6000.c
Comment 10 Alan Modra 2017-09-21 13:26:16 UTC
Author: amodra
Date: Thu Sep 21 13:25:45 2017
New Revision: 253070

URL: https://gcc.gnu.org/viewcvs?rev=253070&root=gcc&view=rev
Log:
PR81996, __builtin_return_address(0) fails

rs6000_return_addr assumes that the stack link is at frame+0, which is
true for count>0.  For count==0, rs6000_return_addr is called with
frame==frame_pointer_rtx and the stack link is *not* at frame+0 if
-fstack-protector-all or -fsanitize=address because rs6000.h sets
FRAME_GROWS_DOWNWARD for those options.

	PR target/81996
	* gcc/config/rs6000/rs6000.c (rs6000_return_addr): Use
	stack_pointer_rtx for count 0.  Update comments.  Break up
	large rtl expression.

Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/config/rs6000/rs6000.c
Comment 11 Alan Modra 2017-09-21 13:30:27 UTC
Fixed all active branches
Comment 12 wschmidt 2017-09-21 13:46:39 UTC
Thanks for sorting this out, Alan!

Bill

> On Sep 21, 2017, at 8:30 AM, amodra at gmail dot com <gcc-bugzilla@gcc.gnu.org> wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81996
> 
> Alan Modra <amodra at gmail dot com> changed:
> 
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>             Status|ASSIGNED                    |RESOLVED
>         Resolution|---                         |FIXED
> 
> --- Comment #11 from Alan Modra <amodra at gmail dot com> ---
> Fixed all active branches
> 
> -- 
> You are receiving this mail because:
> You are watching someone on the CC list of the bug.