Re: [PATCH v6 1/2] x86/delay: Fix the wrong asm constraint in `delay_loop()`

From: Thomas Gleixner
Date: Sun Apr 03 2022 - 12:57:47 EST

On Tue, Mar 29 2022 at 17:47, Ammar Faizi wrote:
> The asm constraint does not reflect that the asm statement can modify
> the value of @loops. But the asm statement in delay_loop() does modify
> the @loops.
> Specifiying the wrong constraint may lead to undefined behavior, it may
> clobber random stuff (e.g. local variable, important temporary value in
> regs, etc.). This is especially dangerous when the compiler decides to
> inline the function and since it doesn't know that the value gets
> modified, it might decide to use it from a register directly without
> reloading it.
> Fix this by changing the constraint from "a" (as an input) to "+a" (as
> an input and output).

This analysis is plain wrong. The assembly code operates on a register
and not on memory:

asm volatile(
" test %0,%0 \n"
" jz 3f \n"
" jmp 1f \n"

".align 16 \n"
"1: jmp 2f \n"

".align 16 \n"
"2: dec %0 \n"
" jnz 2b \n"
"3: dec %0 \n"

: /* we don't need output */
----> :"a" (loops)

This tells the compiler to use [RE]AX and initialize it from the
variable 'loops'. It's never written back because all '%0' in the above
assembly are substituted with [RE]AX. This also tells the compiler that
the inline assembly clobbers [RE]AX and that's all it needs to know.

Nothing to fix here, whether the code is inlined or not.