[Bug c/61904] New: Incorrect stack red-zoning on x86-64 code generation

[Bug c/61904] New: Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

            Bug ID: 61904
           Summary: Incorrect stack red-zoning on x86-64 code generation
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: torvalds@linux-foundation.org

gcc-4.9.0 in Debian seems to miscompile the linux kernel for x86-64 in certain
configurations, creating accesses to below the stack pointer even though the
kernel uses -mno-red-zone.

The kernel cannot use the x86-64 stack red-zoning, because the hardware only
switches stacks on privilege transfers, so interrupts that happen in kernel
mode will not honor the normal 128-byte stack red-zone.

Attached is the pre-processed C code of the current kernel file

   kernel/sched/fair.c

which apparently on gcc-4.9.0 will miscompile the function "load_balance()",
creating code like this:

load_balance:
.LFB2408:
        .loc 2 6487 0
        .cfi_startproc
.LVL1355:
        pushq   %rbp    #
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp      #,
        .cfi_def_cfa_register 6
        pushq   %r15    #
        pushq   %r14    #
        pushq   %r13    #
        pushq   %r12    #
        .cfi_offset 15, -24
        .cfi_offset 14, -32
        .cfi_offset 13, -40
        .cfi_offset 12, -48
        movq    %rdx, %r12      # sd, sd
        pushq   %rbx    #
.LBB2877:
        .loc 2 6493 0
        movq    $load_balance_mask, -136(%rbp)  #, %sfp
.LBE2877:
        .loc 2 6487 0
        subq    $184, %rsp      #,
        .cfi_offset 3, -56
        .loc 2 6489 0
     ....


Note the "subq    $184, %rsp" *after* the compiler has already spilled to the
stack (the spill is insane, btw, since it's spilling a constant value!)

The second attachement is the reported mis-compiled result. I don't personally
have the affected gcc version, but you can see the options passed into the
compiler in the resulting "fair.s" file. The "-Os" in particular seems to be
important, with the bug not happening with "-O2".

[Bug c/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #1 from Linus Torvalds <torvalds@linux-foundation.org> ---
Created attachment 33183
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33183&action=edit
Buggy result of compilation

This was sent by the reporter of the kernel problem, Michel Dänzer
<michel@daenzer.net>, at my request.

It's gzipped to fit the file size limit.
>From gcc-bugs-return-457078-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Fri Jul 25 08:12:11 2014
Return-Path: <gcc-bugs-return-457078-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 14560 invoked by alias); 25 Jul 2014 08:12:10 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 14464 invoked by uid 48); 25 Jul 2014 08:12:05 -0000
From: "pinskia at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/61904] Incorrect stack red-zoning on x86-64 code generation
Date: Fri, 25 Jul 2014 08:12:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: pinskia at gcc dot gnu.org
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: component bug_severity
Message-ID: <bug-61904-4-ydAeCSh3rB@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-61904-4@http.gcc.gnu.org/bugzilla/>
References: <bug-61904-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2014-07/txt/msg01669.txt.bz2
Content-length: 350

https://gcc.gnu.org/bugzilla/show_bug.cgi?ida904

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |target
           Severity|critical                    |normal

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

Markus Trippelsdorf <trippels at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2014-07-25
                 CC|                            |trippels at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
The testcase is missing. Please attach fair.i.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #3 from Linus Torvalds <torvalds@linux-foundation.org> ---
Created attachment 33184
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=33184&action=edit
gzipped pre-processed fair.i file

Apparently attaching a file during the initial bug entry creation failed,
probably because it failed the size check. So here's the fair.i file.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #4 from Linus Torvalds <torvalds@linux-foundation.org> ---
As I already mentioned to Jakub Jelinek separately in the original email thread
about the kernel problem:

 "Note that I don't personally have a reproducer (my machine has
  gcc-4.8.3, and I don't see the same behavior), but I included the
  incorrect fair.s file that Michel sent me (which has all the command
  line options in it), and a pre-processed "fair.i" source file that I
  generated and that *should* match the configuration that was the
  source for that result. So there might be some version/configuration
  skew there between the two files, but I think they match.

  Holler if you cannot reproduce the problem based on that."

so if the attached *.i and resulting buggy *.s files don't match up perfectly,
that's the explanation.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |UNCONFIRMED
     Ever confirmed|1                           |0

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
With both the FSF 4.9.0 and 4.9.1 releases and

gcc -S t.i -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3
-mtune=generic -mno-red-zone -mcmodel=kernel -maccumulate-outgoing-args
-mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx -march=x86-64 -g -Os
-fno-strict-aliasing -fno-common -fno-asynchronous-unwind-tables
-fno-omit-frame-pointer

(too many options only to guess from the .s file - can you please specify
explicitely passed options?)


load_balance:
.LFB2409:
        .loc 2 6487 0
.LVL1260:
        pushq   %rbp
.LCFI442:
        movq    %rsp, %rbp
.LCFI443:
        pushq   %r15
        pushq   %r14
        pushq   %r13
        pushq   %r12
.LCFI444:
        movq    %rdx, %r12
        pushq   %rbx
.LBB2812:
        .loc 2 6493 0
        movq    $load_balance_mask, -376(%rbp)
.LBE2812:
        .loc 2 6487 0
        subq    $416, %rsp
.LCFI445:
        .loc 2 6489 0
        movq    (%rdx), %rax
        .loc 2 6487 0
        movl    %edi, -392(%rbp)
        movl    %ecx, -388(%rbp)
        movq    %r8, -456(%rbp)
        .loc 2 6489 0
        movq    %rax, -424(%rbp)
.LVL1261:
.LBB2813:
        .loc 2 6493 0
        movq    -376(%rbp), %rax
.LVL1262:
#APP
# 6493 "kernel/sched/fair.c" 1
        add %gs:this_cpu_off, %rax
# 0 "" 2
#NO_APP


which I guess is equally bad (even if not matching the output from Michel).
It seems to be triggered by -g or -fvar-tracking and is fixed on the 4.9
branch - so it might be a duplicate of PR61801.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |DUPLICATE

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Reduced set of options to reproduce with 4.9.1 are

-mno-red-zone -mcmodel=kernel -maccumulate-outgoing-args -mno-sse -g  -Os
-fno-omit-frame-pointer

if you use -fno-var-tracking the bug goes away, similar if you update to
the 4.9 branch head.

If you use -fno-schedule-insns2 you see a similar pattern to the mentioned PR:

        subq    $424, %rsp
.LCFI427:
        movl    %edi, -400(%rbp)
        movq    %rdx, %rbx
        movl    %ecx, -396(%rbp)
        movq    %r8, -464(%rbp)
.LVL1306:
        .loc 2 6489 0
        movq    (%rdx), %rax
        movq    %rax, -440(%rbp)
.LVL1307:
.LBB2616:
        .loc 2 6493 0
        movq    $load_balance_mask, -384(%rbp)
        movq    -384(%rbp), %rax
.LVL1308:
#APP
# 6493 "kernel/sched/fair.c" 1
        add %gs:this_cpu_off, %rax

so that spill is right before the asm and we immediately re-load it into
a register required by an asm constraint (stupid register allocator).

*** This bug has been marked as a duplicate of bug 61801 ***

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #7 from Linus Torvalds <torvalds@linux-foundation.org> ---
Just for completeness for the original bug report, here's the actual compiler
command line that the kernel uses to generate the *.s file.

  gcc -Wp,-MD,kernel/sched/.fair.s.d  -nostdinc -isystem
/usr/lib/gcc/x86_64-redhat-linux/4.8.3/include -I./arch/x86/include
-Iarch/x86/include/generated  -Iinclude -I./arch/x86/include/uapi
-Iarch/x86/include/generated/uapi -I./include/uapi -Iinclude/generated/uapi
-include ./include/linux/kconfig.h -D__KERNEL__ -Wall -Wundef
-Wstrict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common
-Werror-implicit-function-declaration -Wno-format-security -m64 -mno-mmx
-mno-sse -mno-80387 -mno-fp-ret-in-387 -mpreferred-stack-boundary=3
-mtune=generic -mno-red-zone -mcmodel=kernel -funit-at-a-time
-maccumulate-outgoing-args -DCONFIG_AS_CFI=1 -DCONFIG_AS_CFI_SIGNAL_FRAME=1
-DCONFIG_AS_CFI_SECTIONS=1 -DCONFIG_AS_FXSAVEQ=1 -DCONFIG_AS_CRC32=1
-DCONFIG_AS_AVX=1 -DCONFIG_AS_AVX2=1 -pipe -Wno-sign-compare
-fno-asynchronous-unwind-tables -mno-sse -mno-mmx -mno-sse2 -mno-3dnow -mno-avx
-fno-delete-null-pointer-checks -Os -Wno-maybe-uninitialized
-Wframe-larger-than=2048 -fno-stack-protector -Wno-unused-but-set-variable
-fno-omit-frame-pointer -fno-optimize-sibling-calls -g
-Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow
-fconserve-stack -Werror=implicit-int -Werror=strict-prototypes
-DCC_HAVE_ASM_GOTO    -D"KBUILD_STR(s)=#s" -D"KBUILD_BASENAME=KBUILD_STR(fair)"
 -D"KBUILD_MODNAME=KBUILD_STR(fair)"  -fverbose-asm -S -o kernel/sched/fair.s
kernel/sched/fair.c

although looking at the generated .s file in the attachment, I really think
it's all there too in the comments at the top of the file.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #8 from Linus Torvalds <torvalds@linux-foundation.org> ---
Oh, and this is marked as a duplicate of 61801, but that one is marked to be in
gcc-4.8.3

The particular problem we see in kernel code generation seems to *not* happen
in 4.8.3 (current fedora 20), and happens with Debian 4.9.0.

So now I worry about

 (a) whether the duplicate bug is really true

 (b) or perhaps 4.8.3 really does have the same problem and we might get bitten
there too, and it just happens to trigger on 4.9.0 for some otherwise unrelated
reason.

I'd like to have some way to tell known-bad compilers, so that we know to avoid
them..

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #9 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Linus Torvalds from comment #8)
> Oh, and this is marked as a duplicate of 61801, but that one is marked to be
> in gcc-4.8.3
> 
> The particular problem we see in kernel code generation seems to *not*
> happen in 4.8.3 (current fedora 20), and happens with Debian 4.9.0.

Different optimization happen before the scheduler between 4.8.3 and 4.9.0
which causes the difference.

> 
> So now I worry about
> 
>  (a) whether the duplicate bug is really true

It is as the true issue was fixed there.

> 
>  (b) or perhaps 4.8.3 really does have the same problem and we might get
> bitten there too, and it just happens to trigger on 4.9.0 for some otherwise
> unrelated reason.

Correct.

> 
> I'd like to have some way to tell known-bad compilers, so that we know to
> avoid them..

The bad compiler versions are 4.5.0 (when debug_insn came in) to 4.8.3 and
4.9.0 and 4.9.1.

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[torvalds@linux-foundation.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #10 from Linus Torvalds <torvalds@linux-foundation.org> ---
(In reply to Andrew Pinski from comment #9)
> 
> The bad compiler versions are 4.5.0 (when debug_insn came in) to 4.8.3 and
> 4.9.0 and 4.9.1.

Ok, so we have no reasonable way of avoiding the problem compiler version. I
had hoped that we could just do a warning if people use 4.9.0/1, since they
aren't very common yet.

Ugh. We had one suggestion of having some post-compile checking pass, but that
one was (so far) just handwaving ("objdump + perl-script"). It doesn't sound
very pleasant.

The problem is that these things are a bitch to debug - they turn into these
completely impossible kernel oopses or corruption, and we were just very lucky
that this one case happened to be repeatable and pinpoint for two people. Are
there others? We have no way of knowing..

Anyway, thanks for the quick resolution, even if I'm now rather nervous about
existing compilers..

[Bug target/61904] Incorrect stack red-zoning on x86-64 code generation

[trippels at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

--- Comment #11 from Markus Trippelsdorf <trippels at gcc dot gnu.org> ---
Here's a small (auto-)reduced testcase for this specific issue:

markus@x4 ~ % cat fair.i
int a, b, d;
enum cpu_idle_type
{
  CPU_MAX_IDLE_TYPES
};
struct sched_domain
{
  int balance_interval;
  int lb_count[0];
  int lb_nobusyg[];
} c,*g ;
__typeof__(int[0]) f;
int *h;
static struct sched_domain *fn1 (int *p1, int *p2)
{
  struct sched_domain *e;
  int i;
  for (; i = fn2 (i);)
    {
      e = &*(
               {

                 __asm__("" : "=r"(d) : "0"(0));
                 (typeof(&c))0 + a;

               });
      fn3 ();
      fn4 (i);
    }
  return e;
}

fn5 (int p1, struct sched_domain *p2, enum cpu_idle_type p3)
{
  int *j = *(
               {
                 long k;
                 asm volatile("" : "=r"(k) : ""(b), "0"(&f));
                 (typeof(*&f) *)k;
               });
  if (p3)
    h = 0;
  int l = *j;
  p2->lb_count[p3]++;
redo:
  p2->lb_nobusyg[p3]++;
  g = fn1 (0, 0);
  if (fn6 (j))
    goto redo;
  p2->balance_interval *= 2;
}

markus@x4 ~ % gcc -c -mcmodel=kernel -Os -fno-omit-frame-pointer
-fcompare-debug fair.i
gcc: error: fair.i: -fcompare-debug failure (length)
markus@x4 ~ % gcc -c -mcmodel=kernel -O2 -fno-omit-frame-pointer
-fcompare-debug fair.i
markus@x4 ~ %