Re: GCC 15 -fzero-init-padding-bits= option and redzone clobber

[Jakub Jelinek <jakub@redhat.com>]

On Fri, Nov 29, 2024 at 01:52:38PM +0100, Peter Zijlstra wrote:
> > Note, there is also __builtin_clear_padding builtin to clear padding bits
> > already since GCC 11, though it doesn't clear bits in unions unless they
> > are padding bits for all possible members, as it doesn't know which union
> > member is current.
> 
> *groan* I suppose we should enable that flag when present :/

If you want to keep previous behavior, sure.  That is why I'posted this FYI.

> > Another new feature since today that might be relevant to kernel is
> > the "redzone" inline asm clobber.
> > It can/should be used on inline asm which does or could clobber memory
> > below the stack pointer and so its presence must disable use of redzone
> > (currently on x86_64 and powerpc*),
> 
> At least on x86_64 we don't currently have a redzone.

Ah, I see, kernel builds with -mno-red-zone, missed that being mentioned
in https://gcc.gnu.org/PR117312 that led to this.

> I'm assuming the
> "memory" clobber still very much includes everything?

No.  "memory" clobber is about clobbering memory which could be clobbered
e.g. by a call to a function one doesn't know anything about and which one
passes the inline asm operands to.
So it can certainly clobber global variables, or address taken whose address
escaped to global state (or could escape), or memory referenced from the
operands of the inline asm.
It can't clobber random stack locations it knows nothing about, say
clobbering the saved registers on the stack in the stack frame, return
address, non-address taken variables which were just spilled to stack due to
register allocator decisions, etc.  There would be nothing the compiler
could do about that.

> And why was it deemed okay to change behaviour that might break existing
> code?

You mean for unions?  It can only change behavior of invalid code (relying
on undefined behavior).  Most compiler optimizations change behavior of code
with UB in it, otherwise the compiler couldn't optimize anything.

> We have this thing:
> 
> /*
>  * This output constraint should be used for any inline asm which has a "call"
>  * instruction.  Otherwise the asm may be inserted before the frame pointer
>  * gets set up by the containing function.  If you forget to do this, objtool
>  * may print a "call without frame pointer save/setup" warning.
>  */
> register unsigned long current_stack_pointer asm(_ASM_SP);
> #define ASM_CALL_CONSTRAINT "+r" (current_stack_pointer)
> 	
> which we use a *LOT*.

And wrongly so.  Inline asm really can't change the stack pointer (with the
meaning that rsp would be different between the entry to the inline asm and
its exit(s) (multiple for asm goto).  So telling the compiler it does change
is wrong.
In GCC 9 we've added a warning for "rsp" and similar clobbers of inline asm,
warning: listing the stack pointer register ‘rsp’ in a clobber list is deprecated
There is none currently for the "+r" (current_stack_pointer), but e.g. doing
current_stack_pointer += 16;
or
current_stack_pointer = whatever;
will certainly cause UB.  The behavior of "+r" (current_stack_pointer) you
are relying on certainly isn't guaranteed.
See https://gcc.gnu.org/PR117312 for some details.  There are surely other
bugs that talk about that.

	Jakub