On 4/3/22 11:57 PM, Thomas Gleixner wrote:
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.
Hi Thomas,
Thanks for taking a look. I doubt about your sentence "This also tells the compiler that the inline assembly clobbers [RE]AX".
How come it tells the compiler that the inline ASM clobbers [RE]AX?
That's an input constraint. Doesn't that mean it is read-only for the ASM statement? That means the compiler is allowed to assume [RE]AX doesn't change inside the ASM statement.
Those `dec`s do really change the [RE]AX. Please review this again.
Thanks!