Bug 58595 - internal compiler error: in gen_movsi when compiling on arm some files of lttng-tools with -fPIE
Summary: internal compiler error: in gen_movsi when compiling on arm some files of ltt...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.8.1
: P3 normal
Target Milestone: 4.8.3
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2013-10-02 19:11 UTC by Yannick Brosseau
Modified: 2014-04-10 08:03 UTC (History)
2 users (show)

See Also:
Host:
Target: arm-linux-gnueabihf
Build:
Known to work:
Known to fail: 4.7.2, 4.8.0, 4.8.1, 4.9.0
Last reconfirmed: 2013-10-31 00:00:00


Attachments
Preprocessed problematic file main.i (82.01 KB, text/plain)
2013-10-02 19:11 UTC, Yannick Brosseau
Details
reduced testcase. (598 bytes, text/x-csrc)
2013-10-31 10:00 UTC, Ramana Radhakrishnan
Details
gcc49-pr58595.patch (802 bytes, patch)
2014-03-03 13:40 UTC, Jakub Jelinek
Details | Diff
Work in progress patch. (420 bytes, patch)
2014-03-03 15:47 UTC, Meador Inge
Details | Diff
gcc49-pr58595-2.patch (1.38 KB, patch)
2014-03-03 16:54 UTC, Jakub Jelinek
Details | Diff
gcc49-pr58595.patch (1.38 KB, text/plain)
2014-03-04 13:13 UTC, Jakub Jelinek
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yannick Brosseau 2013-10-02 19:11:55 UTC
Created attachment 30947 [details]
Preprocessed problematic file main.i

When I try to compile the attached file on arm, I get this error messaage:

main.c: In function ‘thread_manage_kernel’:
main.c:886:1: internal compiler error: in gen_movsi, at config/arm/arm.md:5539
 }

I compile with:
gcc -fPIE -O2 -c main.i
if I remove -fPIE, it compiles correctly. 

Using gcc 3.8.1 on fedora. 
Target: armv7hl-redhat-linux-gnueabi
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-bootstrap --enable-shared --enable-threads=posix --enable-checking=release --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-linker-hash-style=gnu --enable-languages=c,c++,objc,obj-c++,java,fortran,go,lto --enable-plugin --enable-initfini-array --enable-java-awt=gtk --disable-dssi --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre --enable-libgcj-multifile --enable-java-maintainer-mode --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --disable-libjava-multilib --with-isl=/builddir/build/BUILD/gcc-4.8.1-20130603/obj-armv7hl-redhat-linux-gnueabi/isl-install --with-cloog=/builddir/build/BUILD/gcc-4.8.1-20130603/obj-armv7hl-redhat-linux-gnueabi/cloog-install --disable-sjlj-exceptions --with-cpu=cortex-a8 --with-tune=cortex-a8 --with-arch=armv7-a --with-float=hard --with-fpu=vfpv3-d16 --with-abi=aapcs-linux --build=armv7hl-redhat-linux-gnueabi
Thread model: posix
gcc version 4.8.1 20130603 (Red Hat 4.8.1-1) (GCC) 

I've put some info on the lttng tracker: http://bugs.lttng.org/issues/640


Also reproduced with gcc 4.7.2 (Arch Linux)
 Target: armv6l-unknown-linux-gnueabihf
Configured with: /build/src/gcc-4.7.2/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,fortran,lto,objc,obj-c++ --enable-shared --enable-threads=posix --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch --enable-libstdcxx-time --enable-gnu-unique-object --enable-linker-build-id --with-ppl --enable-cloog-backend=isl --disable-ppl-version-check --disable-cloog-version-check --enable-lto --enable-gold --enable-ld=default --enable-plugin --with-plugin-ld=ld.gold --with-linker-hash-style=gnu --disable-multilib --disable-libssp --disable-build-with-cxx --disable-build-poststage1-with-cxx --enable-checking=release --host=armv6l-unknown-linux-gnueabihf --build=armv6l-unknown-linux-gnueabihf --with-arch=armv6 --with-float=hard --with-fpu=vfp
Thread model: posix
Comment 1 Ramana Radhakrishnan 2013-10-31 10:00:53 UTC
Created attachment 31116 [details]
reduced testcase.
Comment 2 Meador Inge 2014-02-21 19:13:18 UTC
I hit this problem as well.  Here is a much reduced test case:

$ cat repro.c                                                                       
struct state {                                                                  
 unsigned long last;                                                            
 unsigned long current;                                                         
};                                                                              
                                                                                
__thread struct state state;                                                    
                                                                                
void foo()                                                                      
{                                                                               
  while (1) {                                                                   
    __sync_add_and_fetch_4(&state.current, (long) 1);                           
  }                                                                             
}                                                                               
$ ./install/bin/arm-none-linux-gnueabi-gcc --version                        
arm-none-linux-gnueabi-gcc (GCC) 4.9.0 20140219 (experimental)                  
Copyright (C) 2014 Free Software Foundation, Inc.                               
This is free software; see the source for copying conditions.  There is NO      
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.     
                                                                                
meadori@build1-lucid-cs:/scratch/meadori/arm-none-linux-gnueabi-fsf-mainline$ ./install/bin/arm-none-linux-gnueabi-gcc -fPIC -O2 repro.c                        
repro.c: In function 'foo':                                                     
repro.c:13:1: internal compiler error: in gen_movsi, at config/arm/arm.md:6272  
 }                                                                              
 ^                                                                              
0x8b3ee69 gen_movsi(rtx_def*, rtx_def*)                                         
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/config/arm/arm.md:6272                                                        
0x838853f insn_gen_fn::operator()(rtx_def*, rtx_def*) const                     
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/recog.h:284                                                                   
0x844b2be emit_move_insn_1(rtx_def*, rtx_def*)                                  
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/expr.c:3556                                                                   
0x844b6f1 emit_move_insn(rtx_def*, rtx_def*)                                    
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/expr.c:3650                                                                   
0x8508d4a process_insert_insn                                                   
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:2133                                                                   
0x85091f4 pre_edge_insert                                                       
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:2310                                                                   
0x8509c2a pre_gcse                                                              
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:2641                                                                   
0x8509d8a one_pre_gcse_pass                                                     
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:2696                                                                   
0x850caa4 execute_rtl_pre                                                       
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:4127                                                                   
0x850cba8 execute                                                               
        /scratch/meadori/arm-none-linux-gnueabi-fsf-mainline/src/gcc-mainline/gcc/gcse.c:4186                                                                   
Please submit a full bug report,                                                
with preprocessed source if appropriate.                                        
Please include the complete backtrace with any bug report.                      
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 3 Meador Inge 2014-02-21 19:16:46 UTC
As shown in the backtrace, the ICE occurs when GCSE tries generate a move.
The the following asserts fire in 'gen_movsi':

  gcc_assert (GET_CODE (tmp) == SYMBOL_REF);                                
  gcc_assert (SYMBOL_REF_TLS_MODEL (tmp) != 0);

This is because the move operands look like:

  (gdb) p debug_rtx(operands[0])
  (reg/f:SI 115)

  (gdb) p debug_rtx(operands[1])
  (mem/u/c:SI (plus:SI (reg:SI 111)
          (unspec:SI [
                  (symbol_ref:SI ("state") [flags 0x90] <var_decl 0xf7c50114 state>)
              ] UNSPEC_PIC_SYM)) [0  S4 A32])

We aren't expecting the 'mem' expression with the TLS reference in side.

I am not sure if this is just an overzealous assert or we have a deeper
issue.  Removing the asserts does allow the code to compile.
Comment 4 Jakub Jelinek 2014-03-03 13:40:29 UTC
Created attachment 32252 [details]
gcc49-pr58595.patch

Untested fix.  While the testcase does compile with -O2 -fpic -fno-gcse, I don't see how it could work,
        ldr     r3, .L5
        ldr     r2, .L5+4
.LPIC0:
        add     r3, pc, r3
        ldr     r4, [r3, r2]
        add     r4, r4, #4
...
.L5:
        .word   _GLOBAL_OFFSET_TABLE_-(.LPIC0+8)
        .word   s(GOT)
simply can't load correct TLS symbol s value when s is in .tbss.

This untested patch does roughly what i386.c does, call legitimize_tls_address even when we see a constant addition to a TLS SYMBOL_REF.
Comment 5 Meador Inge 2014-03-03 15:47:04 UTC
Created attachment 32253 [details]
Work in progress patch.

Yeah, I am came to the same conclusion after making that comment that removing the asserts is a bogus approach.

Although, my follow up approach is a little different.  I was trying to replicate what we currently have in the 'movsi' pattern (see attached).  This seems to work for the basic tests I have tried.  This patch is lightly tested, is obviously in need of cleanup (since it duplicates code), and doesn't cover the thumb case.  I am just posting it for discussion purposes.

Are we guaranteed to always have the const plus form?  Or do we need to be more general like in the patch I attached?
Comment 6 Jakub Jelinek 2014-03-03 16:00:43 UTC
Kyrill (or anyone else), do you think you could bootstrap/regtest that (sorry for that, but my access to ARM hw is limited)?
Comment 7 Jakub Jelinek 2014-03-03 16:12:31 UTC
(In reply to Meador Inge from comment #5)
> Created attachment 32253 [details]
> Work in progress patch.
> 
> Yeah, I am came to the same conclusion after making that comment that
> removing the asserts is a bogus approach.
> 
> Although, my follow up approach is a little different.  I was trying to
> replicate what we currently have in the 'movsi' pattern (see attached). 
> This seems to work for the basic tests I have tried.  This patch is lightly
> tested, is obviously in need of cleanup (since it duplicates code), and
> doesn't cover the thumb case.  I am just posting it for discussion purposes.
> 
> Are we guaranteed to always have the const plus form?  Or do we need to be
> more general like in the patch I attached?

It is good enough for i?86/x86_64, so I don't see why it wouldn't be for arm.
Note, this is about what the middle-end is prepared to feed to emit_move_insn, I can't imagine it would feed something more complex than that.

rs6000 has code similar to your case and has the assert in there too.

Another alternative would be to have something like your patch, but do the check before the
if (!TARGET_ARM)
case in arm_legitimize_address and drop the TLS handling in thumb_legitimize_address to avoid the duplication.  I wonder how it can work on thumb2 anyway without legitimizing TLS.
BTW, the force_operand is perhaps unnecessary, but perhaps we should recurse into arm_legitimize_address instead, so that it takes care of out of range addends?
Comment 8 Jakub Jelinek 2014-03-03 16:54:45 UTC
Created attachment 32254 [details]
gcc49-pr58595-2.patch

So what about this variant?
Comment 9 ktkachov 2014-03-03 17:15:57 UTC
(In reply to Jakub Jelinek from comment #8)
> Created attachment 32254 [details]
> gcc49-pr58595-2.patch
> 
> So what about this variant?

I'll give this a bootstrap and test run, thanks
Comment 10 ktkachov 2014-03-04 10:15:32 UTC
(In reply to ktkachov from comment #9)
> (In reply to Jakub Jelinek from comment #8)
> > Created attachment 32254 [details]
> > gcc49-pr58595-2.patch
> > 
> > So what about this variant?
> 
> I'll give this a bootstrap and test run, thanks

This patch passes bootstrap but fails regtest:

$SOURCE/gcc/testsuite/gcc.dg/tls/opt-12.c:24:28: internal compiler error: in arm_legitimize_address, at config/arm/arm.c:7348
0x616d27 arm_legitimize_address
        $SOURCE/gcc/config/arm/arm.c:7348
0x245d17 memory_address_addr_space(machine_mode, rtx_def*, unsigned char)
        $SOURCE/gcc/explow.c:474
0x2576e9 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        $SOURCE/gcc/expr.c:9678
0x2658bd expand_expr
        $SOURCE/gcc/expr.h:455
0x2658bd expand_assignment(tree_node*, tree_node*, bool)
        $SOURCE/gcc/expr.c:5054
0x1bff1f expand_gimple_stmt_1
        $SOURCE/gcc/cfgexpand.c:3214
0x1bff1f expand_gimple_stmt
        $SOURCE/gcc/cfgexpand.c:3310
0x1c13cb expand_gimple_basic_block
        $SOURCE/gcc/cfgexpand.c:5150
0x1c2873 gimple_expand_cfg
        $SOURCE/gcc/cfgexpand.c:5729
0x1c2873 execute
        $SOURCE/gcc/cfgexpand.c:5949
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

FAIL: gcc.dg/tls/opt-12.c (internal compiler error)

In particular, this assertion fails in arm_legitimize_address:
      gcc_assert (GET_CODE (x) == SYMBOL_REF);
Comment 11 Jakub Jelinek 2014-03-04 13:13:46 UTC
Created attachment 32260 [details]
gcc49-pr58595.patch

Ah, thanks.  The problem is that this was matched as TARGET_MEM_REF, which can be fairly complex.  Just return x; in that case will result in the right thing in the callers.
Comment 12 Meador Inge 2014-03-04 21:01:46 UTC
The latest patch LGTM.  I don't currently have ARM hardware setup for a bootstrap, but I did run all the GCC and GLIBC tests for the default options and -mthumb via QEMU.  I saw no regressions.
Comment 13 ktkachov 2014-03-05 08:59:02 UTC
This patch bootstrapped on a Chromebook and passed regtest fine for me.
Comment 14 Jakub Jelinek 2014-03-06 12:07:40 UTC
Author: jakub
Date: Thu Mar  6 12:07:07 2014
New Revision: 208380

URL: http://gcc.gnu.org/viewcvs?rev=208380&root=gcc&view=rev
Log:
	PR target/58595
	* config/arm/arm.c (arm_tls_symbol_p): Remove.
	(arm_legitimize_address): Call legitimize_tls_address for any
	arm_tls_referenced_p expression, handle constant addend.  Call it
	before testing for !TARGET_ARM.
	(thumb_legitimize_address): Don't handle arm_tls_symbol_p here.

	* gcc.dg/tls/pr58595.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/tls/pr58595.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.c
    trunk/gcc/testsuite/ChangeLog
Comment 15 Jakub Jelinek 2014-04-10 07:45:58 UTC
Author: jakub
Date: Thu Apr 10 07:45:21 2014
New Revision: 209262

URL: http://gcc.gnu.org/viewcvs?rev=209262&root=gcc&view=rev
Log:
	Backport from mainline
	2014-03-06  Jakub Jelinek  <jakub@redhat.com>
		    Meador Inge  <meadori@codesourcery.com>

	PR target/58595
	* config/arm/arm.c (arm_tls_symbol_p): Remove.
	(arm_legitimize_address): Call legitimize_tls_address for any
	arm_tls_referenced_p expression, handle constant addend.  Call it
	before testing for !TARGET_ARM.
	(thumb_legitimize_address): Don't handle arm_tls_symbol_p here.

	2014-03-06  Jakub Jelinek  <jakub@redhat.com>

	PR target/58595
	* gcc.dg/tls/pr58595.c: New test.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/tls/pr58595.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/arm/arm.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog
Comment 16 Jakub Jelinek 2014-04-10 08:03:31 UTC
Fixed for 4.8.3+.