Repository navigation
Volatile not respected in some edge cases #21033
Description
Activity
For a little bit more background:
- This doesn't happen in debug mode. The Godbolt linked above uses release-small, since that's common for embedded
- This is super easy to hit in microzig.
while (gpio.num(18).read() == 0) {}(RP2040 target in microzig) is a footgun because the read just gets optimized out. Spinning the CPU while waiting for a MMIO value to update is a very common pattern, and I would expect it to work. - I suspect this may be related to Zig 0.11.0 regression around @bitCast() / @intCast() in non-debug mode on embedded Cortex-M4 #17882.
- I have a reproduction that doesn't involve arrays. It's not minimal-enough to be worth sharing, I don't think, but I suspect a slightly more general issue where maybe Zig is emitting a temporary in the IR when it can't access a value in a single load, and then the temporary loses the volatile information (but this is speculation).
Because of this issue, when writing Zig code for embedded you basically have to use debug mode (what I do) or include a
asm volatile ("" ::: "memory");inside any loops (which microzig does, https://github.com/search?q=repo%3AZigEmbeddedGroup%2Fmicrozig+tight_loop_contents&type=code).Reacted by kibels, Theo Fabi and RatakorThis effects my confidence in Zig's suitability for my uses (embedded robotics), so I wanted to look into it more.
I've come up with a minimal version of my struct example, and by looking at the LLVM IR I have a better idea what's going on.In this example, Zig (incorrectly) outputs LLVM that does not have any volatile restrictions, so as soon as it is complied in a release mode, LLVM moves the struct access outside of the loop, breaking the intention of the code.
const PeripheralType = struct { raw: struct { value: usize }, }; export fn struct_bad() void { const peripherals: *volatile PeripheralType = @ptrFromInt(0xd0000000); while (peripherals.raw.value == 0) {} }
https://godbolt.org/z/8W4dxY8WW
Compiling in debug mode, we can see that in this case, Zig outputs LLVM IR corresponding to
getelementptrthenloadfor the double-lookup ofperipherals.raw.value(with novolatilein sight). With only a single-level struct (e.g.peripherals.value) Zig outputs insteadload volatile.Workaround:
const PeripheralType = extern struct { raw: extern struct { value: usize }, }; export fn struct_bad() void { const peripherals: *volatile PeripheralType = @ptrFromInt(0xd0000000); const derived_ptr = &peripherals.raw.value; while (derived_ptr.* == 0) {} }
Also, this use case requires types that have well-defined memory layouts, otherwise the load/store may occur at a different address than intended. I added
externkeywords to the structs to demonstrate this.I ran into this issue as well, but with a particularly bizarre workaround that might shed some light. Apparently, the illegal read occurs only when writing the packed struct using an anonymous struct literal. Specifying the type explicitly instead of anonymous causes the correct behavior.
It's behaving as though the compiler doesn't understand that the anonymous struct is the same layout as the destination struct, so it's using bitmasks to write to the destination, causing the illegal reads.
I believe this behavior occurs regardless of build mode (Debug, ReleaseFast, ReleaseSmall). Also, if the
volatilequalifier is removed, both functions lower to the exact same IR.https://godbolt.org/z/3cnaYhGMK
Copy of code:
const custom: *volatile Custom = @ptrFromInt(0xdff000); const Bltsize = packed struct { width: u6, height: u10 }; const Custom = extern struct { bltsize: Bltsize = undefined }; export fn illegalReadAndWrite(height: u8) void { custom.* = .{ .bltsize = .{ .width = 40, .height = height } }; } export fn legalWriteOnly(height: u8) void { custom.* = .{ .bltsize = Bltsize{ .width = 40, .height = height } }; }
Resulting LLVM IR:
; Function Attrs: minsize nofree norecurse noredzone nounwind optsize define dso_local void @illegalReadAndWrite(i8 zeroext %0) local_unnamed_addr #0 { %2 = load i16, ptr inttoptr (i32 14675968 to ptr), align 4096 %3 = and i16 %2, -64 %4 = or disjoint i16 %3, 40 store volatile i16 %4, ptr inttoptr (i32 14675968 to ptr), align 4096 %5 = zext i8 %0 to i16 %6 = shl nuw nsw i16 %5, 6 %7 = or disjoint i16 %6, 40 store volatile i16 %7, ptr inttoptr (i32 14675968 to ptr), align 4096 ret void } ; Function Attrs: minsize nofree norecurse noredzone nounwind optsize define dso_local void @legalWriteOnly(i8 zeroext %0) local_unnamed_addr #0 { %2 = zext i8 %0 to i16 %3 = shl nuw nsw i16 %2, 6 %4 = or disjoint i16 %3, 40 store volatile i16 %4, ptr inttoptr (i32 14675968 to ptr), align 4096 ret void }
I believe this has been resolved, at least the reproductions here, by #25154.
Zig Version
0.13.0
Steps to Reproduce and Observed Behavior
Taking a volatile pointer to a nested array of packed structs (such as an array of machine registers) causes loads to not be volatile which can lead to bad machine code like infinite loops being generated
The minimal repro I could achieve is this:
https://godbolt.org/z/dcrae4639
This while loop doesn't execute the ldr(line 5 in the 0.12 asm) again each loop on arm or riscv32 since at least 0.13, which causes an infinite loop if polling a machine register for a change. In 0.12 it could be worked around by populating the loop.
In the latest versions it can still be worked around by putting a memory clobber in the loop. Notably this doesn't repro on x86 but does repro on at least aarch64, thumb, and riscv32.
This affects the mmio.Mmio struct used for accessing machine registers in Microzig in some cases, so does have some impact for common embedded use cases.
Copy of code in case godbolt gets lost:
Expected Behavior
Expect loads through
*volatileto always be treated as volatile.