Skip to content

Commit 6b1ad6f

Browse files
committed
Sema: convert result location packed struct inits into aggregate inits
Initializing packed structs with result location is problematic since storing to a packed struct field pointer involves loading the field first which can cause loading uninitialized memory unless specifically zeroed first. Since packed structs are integer backed now using aggregate init makes more sense and avoids this issue. Closes ziglang#13480
1 parent 2c2a951 commit 6b1ad6f

File tree

3 files changed

+99
-0
lines changed

3 files changed

+99
-0
lines changed

src/Sema.zig

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4258,6 +4258,72 @@ fn validateStructInit(
42584258
}
42594259
try sema.resolveStructLayout(struct_ty);
42604260

4261+
if (struct_ty.containerLayout() == .Packed) {
4262+
// Convert stores to ptr into an aggregate init.
4263+
const element_refs = try sema.arena.alloc(Air.Inst.Ref, struct_ty.structFieldCount());
4264+
4265+
var block_index = block.instructions.items.len - 1;
4266+
var field_ptr_count = instrs.len;
4267+
while (block_index > 0 and field_ptr_count > 0) : (block_index -= 1) {
4268+
const store_inst = block.instructions.items[block_index];
4269+
if (air_tags[store_inst] != .store) continue;
4270+
const store_index = block_index;
4271+
const bin_op = air_datas[store_inst].bin_op;
4272+
var lhs_index = Air.refToIndex(bin_op.lhs) orelse continue;
4273+
var bitcast_index: ?usize = null;
4274+
{
4275+
if (air_tags[lhs_index] == .bitcast) {
4276+
const lhs = air_datas[lhs_index].ty_op.operand;
4277+
lhs_index = Air.refToIndex(lhs).?;
4278+
while (block_index > 0) : (block_index -= 1) {
4279+
if (block.instructions.items[block_index] == lhs_index) {
4280+
bitcast_index = block_index;
4281+
break;
4282+
}
4283+
} else continue;
4284+
}
4285+
}
4286+
const index = switch (air_tags[lhs_index]) {
4287+
.struct_field_ptr_index_0 => 0,
4288+
.struct_field_ptr_index_1 => 1,
4289+
.struct_field_ptr_index_2 => 2,
4290+
.struct_field_ptr_index_3 => 3,
4291+
.struct_field_ptr => blk: {
4292+
const ty_pl = air_datas[lhs_index].ty_pl;
4293+
const struct_field = sema.getTmpAir().extraData(Air.StructField, ty_pl.payload).data;
4294+
break :blk struct_field.field_index;
4295+
},
4296+
else => continue,
4297+
};
4298+
_ = block.instructions.orderedRemove(store_index);
4299+
block_index -= 1;
4300+
if (bitcast_index) |some| {
4301+
_ = block.instructions.orderedRemove(some);
4302+
block_index -= 1;
4303+
}
4304+
while (block_index > 0) : (block_index -= 1) {
4305+
if (block.instructions.items[block_index] == lhs_index) {
4306+
_ = block.instructions.orderedRemove(block_index);
4307+
block_index -= 1;
4308+
break;
4309+
}
4310+
}
4311+
field_ptr_count -= 1;
4312+
element_refs[index] = bin_op.rhs;
4313+
continue;
4314+
}
4315+
4316+
for (found_fields) |field_ptr, i| {
4317+
if (field_ptr != 0) continue;
4318+
4319+
element_refs[i] = try sema.addConstant(struct_ty.structFieldType(i), field_values[i]);
4320+
}
4321+
4322+
const init = try block.addAggregateInit(struct_ty, element_refs);
4323+
try sema.storePtr2(block, init_src, struct_ptr, init_src, init, init_src, .store);
4324+
return;
4325+
}
4326+
42614327
// Our task is to insert `store` instructions for all the default field values.
42624328
for (found_fields) |field_ptr, i| {
42634329
if (field_ptr != 0) continue;

test/behavior.zig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ test {
116116
_ = @import("behavior/bugs/13171.zig");
117117
_ = @import("behavior/bugs/13285.zig");
118118
_ = @import("behavior/bugs/13435.zig");
119+
_ = @import("behavior/bugs/13480.zig");
119120
_ = @import("behavior/byteswap.zig");
120121
_ = @import("behavior/byval_arg_var.zig");
121122
_ = @import("behavior/call.zig");

test/behavior/bugs/13480.zig

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
const builtin = @import("builtin");
2+
const std = @import("std");
3+
const expect = std.testing.expect;
4+
5+
const BitSet = packed struct {
6+
mask: u7,
7+
};
8+
fn smash() void {
9+
var stack: [0x500]u8 = undefined;
10+
@memset(&stack, 0xFF, stack.len);
11+
}
12+
fn stackUp() void {
13+
var space: [0x100]u8 = undefined;
14+
part2();
15+
_ = space;
16+
}
17+
fn part2() void {
18+
var a: u7 = 0b0001000;
19+
var mask_a = BitSet{
20+
.mask = a,
21+
};
22+
if (mask_a.mask != 0b0001000) unreachable;
23+
}
24+
test {
25+
if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
26+
if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
27+
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
28+
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
29+
30+
smash();
31+
stackUp();
32+
}

0 commit comments

Comments
 (0)