Skip to content

Commit 21507c2

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 f4bfbdb commit 21507c2

File tree

4 files changed

+103
-0
lines changed

4 files changed

+103
-0
lines changed

src/Compilation.zig

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3255,6 +3255,7 @@ fn processOneJob(comp: *Compilation, job: Job) !void {
32553255
module.semaPkg(pkg) catch |err| switch (err) {
32563256
error.OutOfMemory => return error.OutOfMemory,
32573257
error.AnalysisFail => return,
3258+
else => unreachable,
32583259
};
32593260
},
32603261
.glibc_crt_file => |crt_file| {

src/Sema.zig

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4258,6 +4258,75 @@ 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+
4273+
var lhs_index = Air.refToIndex(bin_op.lhs) orelse continue;
4274+
var bitcast_index: ?usize = null;
4275+
{
4276+
if (air_tags[lhs_index] == .bitcast) {
4277+
const lhs = air_datas[lhs_index].ty_op.operand;
4278+
lhs_index = Air.refToIndex(lhs).?;
4279+
var i = block_index;
4280+
while (i > 0) : (i -= 1) {
4281+
if (block.instructions.items[i] == lhs_index) {
4282+
bitcast_index = i;
4283+
break;
4284+
}
4285+
} else continue;
4286+
}
4287+
}
4288+
4289+
const index = switch (air_tags[lhs_index]) {
4290+
.struct_field_ptr_index_0 => 0,
4291+
.struct_field_ptr_index_1 => 1,
4292+
.struct_field_ptr_index_2 => 2,
4293+
.struct_field_ptr_index_3 => 3,
4294+
.struct_field_ptr => blk: {
4295+
const ty_pl = air_datas[lhs_index].ty_pl;
4296+
const struct_field = sema.getTmpAir().extraData(Air.StructField, ty_pl.payload).data;
4297+
break :blk struct_field.field_index;
4298+
},
4299+
else => continue,
4300+
};
4301+
4302+
// Delete dead store, field_ptr and bitcast instructions.
4303+
_ = block.instructions.orderedRemove(store_index);
4304+
if (bitcast_index) |some| _ = block.instructions.orderedRemove(some);
4305+
block_index = block.instructions.items.len - 1;
4306+
while (block_index > 0) : (block_index -= 1) {
4307+
if (block.instructions.items[block_index] == lhs_index) {
4308+
_ = block.instructions.orderedRemove(block_index);
4309+
break;
4310+
}
4311+
}
4312+
4313+
field_ptr_count -= 1;
4314+
element_refs[index] = bin_op.rhs;
4315+
continue;
4316+
}
4317+
4318+
assert(field_ptr_count == 0); // Unable to eliminate all stores.
4319+
for (found_fields) |field_ptr, i| {
4320+
if (field_ptr != 0) continue;
4321+
4322+
element_refs[i] = try sema.addConstant(struct_ty.structFieldType(i), field_values[i]);
4323+
}
4324+
4325+
const init = try block.addAggregateInit(struct_ty, element_refs);
4326+
try sema.storePtr2(block, init_src, struct_ptr, init_src, init, init_src, .store);
4327+
return;
4328+
}
4329+
42614330
// Our task is to insert `store` instructions for all the default field values.
42624331
for (found_fields) |field_ptr, i| {
42634332
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)