Bug 49618 - When building uClibc with GCC 4.6.1 old_atexit is miscompiled
Summary: When building uClibc with GCC 4.6.1 old_atexit is miscompiled
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.6.1
: P3 normal
Target Milestone: 4.6.2
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2011-07-03 10:52 UTC by Sedat Dilek
Modified: 2011-07-05 19:12 UTC (History)
6 users (show)

See Also:
Host: i386-linux-gnu
Target: mipsel-linux-uclibc
Build: i386-linux-gnu
Known to work: 4.5.3
Known to fail: 4.6.1
Last reconfirmed: 2011-07-04 12:38:03


Attachments
Preprocessed file "old_atexit.i" (13.39 KB, application/octet-stream)
2011-07-03 10:52 UTC, Sedat Dilek
Details
gcc47-pr49618.patch (631 bytes, patch)
2011-07-04 12:38 UTC, Jakub Jelinek
Details | Diff
gcc46-pr49618.patch (347 bytes, patch)
2011-07-04 12:39 UTC, Jakub Jelinek
Details | Diff
Simple testcase-script for gcc-PR49618 (623 bytes, application/x-sh)
2011-07-04 16:47 UTC, Sedat Dilek
Details
old_atexit.os_OptLevel-O0 (491 bytes, application/octet-stream)
2011-07-04 16:48 UTC, Sedat Dilek
Details
old_atexit.os_OptLevel-O1 (437 bytes, application/octet-stream)
2011-07-04 16:48 UTC, Sedat Dilek
Details
old_atexit.os_OptLevel-O2 (377 bytes, application/octet-stream)
2011-07-04 16:49 UTC, Sedat Dilek
Details
old_atexit.os_OptLevel-O3 (377 bytes, application/octet-stream)
2011-07-04 16:49 UTC, Sedat Dilek
Details
old_atexit.os_OptLevel-Os (376 bytes, application/octet-stream)
2011-07-04 16:50 UTC, Sedat Dilek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sedat Dilek 2011-07-03 10:52:57 UTC
Created attachment 24661 [details]
Preprocessed file "old_atexit.i"

Hi,

this issue kept me UP for several weeks. It first occured when generating a mipsel target-toolchain based on gcc-4.6.0 with uClibc-0.9.32 for a router project called freetz. Finally, I could track the problem with the assistance from Edwin Török. A big thank you, Edwin.

### Problem description:

When building uClibc with GCC 4.6.1 old_atexit is miscompiled, which causes this testprogram to crash when calling old_atexit:

#include <stdlib.h>
void foo() {}
int main() {     return atexit(foo);}

This is a regression from GCC 4.5.3 which compiled old_atexit() fine.

Attached is the preprocessed file old_atexit.i.

Commandline to create old_atexit.os:

mipsel-linux-uclibc-gcc -S old_atexit.i -o old_atexit.os -funsigned-char -fno-builtin -fno-asm -msoft-float -std=gnu99 -march=4kc -mtune=4kc -mabi=32 -fno-stack-protector -Os -funit-at-a-time
-fmerge-all-constants -fstrict-aliasing -fno-tree-loop-optimize -fno-tree-dominator-opts -fno-strength-reduce -mno-split-addresses -fPIC

As seen below with GCC 4.6.1 &__dso_handle is assumed to be non-NULL
and the branch (beqz) eliminated, but it is in fact NULL at runtime
which causes the crash.
With GCC 4.5.3 there is a beqz that tests for &__dso_handle == NULL:

000537d0 <old_atexit>:
  537d0:       3c1c0003        lui     gp,0x3
  537d4:       279c8d10        addiu   gp,gp,-29424
  537d8:       0399e021        addu    gp,gp,t9
  537dc:       8f828a2c        lw      v0,-30164(gp)
  537e0:       8f9989ac        lw      t9,-30292(gp)
  537e4:       8c460000        lw      a2,0(v0)
  ^^^^^^^^^^ SIGSEGV here, with a NULL dereference
  537e8:       00002821        move    a1,zero
  537ec:       03200008        jr      t9
  537f0:       0002300a        movz    a2,zero,v0

old_atexit.os with GCC 4.6.1:
       .file   1 "old_atexit.c"
       .section .mdebug.abi32
       .previous
       .gnu_attribute 4, 3
       .abicalls
       .text
       .align  2
       .globl  old_atexit
       .set    nomips16
       .ent    old_atexit
       .type   old_atexit, @function
old_atexit:
       .frame  $sp,0,$31               # vars= 0, regs= 0/0, args= 0, gp= 0
       .mask   0x00000000,0
       .fmask  0x00000000,0
       .set    noreorder
       .cpload $25
       .set    nomacro
       lw      $2,%got(__dso_handle)($28)
       lw      $25,%call16(__cxa_atexit)($28)
       lw      $6,0($2)
       move    $5,$0
       .reloc  1f,R_MIPS_JALR,__cxa_atexit
1:      jr      $25
       movz    $6,$0,$2

       .set    macro
       .set    reorder
       .end    old_atexit
       .size   old_atexit, .-old_atexit
       .weak   atexit
       atexit = old_atexit
       .weak   __dso_handle
       .ident  "GCC: (GNU) 4.6.1"

old_atexit.os with GCC 4.5.3:
       .file   1 "old_atexit.c"
       .section .mdebug.abi32
       .previous
       .gnu_attribute 4, 3
       .abicalls
       .text
       .align  2
       .globl  old_atexit
       .set    nomips16
       .ent    old_atexit
       .type   old_atexit, @function
old_atexit:
       .frame  $sp,0,$31               # vars= 0, regs= 0/0, args= 0, gp= 0
       .mask   0x00000000,0
       .fmask  0x00000000,0
       .set    noreorder
       .cpload $25
       .set    nomacro
       lw      $2,%got(__dso_handle)($28)
       beq     $2,$0,$L2
       move    $6,$0

       lw      $6,0($2)
$L2:
       lw      $25,%call16(__cxa_atexit)($28)
       .reloc  1f,R_MIPS_JALR,__cxa_atexit
1:      jr      $25
       move    $5,$0

       .set    macro
       .set    reorder
       .end    old_atexit
       .size   old_atexit, .-old_atexit
       .weak   atexit
       atexit = old_atexit
       .weak   __dso_handle
       .ident  "GCC: (GNU) 4.5.3"

### GCC versions (with gcc -v output) for target and host:

$ /mnt/sdb3/freetz/freetz-trunk_gcc-4.6.1/toolchain/build/mipsel_gcc-4.6.1_uClibc-0.9.32/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc -v
Using built-in specs.
COLLECT_GCC=/mnt/sdb3/freetz/freetz-trunk_gcc-4.6.1/toolchain/build/mipsel_gcc-4.6.1_uClibc-0.9.32/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc
COLLECT_LTO_WRAPPER=/mnt/sdb3/freetz/freetz-trunk_gcc-4.6.1/toolchain/build/mipsel_gcc-4.6.1_uClibc-0.9.32/mipsel-linux-uclibc/bin/../libexec/gcc/mipsel-linux-uclibc/4.6.1/lto-wrapper
Target: mipsel-linux-uclibc
Configured with: /mnt/sdb3/freetz/freetz-trunk/source/toolchain-mipsel_gcc-4.6.1_uClibc-0.9.32/gcc-4.6.1/configure --prefix=/mnt/sdb3/freetz/freetz-trunk/toolchain/build/mipsel_gcc-4.6.1_uClibc-0.9.32/mipsel-linux-uclibc --with-sysroot=/mnt/sdb3/freetz/freetz-trunk/toolchain/build/mipsel_gcc-4.6.1_uClibc-0.9.32/mipsel-linux-uclibc/usr/ --build=i386-pc-linux-gnu --host=i386-pc-linux-gnu --target=mipsel-linux-uclibc --enable-languages=c,c++ --enable-shared --enable-threads --with-gmp=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-mpfr=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-mpc=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-gnu-ld --disable-__cxa_atexit --disable-libgomp --disable-libmudflap --disable-multilib --disable-tls --disable-fixed-point --with-float=soft --enable-cxx-flags=-msoft-float --disable-libssp --with-march=4kc --disable-nls --with-mips-plt --disable-decimal-float
Thread model: posix
gcc version 4.6.1 (GCC)

$ /mnt/sdb3/freetz/freetz-trunk_gcc-4.5.3/toolchain/build/mipsel_gcc-4.5.3_uClibc-0.9.32/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc -v
Using built-in specs.
COLLECT_GCC=/mnt/sdb3/freetz/freetz-trunk_gcc-4.5.3/toolchain/build/mipsel_gcc-4.5.3_uClibc-0.9.32/mipsel-linux-uclibc/bin/mipsel-linux-uclibc-gcc
COLLECT_LTO_WRAPPER=/mnt/sdb3/freetz/freetz-trunk_gcc-4.5.3/toolchain/build/mipsel_gcc-4.5.3_uClibc-0.9.32/mipsel-linux-uclibc/bin/../libexec/gcc/mipsel-linux-uclibc/4.5.3/lto-wrapper
Target: mipsel-linux-uclibc
Configured with: /mnt/sdb3/freetz/freetz-trunk/source/toolchain-mipsel_gcc-4.5.3_uClibc-0.9.32/gcc-4.5.3/configure --prefix=/mnt/sdb3/freetz/freetz-trunk/toolchain/build/mipsel_gcc-4.5.3_uClibc-0.9.32/mipsel-linux-uclibc --with-sysroot=/mnt/sdb3/freetz/freetz-trunk/toolchain/build/mipsel_gcc-4.5.3_uClibc-0.9.32/mipsel-linux-uclibc/usr/ --build=i386-pc-linux-gnu --host=i386-pc-linux-gnu --target=mipsel-linux-uclibc --enable-languages=c,c++ --enable-shared --enable-threads --with-gmp=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-mpfr=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-mpc=/mnt/sdb3/freetz/freetz-trunk/tools/build --with-gnu-ld --disable-__cxa_atexit --disable-libgomp --disable-libmudflap --disable-multilib --disable-tls --disable-fixed-point --with-float=soft --enable-cxx-flags=-msoft-float --disable-libssp --with-march=4kc --disable-nls --with-mips-plt --disable-decimal-float
Thread model: posix
gcc version 4.5.3 (GCC)

$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/i386-linux-gnu/gcc/i486-linux-gnu/4.6.1/lto-wrapper
Target: i486-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 4.6.1-1' --with-bugurl=file:///usr/share/doc/gcc-4.6/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++,go --prefix=/usr --program-suffix=-4.6 --enable-shared --enable-multiarch --with-multiarch-defaults=i386-linux-gnu --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib/i386-linux-gnu --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.6 --libdir=/usr/lib/i386-linux-gnu --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-plugin --enable-objc-gc --enable-targets=all --with-arch-32=i586 --with-tune=generic --enable-checking=release --build=i486-linux-gnu --host=i486-linux-gnu --target=i486-linux-gnu
Thread model: posix
gcc version 4.6.1 (Debian 4.6.1-1)

### Target is a Speedport W701V router:

# uname -a
Linux fritz.fonwlan.box 2.6.13.1-ohio #1 Thu Jun 30 17:59:33 CEST 2011 mips GNU/Linux
# cat /proc/version 
Linux version 2.6.13.1-ohio () (gcc version 3.4.6) #1 Thu Jun 30 17:59:33 CEST 2011

### Host is a Debian/sid i386 system:

$ uname -a
Linux seduxbox 2.6.39-2-686-pae #1 SMP Wed Jun 8 11:33:14 UTC 2011 i686 GNU/Linux
$ cat /proc/version 
Linux version 2.6.39-2-686-pae (Debian 2.6.39-2) (ben@decadent.org.uk) (gcc version 4.4.6 (Debian 4.4.6-3) ) #1 SMP Wed Jun 8 11:33:14 UTC 2011

Hope this helps to kill that BUG.

Kind Regards,
- Sedat -
Comment 1 Thorsten Glaser 2011-07-03 14:58:47 UTC
Ouch. I can see where it might be NULL… but in anal-C99 taking a pointer of
an object (&__dso_handle) can never be a NULL pointer, so GCC might, in a
very theoretical sense, be correct in optimising away this check.

Does adding -fno-delete-null-pointer-checks to the compile options help?
Comment 2 Török Edwin 2011-07-03 15:07:02 UTC
(In reply to comment #1)
> Ouch. I can see where it might be NULL… but in anal-C99 taking a pointer of
> an object (&__dso_handle) can never be a NULL pointer, so GCC might, in a
> very theoretical sense, be correct in optimising away this check.
> 
> Does adding -fno-delete-null-pointer-checks to the compile options help?

No, with -fno-delete-null-pointer-checks the 'beq' instruction is still deleted.

-O1 works though.
Comment 3 Sedat Dilek 2011-07-03 15:39:02 UTC
Short feedback: -O{0,1,2,} works (beq* is there), -Os doesn't (beq deleted).

[ -O0 ]
    beq    $2,$0,$L2

[ -O1 ]
    beql    $2,$0,$L3

[ -O2 ]
    beql    $2,$0,$L2

[ -Os ]
[ empty ]
Comment 4 Sedat Dilek 2011-07-03 17:46:07 UTC
With the Patch from [1] I could boot a firmware-image on my router.

[1] http://freetz.org/attachment/ticket/1310/gcc-4.6.x-disable-if-conversion.patch
Comment 5 Andrew Pinski 2011-07-03 19:27:08 UTC
__dso_handle should be marked as weak in the source.
Comment 6 Andrew Pinski 2011-07-03 19:28:08 UTC
Hmm, it is already marked as weak.
Comment 7 Eugene Rudoy 2011-07-03 20:09:42 UTC
(In reply to comment #4)
> With the Patch from [1] I could boot a firmware-image on my router.
It seems the "if-conversion"-opt is not the actual reason. It does solve the issue, but it's not the reason. Explanation: "if-conversion" is enabled by default at -O1. The code is however not miss-compiled when compiled using -O1 (-Os causes it to be miss-compiled).

I found out that disabling gcse and cse-follow-jumps (i.e. passing -fno-gcse -fno-cse-follow-jumps [1]) also solves the issue. As the optimization level (-O2/-Os) these both are by default enabled on correspond to the -Os flag we use I consider them to be the actual reason for the miss-compiling.

[1] http://freetz.org/attachment/ticket/1310/gcc-4.6.x-disable-opt-flags.patch
Comment 8 Sedat Dilek 2011-07-03 20:47:37 UTC
Confirmed... Image built with gcc-4.6.x-disable-opt-flags.patch works also fine.
Comment 9 Jakub Jelinek 2011-07-04 08:10:19 UTC
I think the problem is either that the MEM has the MEM_NOTRAP_P flag set on it:
(mem/f/i:SI (symbol_ref/i:SI ("__dso_handle") <var_decl 0x7ffff13ce320 __dso_handle>) [0 S4 A32])
That makes ifcvt believe it can try to use a conditional move with that as an operand, and as there is no conditional move with MEM operand on this target, it is forced into register first.

MEM_NOTRAP_P is set in set_mem_attributes_minus_bitpos:
  /* Note whether this expression can trap.  */
  MEM_NOTRAP_P (ref) = !tree_could_trap_p (t);
where t is VAR_DECL for __dso_handle with DECL_WEAK.

Or the problem might be that noce_try_cmove_arith should be using
  else if (may_trap_or_fault_p (a) || may_trap_or_fault_p (b))
    return FALSE;
instead of
  else if (may_trap_p (a) || may_trap_p (b))
    return FALSE;

Eric/Richard, what do you think?  The comments seem to be fuzzy to me, some comments say that MEM_NOTRAP_P only apply to the position where the MEM is originally used (and in this case the __dso_handle var is read only guarded with if (__dso_handle != NULL), on the other side I wonder if we ever clear MEM_NOTRAP_P on unguarded DECL_WEAK var references.
Comment 10 Eric Botcazou 2011-07-04 10:14:40 UTC
> Eric/Richard, what do you think?  The comments seem to be fuzzy to me, some
> comments say that MEM_NOTRAP_P only apply to the position where the MEM is
> originally used (and in this case the __dso_handle var is read only guarded
> with if (__dso_handle != NULL), on the other side I wonder if we ever clear
> MEM_NOTRAP_P on unguarded DECL_WEAK var references.

I think that DECL_WEAK should always prevent MEM_NOTRAP_P from being set.  This is a regression because of the change in set_mem_attributes_minus_bitpos, right?

The code on the 4.5 branch reads:

      if (DECL_P (base))
	{
	  if (CODE_CONTAINS_STRUCT (TREE_CODE (base), TS_DECL_WITH_VIS))
	    MEM_NOTRAP_P (ref) = !DECL_WEAK (base);
	  else
	    MEM_NOTRAP_P (ref) = 1;
	}
      else
	MEM_NOTRAP_P (ref) = TREE_THIS_NOTRAP (base);

Maybe the first part should be reinstated.
Comment 11 Jakub Jelinek 2011-07-04 12:38:03 UTC
Created attachment 24673 [details]
gcc47-pr49618.patch

Untested 4.7 fix.
Comment 12 Jakub Jelinek 2011-07-04 12:39:21 UTC
Created attachment 24674 [details]
gcc46-pr49618.patch

Untested more conservative 4.6 fix.
Comment 13 Sedat Dilek 2011-07-04 14:29:27 UTC
Confirmed... gcc46-pr49618.patch solves the problem here, workaround patches from freetz project are no more necessary.
Comment 14 Sedat Dilek 2011-07-04 14:38:05 UTC
Thank you gcc folks and all involved people to fix this bug.
Comment 15 Sedat Dilek 2011-07-04 16:47:25 UTC
Created attachment 24677 [details]
Simple testcase-script for gcc-PR49618
Comment 16 Sedat Dilek 2011-07-04 16:48:15 UTC
Created attachment 24678 [details]
old_atexit.os_OptLevel-O0
Comment 17 Sedat Dilek 2011-07-04 16:48:44 UTC
Created attachment 24679 [details]
old_atexit.os_OptLevel-O1
Comment 18 Sedat Dilek 2011-07-04 16:49:13 UTC
Created attachment 24680 [details]
old_atexit.os_OptLevel-O2
Comment 19 Sedat Dilek 2011-07-04 16:49:44 UTC
Created attachment 24681 [details]
old_atexit.os_OptLevel-O3
Comment 20 Sedat Dilek 2011-07-04 16:50:10 UTC
Created attachment 24682 [details]
old_atexit.os_OptLevel-Os
Comment 21 Jakub Jelinek 2011-07-05 18:43:07 UTC
Author: jakub
Date: Tue Jul  5 18:43:04 2011
New Revision: 175884

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175884
Log:
	PR tree-optimization/49618
	* tree-eh.c (tree_could_trap_p) <case CALL_EXPR>: For DECL_WEAK
	t recurse on the decl.
	<case FUNCTION_DECL, case VAR_DECL>: For DECL_WEAK decls
	return true if expr isn't known to be defined in current
	TU or some other LTO partition.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-eh.c
Comment 22 Jakub Jelinek 2011-07-05 18:44:35 UTC
Author: jakub
Date: Tue Jul  5 18:44:32 2011
New Revision: 175885

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175885
Log:
	PR tree-optimization/49618
	* tree-eh.c (tree_could_trap_p) <case FUNCTION_DECL, case VAR_DECL>:
	For DECL_WEAK decls return true.

Modified:
    branches/gcc-4_6-branch/gcc/ChangeLog
    branches/gcc-4_6-branch/gcc/tree-eh.c
Comment 23 Jakub Jelinek 2011-07-05 19:12:01 UTC
Should be fixed now.