- Notifications
You must be signed in to change notification settings - Fork 15.3k
Description
Not exactly a duplicate of #60419 because that issue specifically mentions x86.
I just hit this issue in RISC-V bare-metal target with clang 17.0.0, so most definitively not restricted to x86. My code is fairly similar to the test case above:
#define DO_WAIT 0 #define DO_ACTION 1 int g_lock = DO_WAIT; void __attribute__((noinline)) worker() { for (;;) { volatile int i = 0; i++; g_lock = DO_WAIT; while (g_lock != DO_ACTION) { } i = 0xCAFECAFE; } return; } void dontcallme() { volatile int i = 0; i++; } int main() { worker(); return 0; }The disassembly for that is:
worker: # @worker # %bb.0: # %for.cond addisp, sp, -16 sw zero, 12(sp) lw a0, 12(sp) addi a0, a0, 1 sw a0, 12(sp) .Lfunc_end0: .size worker, .Lfunc_end0-worker # -- End function .globl dontcallme # -- Begin function dontcallme .p2align2 .type dontcallme,@function dontcallme: # @dontcallme # %bb.0: # %entry addisp, sp, -16 sw zero, 12(sp) lw a0, 12(sp) addi a0, a0, 1 sw a0, 12(sp) addisp, sp, 16 ret .Lfunc_end1: .size dontcallme, .Lfunc_end1-dontcallme # -- End function .globl main # -- Begin function mainThere's no control instruction at the end of worker, so it just falls through to dontcallme.
I'm baffled as to how can anyone think that's reasonable.
GCC for RISC-V does the right thing and adds a j: https://godbolt.org/z/h55bYsdo9
I've tried the -fsanitize=undefined flag as suggested here, but that doesn't work: https://godbolt.org/z/aYrqfsca7
The workaround is very counter-intuitive:
while (g_lock != DO_ACTION) { asm volatile(""); }Yields:
.LBB0_2: # %while.body # Parent Loop BB0_1 Depth=1 # => This Inner Loop Header: Depth=2 #APP #NO_APP lw a3, %pcrel_lo(.Lpcrel_hi0)(a0) bne a3, a1, .LBB0_2 # %bb.3: # %while.end # in Loop: Header=BB0_1 Depth=1 sw a2, 12(sp) j .LBB0_1 .Lfunc_end0: .size worker, .Lfunc_end0-worker # -- End functionAlso here: https://godbolt.org/z/f6Y6qdrMT
I also disagree that #60622, #60419, and #60588 should be closed. Especially because there are no proposed workarounds.
#60622 has been over for over an year, and it's currently frozen for comments.