Bug 86832 - [8 Regression] GCC v8.2.0 tries to use native TLS with -fstack-protector-strong on Windows (mingw-w64)
Summary: [8 Regression] GCC v8.2.0 tries to use native TLS with -fstack-protector-stro...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 8.2.0
: P2 normal
Target Milestone: 8.3
Assignee: Jakub Jelinek
URL:
Keywords: wrong-code
: 86929 (view as bug list)
Depends on: 85644
Blocks:
  Show dependency treegraph
 
Reported: 2018-08-02 16:30 UTC by Johannes Schindelin
Modified: 2019-01-08 10:43 UTC (History)
9 users (show)

See Also:
Host:
Target: x86_64-w64-mingw32
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-08-13 00:00:00


Attachments
gcc9-pr85644.patch (902 bytes, patch)
2018-11-21 17:18 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Johannes Schindelin 2018-08-02 16:30:44 UTC
When I try to compile this program:

-- snip --
static void a1(void *p) { }

int main(int argc, char **argv) {
    int i;
    a1(&i);
    return 0;
}
-- snap --

using

    gcc -o a1.o -c -fstack-protector-strong a1.c
    gcc -fstack-protector-strong -o a1.exe a1.o

the resulting a1.exe causes a segmentation fault in `main()`. The offending assembler instruction is this:

0x40156f <main+20>: mov %fs:0x0,%rax

The good mingw-w64 people pointed out that this looks like native TLS, but mingw-w64 only supports emulated TLS.

When compiling without -fstack-protector-strong, everything works.

Output of `gcc -v`:

Using built-in specs.
COLLECT_GCC=C:\git-sdk-64-ci\mingw64\bin\gcc.exe
COLLECT_LTO_WRAPPER=C:/git-sdk-64-ci/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.2.0/lto-wrapper.exe
Target: x86_64-w64-mingw32
Configured with: ../gcc-8.2.0/configure --prefix=/mingw64 --with-local-prefix=/mingw64/local --build=x86_64-w64-mingw32 --host=x86_64-w64-mingw32 --target=x86_64-w64-mingw32 --with-native-system-header-dir=/mingw64/x86_64-w64-mingw32/include --libexecdir=/mingw64/lib --enable-bootstrap --with-arch=x86-64 --with-tune=generic --enable-languages=ada,c,lto,c++,objc,obj-c++,fortran --enable-shared --enable-static --enable-libatomic --enable-threads=posix --enable-graphite --enable-fully-dynamic-string --enable-libstdcxx-filesystem-ts=yes --enable-libstdcxx-time=yes --disable-libstdcxx-pch --disable-libstdcxx-debug --disable-isl-version-check --enable-lto --enable-libgomp --disable-multilib --enable-checking=release --disable-rpath --disable-win32-registry --disable-nls --disable-werror --disable-symvers --with-libiconv --with-system-zlib --with-gmp=/mingw64 --with-mpfr=/mingw64 --with-mpc=/mingw64 --with-isl=/mingw64 --with-pkgversion='Rev1, Built by MSYS2 project' --with-bugurl=https://sourceforge.net/projects/msys2 --with-gnu-as --with-gnu-ld
Thread model: posix
gcc version 8.2.0 (Rev1, Built by MSYS2 project)
Comment 1 Andrew Pinski 2018-08-02 17:52:32 UTC
Related to bug 85644.
Comment 2 Andrew Pinski 2018-08-13 06:27:51 UTC
*** Bug 86929 has been marked as a duplicate of this bug. ***
Comment 3 Andrew Pinski 2018-08-13 06:29:10 UTC
.
Comment 5 John Warburton 2018-10-28 10:21:55 UTC
Related bug in other software:

https://gitlab.com/mbunkus/mkvtoolnix/issues/2417

(it also affects cross-compilation of the VLC player in addition to the MKVToolNix suite, above)
Comment 6 Jakub Jelinek 2018-11-21 17:18:00 UTC
Created attachment 45061 [details]
gcc9-pr85644.patch

Untested fix.  I have no access to Windows though.
Comment 7 Jakub Jelinek 2018-11-21 17:36:49 UTC
I believe this is caused by the PR81708 changes.
While i386 defaulted to SSP_TLS rather than SSP_GLOBAL on everything but Android,
the -mstack-protector-guard= switch controlled pretty much whether the i386.md special stack protector patterns are used (if tls) or whether generic code is used (global).  These special stack protector patterns did one thing if
TARGET_THREAD_SSP_OFFSET macro was defined (only defined on glibc targets) - code like:
        movq    %fs:40, %rax
        movq    %rax, -8(%rbp)
        xorl    %eax, %eax
in the prologue and
        movq    -8(%rbp), %rdx
        xorq    %fs:40, %rdx
        je      .L4
in the epilogue.  If TARGET_THREAD_SSP_OFFSET macro wasn't defined, it would do
instead:
        movq    .refptr.__stack_chk_guard(%rip), %rax
        movq    (%rax), %rcx
        movq    %rcx, -8(%rbp)
        xorl    %ecx, %ecx
and
        movq    .refptr.__stack_chk_guard(%rip), %rdx
        movq    -8(%rbp), %rcx
        xorq    (%rdx), %rcx
        je      .L4
(this is taken from 7.x cross to mingw).
Finally, for Android or when -mstack-protector-guard=global was used, it emitted:
        movq    __stack_chk_guard(%rip), %rax
        movq    %rax, -8(%rbp)
and
        movq    __stack_chk_guard(%rip), %rdx
        cmpq    %rdx, %rcx
        je      .L4
Note, apart from OS specific details, those =global sequences are similar to the =tls ones when TARGET_THREAD_SSP_OFFSET is not defined, the main difference is that the =tls ones are more secure as they clear registers containing the guard as quickly as possible.  The PR81708 changes dropped the non-tls special stack_protector_* patterns from i386.md and now =tls implies really tls, but the default remained, so mingw32 or darwin still default to tls and just use 0 offset by default.

So, this patch changes the default for mingw32, darwin and everything else except gnu-user*.h to be =global, and just forces those special i386.md more secure patterns unconditionally (slightly changing the generated code on Android, but it is one extra insn in prologue and one fewer in the epilogue).
Comment 8 Jakub Jelinek 2018-11-21 17:40:02 UTC
As a workaround, you can use -mstack-protector-guard=global on these targets (together with -fstack-protector{,-all,-strong,-explicit}).
Comment 9 Jakub Jelinek 2018-11-22 09:49:34 UTC
Author: jakub
Date: Thu Nov 22 09:48:43 2018
New Revision: 266370

URL: https://gcc.gnu.org/viewcvs?rev=266370&root=gcc&view=rev
Log:
	PR target/85644
	PR target/86832
	* config/i386/i386.c (ix86_option_override_internal): Default
	ix86_stack_protector_guard to SSP_TLS only if TARGET_THREAD_SSP_OFFSET
	is defined.
	* config/i386/i386.md (stack_protect_set, stack_protect_set_<mode>,
	stack_protect_test, stack_protect_test_<mode>): Use empty condition
	instead of TARGET_SSP_TLS_GUARD.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.md
Comment 10 Jakub Jelinek 2018-11-22 10:09:40 UTC
Can somebody please test this on mingw (on the trunk)? Thanks.
Comment 11 John Warburton 2018-11-23 11:27:49 UTC
On Thu, Nov 22, 2018 at 10:09 AM jakub at gcc dot gnu.org
<gcc-bugzilla@gcc.gnu.org> wrote:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86832
>
> --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Can somebody please test this on mingw (on the trunk)? Thanks.

I've started a compilation of the trunk on GNU/Linux outputting
x86_64-w64-mingw32-gcc etc., and will then cross-compile a fairly
complex program that uses the flag that caused trouble. Or, at least,
will try. I normally use gcc-8-branch, not trunk.

JW
Comment 12 John Warburton 2018-11-23 15:32:19 UTC
On Fri, Nov 23, 2018 at 11:26 AM John Warburton <john@johnwarburton.net> wrote:
>
> On Thu, Nov 22, 2018 at 10:09 AM jakub at gcc dot gnu.org
> <gcc-bugzilla@gcc.gnu.org> wrote:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86832
> >
> > --- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> > Can somebody please test this on mingw (on the trunk)? Thanks.
>
> I've started a compilation of the trunk on GNU/Linux outputting
> x86_64-w64-mingw32-gcc etc.

This happens during the compilation of the trunk, I'm afraid:

 ../../../../src/gcc-trunk/libgfortran/generated/matmul_i4.c: In
function 'matmul_i4_avx2':
 ../../../../src/gcc-trunk/libgfortran/generated/matmul_i4.c:1210:1:
error: insn outside basic block
  1210 | }
       | ^
 (insn 10250 8120 10251 (parallel [
             (set (reg:DI 5752)
                 (plus:DI (reg/f:DI 19 frame)
                     (const_int -1200 [0xfffffffffffffb50])))
             (clobber (reg:CC 17 flags))
         ]) 191 {*adddi_1}
      (nil))
 during RTL pass: reload
 ../../../../src/gcc-trunk/libgfortran/generated/matmul_i4.c:1210:1:
internal compiler error: in rtl_verify_bb_layout, at cfgrtl.c:2987
 0x6a259f _fatal_insn(char const*, rtx_def const*, char const*, int,
char const*)
     ../../../src/gcc-trunk/gcc/rtl-error.c:108
 0x92b8f1 rtl_verify_bb_layout
     ../../../src/gcc-trunk/gcc/cfgrtl.c:2987
 0x92b8f1 rtl_verify_flow_info
     ../../../src/gcc-trunk/gcc/cfgrtl.c:3033
 0x90fb1d verify_flow_info()
     ../../../src/gcc-trunk/gcc/cfghooks.c:263
 0x14c3569 checking_verify_flow_info
     ../../../src/gcc-trunk/gcc/cfghooks.h:198
 0x14c3569 try_optimize_cfg
     ../../../src/gcc-trunk/gcc/cfgcleanup.c:2991
 0x14c3569 cleanup_cfg(int)
     ../../../src/gcc-trunk/gcc/cfgcleanup.c:3156
 0xb6c969 do_reload
     ../../../src/gcc-trunk/gcc/ira.c:5518
 0xb6c969 execute
     ../../../src/gcc-trunk/gcc/ira.c:5653
 Please submit a full bug report,
 with preprocessed source if appropriate.
 Please include the complete backtrace with any bug report.
 See <https://gcc.gnu.org/bugs/> for instructions.
 make[3]: *** [Makefile:4194: matmul_i4.lo] Error 1
Comment 13 Moritz Bunkus 2018-11-23 19:46:17 UTC
I'm the author of MKVToolNix which has been mentioned as being affected by the bug. I've just re-compiled my whole mingw cross compilation setup (using MXE) using gcc 8.2.0 release + your proposed patch. The patch applies with offsets but without rejects.

I'm happy to report that both all the components of MXE as well as MKVToolNix itself compile fine, and more importantly MKVToolNix runs fine, too.

From my point of view the patch does what it's supposed to.

Note that I've only built 64-bit binaries, not 32-bit ones.
Comment 14 Jakub Jelinek 2019-01-08 09:57:10 UTC
Author: jakub
Date: Tue Jan  8 09:56:36 2019
New Revision: 267686

URL: https://gcc.gnu.org/viewcvs?rev=267686&root=gcc&view=rev
Log:
	Backported from mainline
	2018-11-22  Jakub Jelinek  <jakub@redhat.com>

	PR target/85644
	PR target/86832
	* config/i386/i386.c (ix86_option_override_internal): Default
	ix86_stack_protector_guard to SSP_TLS only if TARGET_THREAD_SSP_OFFSET
	is defined.
	* config/i386/i386.md (stack_protect_set, stack_protect_set_<mode>,
	stack_protect_test, stack_protect_test_<mode>): Use empty condition
	instead of TARGET_SSP_TLS_GUARD.

Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/config/i386/i386.c
    branches/gcc-8-branch/gcc/config/i386/i386.md
Comment 15 Jakub Jelinek 2019-01-08 10:43:45 UTC
Fixed for 8.3+ too.