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

From: Alviro Iskandar Setiawan
Date: Tue Mar 01 2022 - 06:43:47 EST


On Tue, Mar 1, 2022 at 4:46 PM Ammar Faizi wrote:
> Fortunately, the constraint violation that's fixed by patch 1 doesn't
> yield any bug due to the nature of System V ABI. Should we backport
> this?

hi sir, it might also be interesting to know that even if it never be
inlined, it's still potential to break.

for example this code (https://godbolt.org/z/xWMTxhTET)

__attribute__((__noinline__)) static void x(int a)
{
asm("xorl\t%%r8d, %%r8d"::"a"(a));
}

extern int p(void);

int f(void)
{
int ret = p();
x(ret);
return ret;
}

translates to this asm

x:
movl %edi, %eax
xorl %r8d, %r8d
ret
f:
subq $8, %rsp
call p
movl %eax, %r8d
movl %eax, %edi
call x
movl %r8d, %eax
addq $8, %rsp
ret

See the %r8d? It should be clobbered by a function call too. But since
no one tells the compiler that we clobber %r8d, it assumes %r8d never
changes after that call. The compiler thinks x() is static and will
not clobber %r8d, even the ABI says %r8d will be clobbered by a
function call. So i think it should be backported to the stable
kernel, it's still a fix

-- Viro