Messages in this thread |  | | From | Thomas Gleixner <> | Subject | Re: [PATCH v6 1/2] x86/delay: Fix the wrong asm constraint in `delay_loop()` | Date | Sun, 03 Apr 2022 18:57:40 +0200 |
| |
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.
Thanks,
tglx
|  |