x86.c: mark stack read implies execute false and pop stack on exiting

[rilysh]

Binary files don't mark their stack non-executable if GNU stack isn't set to non-executable (or read implies executable value to false). As a result, every memory segments in the binary file will be treated as executable, even they shouldn't be. This isn't yet fixed in the binutils (at least they don't force to mark stack as non-executable), but deprecated. And also, pop the stack instead using leave instruction. Both does the same task, except later one copies frame pointer to the stack pointer which frees the stack.

[Jamesbarford]

Thanks for your proposed contribution.

For the non executable stack, rather than adding to the assembly, perhaps we could add this to the assembler with -Wa,--noexecstack? Then we'd achieve the same thing? Moreover, from what I've read, the stack is non-executable by default. It would need to be explicitly turned on which is typically only required for function trampolining. Though please correct me if I'm wrong.

From my understanding you can either pop %rbp or use leave where leave is the prefered mechanism for clearing up the stack docs, from looking at what gcc and friends output it also seems as though they use leave. And we want to clean up stack, like you say. So I'm not sure why we'd want to pop and would prefer to keep this as leave for the time being.

[rilysh]

The reason I don't like to use -Wa,--noexecstack is that you've to pass this option to the linker and it hides itself until the user sees the source of the compiler or the script that the compiler will invoke to invoke the linker. Creating a marker makes more sense as when user will pass the '-S' flag to emit the assembly output, the marker at the bottom of the assembly source makes sure what it's intended for.

This GNU stack section is merely a marker for the compiler, it has no downside other than it has to be present somewhere in the source file. Using either one is just fine.

From my understanding you can either pop %rbp or use leave where leave is the prefered mechanism for clearing up the stack docs,

It's actually not exactly. leave instruction is there for compatibility reasons with earlier x86 CPUs, and has no real advantage over moving the base pointer to the stack pointer and the poping the stack pointer. In fact leave has 3 uops, whereas mov esp, ebp and pop esp has only 2 uops. Most C compilers can also not move the base pointer to the stack pointer, at the end, but they can at the beginning, which makes cleanup fairly easy, as you now only have to pop the base pointer.

Also, leave is mainly intended to use to clear up stack frames created by enter instruction. But enter is very slow and inefficient in many cases, so it's not being used anymore and no compilers actually emit this instruction. leave on the other hand isn't "slow" in micro-bench mark, but has no benefit.

I think you're using an old version, or some modified version of GCC, because using leave instruction is no longer preferred by most compilers.

Compiling with GCC 4.5: https://godbolt.org/z/Mnrrnbz58 Compiling with GCC 14.1: https://godbolt.org/z/6vrj3f8oG

[Jamesbarford]

Thanks for the indepth comment, that is much appreciated.

With respect to the marker being created this is not something I see when compiling usual c code with gcc-14. The addition of this marker also makes some of the tests fail with:

/tmp/holyc-asm.s:572:20: error: unexpected token in '.section' directive
 .section .note.GNU-stack,"",@progbits
                   ^
sh: ./a.out: No such file or directory
FAILED to run: 10_qsort.HC

... I really need a better testing framework.

With regard to leave, yes you are correct gcc-14 -O2 <file>.c sometimes produces a leave instruction or it sometimes instead does

add   $<imm>, %rsp
mov   %rsp, %rbp
pop   %rbp

I've modified the gcc-14 snippet you provided in godbolt (added a few variables to get some stack allocation):

• gcc-14 -O0 https://godbolt.org/z/c7ac7Mx37, which produces a leave.
• gcc-14 -O2 https://godbolt.org/z/71v1Gdqd1, which does more stack manipulation

As you can probably see from the x86.c file there are a number of fragile hacks manipulating the stack thus using leave is simpler. This file really needs redoing and the assembly code this compiler outputs is generally fairly low quality, thus losing a few cycles for a leave instruction is a trade off I'm willing to make for the simplicity.

[rilysh]

Testing locally on my end, I'm not seeing any kind of test failures. Are you sure system linker is up to date alongside the compiler?

In that assembly section, it's "freeing" up the stack and moving the stack pointer for some reason to the base pointer to pop the stack. I'm not sure why GCC is doing this in '-O2' mode (again, not happening on my end, nor in godbolt), as you don't need to do this, instead you can ask for stack space by subtracting relative space from the stack pointer directly.

gcc-14 -O2 https://godbolt.org/z/71v1Gdqd1, which does more stack manipulation

It does nothing fancy particular with the stack. In the first godbolt link, I'm not sure why GCC still stuck using leave, as in the debugger it doesn't bring any points. I'm guessing they can tradeoff this as this wasn't an optimizing build. Clang on the other hand, doesn't uses leave, instead just pops the stack.

For now I'm closing this PR.