OOB panic when reading inactive field of a `comptime var extern union`

[jayschwa]

Zig Version

0.12.0-dev.614-ab3ac1e67

Steps to Reproduce and Observed Behavior

More stuff discovered while working on PR #17286. To be honest, I'm not sure why the zeroes test is using comptime var vs const, but it matters for triggering the panic.

const std = @import("std");
const expectEqual = std.testing.expectEqual;

test {
 const U = extern union {
 a: u8,
 b: u64,
 };
 comptime var u = blk: {
 var u: U = undefined;
 @memset(std.mem.asBytes(&u), 0);
 u.a = 0;
 break :blk u;
 };
 try expectEqual(@as(u8, 0), u.a);
 try expectEqual(@as(u64, 0), u.b);
}

thread 204736 panic: index out of bounds: index 8, len 1
Analyzing test.zig: test.zig:test_0
 %57 = load(%16) node_offset:18:34 to :18:35
 %58 = dbg_stmt(15, 35)
 > %59 = field_val(%57, "b") node_offset:18:34 to :18:37
 %60 = break_inline(%54, %59)
 For full context, use the command
 zig ast-check -t test.zig

 in test.zig: test.zig:test_0
 > %54 = call(.auto, %52, [
 {%55, %56},
 {%57..%60},
 ]) node_offset:18:9 to :18:38
 in test.zig: test.zig:test_0
 > %10 = block({%11..%68}) node_offset:4:6 to :4:6

/home/jayschwa/zig/src/value.zig:915:67: 0x15c6154 in readFromMemory (zig)
 const val = std.mem.readVarInt(u64, buffer[0..byte_count], endian);
 ^
/home/jayschwa/zig/src/Sema.zig:30661:32: 0x1783130 in bitCastVal (zig)
 return Value.readFromMemory(new_ty, mod, buffer[buffer_offset..], sema.arena) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Sema.zig:27215:44: 0x1b24547 in unionFieldVal (zig)
 if (try sema.bitCastVal(block, src, un.val.toValue(), old_ty, field_ty, 0)) |new_val| {
 ^
/home/jayschwa/zig/src/Sema.zig:26255:38: 0x16d8447 in fieldVal (zig)
 return sema.unionFieldVal(block, src, object, field_name, field_name_src, inner_ty);
 ^
/home/jayschwa/zig/src/Sema.zig:9927:25: 0x1113f46 in zirFieldVal (zig)
 return sema.fieldVal(block, src, object, field_name, field_name_src);
 ^
/home/jayschwa/zig/src/Sema.zig:1060:66: 0xde1301 in analyzeBodyInner (zig)
 .field_val => try sema.zirFieldVal(block, inst),
 ^
/home/jayschwa/zig/src/Sema.zig:934:45: 0xba3371 in analyzeBodyBreak (zig)
 const break_inst = sema.analyzeBodyInner(block, body) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Sema.zig:883:50: 0x16bfad3 in resolveBody (zig)
 const break_data = (try sema.analyzeBodyBreak(block, body)) orelse
 ^
/home/jayschwa/zig/src/Sema.zig:7014:59: 0x1b0c175 in analyzeArg (zig)
 const uncoerced_arg = try sema.resolveBody(block, arg_body, zir_call.call_inst);
 ^
/home/jayschwa/zig/src/Sema.zig:7942:49: 0x1b0ef96 in instantiateGenericCall (zig)
 const arg_ref = try args_info.analyzeArg(sema, block, arg_index, param_ty, generic_owner_ty_info, func);
 ^
/home/jayschwa/zig/src/Sema.zig:7242:40: 0x16c6445 in analyzeCall (zig)
 if (sema.instantiateGenericCall(
 ^
/home/jayschwa/zig/src/Sema.zig:6725:43: 0x1108b94 in zirCall__anon_151125 (zig)
 const call_inst = try sema.analyzeCall(block, func, func_ty, callee_src, call_src, modifier, ensure_result_used, args_info, call_dbg_node, .call);
 ^
/home/jayschwa/zig/src/Sema.zig:1029:62: 0xddfbc0 in analyzeBodyInner (zig)
 .call => try sema.zirCall(block, inst, .direct),
 ^
/home/jayschwa/zig/src/Sema.zig:5887:34: 0x177158b in resolveBlockBody (zig)
 if (sema.analyzeBodyInner(child_block, body)) |_| {
 ^
/home/jayschwa/zig/src/Sema.zig:5870:33: 0x11dc4a0 in zirBlock (zig)
 return sema.resolveBlockBody(parent_block, src, &child_block, body, inst, &label.merges);
 ^
/home/jayschwa/zig/src/Sema.zig:1578:49: 0xdf096d in analyzeBodyInner (zig)
 break :blk try sema.zirBlock(block, inst, tags[inst] == .block_comptime);
 ^
/home/jayschwa/zig/src/Sema.zig:917:30: 0x10a53c8 in analyzeBody (zig)
 _ = sema.analyzeBodyInner(block, body) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Module.zig:4847:21: 0xdbd826 in analyzeFnBody (zig)
 sema.analyzeBody(&inner_block, fn_info.body) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Module.zig:3442:40: 0xb834cd in ensureFuncBodyAnalyzed (zig)
 var air = mod.analyzeFnBody(func_index, sema_arena) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Compilation.zig:3392:42: 0xb8158d in processOneJob (zig)
 module.ensureFuncBodyAnalyzed(func) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/Compilation.zig:3329:30: 0x9ee36b in performAllTheWork (zig)
 try processOneJob(comp, work_item, main_progress_node);
 ^
/home/jayschwa/zig/src/Compilation.zig:2194:31: 0x9e98fc in update (zig)
 try comp.performAllTheWork(main_progress_node);
 ^
/home/jayschwa/zig/src/main.zig:4177:24: 0xa18940 in updateModule (zig)
 try comp.update(main_progress_node);
 ^
/home/jayschwa/zig/src/main.zig:3593:17: 0xa3ba96 in buildOutputType (zig)
 updateModule(comp) catch |err| switch (err) {
 ^
/home/jayschwa/zig/src/main.zig:275:31: 0x8920a7 in mainArgs (zig)
 return buildOutputType(gpa, arena, args, .zig_test);
 ^
/home/jayschwa/zig/src/main.zig:213:20: 0x88f105 in main (zig)
 return mainArgs(gpa, arena, args);
 ^
/home/jayschwa/zig/lib/std/start.zig:486:37: 0x88eb1e in main (zig)
 std.os.argv = @as([*][*:0]u8, @ptrCast(c_argv))[0..@as(usize, @intCast(c_argc))];
 ^
../sysdeps/nptl/libc_start_call_main.h:58:16: 0x7f4d55829d8f in __libc_start_call_main (../sysdeps/x86/libc-start.c)
../csu/libc-start.c:392:3: 0x7f4d55829e3f in __libc_start_main_impl (../sysdeps/x86/libc-start.c)
???:?:?: 0x88e724 in ??? (???)
???:?:?: 0x0 in ??? (???)
Aborted (core dumped)

Expected Behavior

Success, because extern union is allowed to reinterpret memory.

[jayschwa]

I'm having trouble squaring these two pieces of code:

zig/src/InternPool.zig

Lines 1103 to 1110 in 937138c

	pub const Union = extern struct {
	/// This is the union type; not the field type.
	ty: Index,
	/// Indicates the active field. This could be `none`, which indicates the tag is not known. `none` is only a valid value for extern and packed unions.
	/// In those cases, the type of `val` is:
	/// extern: a u8 array of the same byte length as the union
	/// packed: an unsigned integer with the same bit size as the union
	tag: Index,

https://github.com/ziglang/zig/blob/937138cb90ae9327b0f7a932910c8d6080984f5c/src/Sema.zig#L27233-L27240

The InternPool.Union comment says that tag should only be none for extern and packed. But the extern and packed branch in unionFieldVal is checking it as if it can be something else (and it is).

[jayschwa]

Okay, I may have misread that comment. none may be used as a tag, but is not necessarily the only tag that can be used.

[jayschwa]

I can no longer reproduce this issue after using a newer build. PR #17286 includes some test changes that will exercise this case.

[Vexu]

It still crashes for me using the latest master (873c695).

[kamidev]

@Vexu A release build of (873c695) works just fine for me. But I do get the same crash with a debug build.

➜ zig ~/src/zig/zig/build/stage3/bin/zig test 17311.zig
All 1 tests passed.
➜ zig ~/src/zig/zig/build/stage4/bin/zig test 17311.zig
thread 511024 panic: index out of bounds: index 8, len 1
Analyzing 17311.zig: 17311.zig:test_0
 %57 = load(%16)
 %58 = dbg_stmt(13, 35)
 > %59 = field_val(%57, "b")
 %60 = break_inline(%54, %59)
 For full context, use the command
 zig ast-check -t 17311.zig
...

[kcbanner]

I've been working on a fix for this, the specific problem is writing a smaller field then reading a larger field from the union at comptime.

Writing the smaller field sets the entire union's value to that field. This is a problem when a larger field is read because there are not enough bytes in the backing storage. The write of the smaller field should only mutate the bytes it covers.

[jayschwa]

Yeah, when I was investigating this a few days ago, it seemed like behavior that should only apply to native Zig union was also being used for extern union, which is supposed to be more loosey-goosey. When I closed the issue, I wasn't able to repro with either a release or a debug build. Weird. I'll try again later.