2

This is compiler output from a Linux kernel function (compiled with -mno-red-zone):

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!)

Linus reported this bug to gcc 2 days ago. But I don't understand what the bug is. Why is that subq wrong?

Edit: bug report is here: sorry for not included this before https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904

Peter Cordes
  • 328,167
  • 45
  • 605
  • 847
BufBills
  • 8,005
  • 12
  • 48
  • 90
  • 1
    This question appears to be off-topic because it is about having a discussion about current events, not a specific programming problem one is encountering. – Pascal Cuoq Jul 30 '14 at 08:17
  • One of the things that are great with the description “specific programming problem one is encountering” which I think used to be more or less verbatim in the description of on-topic questions, is that a problem that you **encounter** is automatically the right level to be the subject of a question. If you were writing a compiler for x86-64, you would already know that there is a recommended ABI, that an exception must be made to the ABI when compiling kernel code, and if somehow you didn't understand how the instructions break this ABI+exception, that could be answered simply. – Pascal Cuoq Jul 30 '14 at 17:15
  • 1
    Unfortunately, this is not a specific programming problem that you encountered, this is an event that you read about on a news site. You did not read about it because it was just the right level of difficulty for you to have an a-ha moment about. You read about it because it involved Linux Torvalds. Anyone who wants to explain it to the level of understanding displayed in your question needs either to provide links to huge external documentation (http://www.x86-64.org/documentation/abi.pdf ) or write a book chapter to explain everything. Neither is considered appropriate for StackOverflow. – Pascal Cuoq Jul 30 '14 at 17:19
  • @PascalCuoq u are right. close it you could. – BufBills Jul 30 '14 at 18:59
  • 1
    I made an edit to rephrase it as a real question about asm programming. (And to add the critical fact that this was compiled with `-mno-red-zone`.) – Peter Cordes Jun 28 '16 at 12:29

2 Answers2

3

I don't understand why that subq is wrong?

The problem is its order relative to the movq $load_balance_mask, -136(%rbp) instruction. The subq allocates space on the stack by modifying the stack pointer, and the movq writes to a location within that allocated area. But in this case the movq comes before the subq, i.e. it's writing to (as of yet) unallocated stack space. Now what if an interrupt occurs in between the movq and the subq and the interrupt handler tries to touch that same area of the stack? All sorts of weird things could happen as a result, most of which probably would be bad.

Having the movq first would have been ok in the presence of a red zone. Quoting from wikipedia:

A red zone is a fixed-size area in memory beyond the stack pointer that has not been "allocated". This region of memory is not to be modified by interrupt/exception/signal handlers. This allows the space to be used for temporary data without the extra overhead of modifying the stack pointer. The x86-64 ABI mandates a 128-byte red zone.

However, as Linus wrote in the email thread about this bug: "But we build the kernel with -mno-red-zone. We do *not* follow the x86-64 ABI wrt redzoning".
And with red zones disabled the code generator should not have been allowed to output that movq before the subq.

Michael
  • 57,169
  • 9
  • 80
  • 125
1

I don't see a problem there. The constant isn't really spilling, it's initializing a local variable. The red zone is 128 bytes under the stack pointer, so -136(%rbp) is within limits because rbp has the value of rsp before the five pushes decremented it by 40. The compiler is allowed to adjust rsp whenever it feels like. It might be an alloca invocation too.

You could have provided a link or at least a summary of the bug report. I couldn't find anything relevant in the gcc bugzilla. The original C source would have been useful as well.

Jester
  • 56,577
  • 4
  • 81
  • 125
  • 2
    The kernel disables the red zone (`-mno-red-zone` compiler option), because an interrupt handler might clobber whatever lies past the stack pointer. https://lkml.org/lkml/2014/7/24/584 - The way around this is, if you don't want it clobbered, subtract from the stack pointer! (as I would expect `alloca` to do). – asveikau Jul 29 '14 at 21:47
  • @asveikau thanks. The original question never mentioned that this code was compiled with red zone disabled. – Jester Jul 29 '14 at 21:56
  • https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904 bug report is here. sorry for late – BufBills Jul 30 '14 at 01:40
  • 2
    Initializing the backing storage of a variable with a constant looks like terrible code generation. Later, the variable will be loaded into a register when it is used: at that point, the register can be loaded with the constant (straightforward constant propagation). Initializing the backing memory makes sense if the first use of the variable is that its address is taken and the pointer to it is passed to some other function. That doesn't seem to be going on in that function. – Kaz Jul 30 '14 at 01:56
  • @Kaz I wouldn't be that absolutist. If this is the backing store of a variable that is only used in a loop, and RA elects to spill it for whatever reason between iterations, it's probably better to initialize with a constant than to peel one iteration and const-prop through it. Indeed this appears to be the case here: Linus has a loop with several function calls within it and GCC's codegen for them is at issue here. – Iwillnotexist Idonotexist Jun 28 '16 at 13:16