| author |  Jakub Konka <kubkon@jakubkonka.com> 2022-02-02 00:20:41+01:00 |
| committer | Jakub Konka <kubkon@jakubkonka.com> 2022-02-02 13:43:41+01:00 |
| log | 15ff891f044102ba6515cb07f40805452420ea24 |
| tree | 05e00c580057290dfcfccda0a8483668322aa11f |
| parent | 521bd2e94a3b32382b2d1de1e6185149032b49db |
stage2: pad out (non-packed) struct fields when lowering to bytes
* pad out (non-packed) struct fields when lowering to bytes to be
saved in the binary - prior to this change, fields would be
saved at non-aligned addresses leading to wrong accesses
* add a matching test case to `behavior/struct.zig` tests
* fix offset to field calculation in `struct_field_ptr` on `x86_64`3 files changed, 60 insertions(+), 4 deletions(-)
src- arch
- x86_64
- codegen.zig
- test
- behavior
src/arch/x86_64/CodeGen.zig+3-3
| ... | @@ -1801,13 +1801,12 @@ fn structFieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, inde | ... | @@ -1801,13 +1801,12 @@ fn structFieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, inde |
| 1801 | const struct_field_offset = @intCast(u32, struct_ty.structFieldOffset(index, self.target.*)); | 1801 | const struct_field_offset = @intCast(u32, struct_ty.structFieldOffset(index, self.target.*)); |
| 1802 | const struct_field_ty = struct_ty.structFieldType(index); | 1802 | const struct_field_ty = struct_ty.structFieldType(index); |
| 1803 | const struct_field_size = @intCast(u32, struct_field_ty.abiSize(self.target.*)); | 1803 | const struct_field_size = @intCast(u32, struct_field_ty.abiSize(self.target.*)); |
| 1804 | const offset_to_field = struct_size - struct_field_offset - struct_field_size; | ||
| 1805 | 1804 | ||
| 1806 | const dst_mcv: MCValue = result: { | 1805 | const dst_mcv: MCValue = result: { |
| 1807 | switch (mcv) { | 1806 | switch (mcv) { |
| 1808 | .stack_offset => { | 1807 | .stack_offset => { |
| 1809 | const offset_reg = try self.copyToTmpRegister(ptr_ty, .{ | 1808 | const offset_reg = try self.copyToTmpRegister(ptr_ty, .{ |
| 1810 | .immediate = offset_to_field, | 1809 | .immediate = struct_field_offset, |
| 1811 | }); | 1810 | }); |
| 1812 | self.register_manager.freezeRegs(&.{offset_reg}); | 1811 | self.register_manager.freezeRegs(&.{offset_reg}); |
| 1813 | defer self.register_manager.unfreezeRegs(&.{offset_reg}); | 1812 | defer self.register_manager.unfreezeRegs(&.{offset_reg}); |
| ... | @@ -1817,12 +1816,13 @@ fn structFieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, inde | ... | @@ -1817,12 +1816,13 @@ fn structFieldPtr(self: *Self, inst: Air.Inst.Index, operand: Air.Inst.Ref, inde |
| 1817 | break :result dst_mcv; | 1816 | break :result dst_mcv; |
| 1818 | }, | 1817 | }, |
| 1819 | .ptr_stack_offset => |off| { | 1818 | .ptr_stack_offset => |off| { |
| 1819 | const offset_to_field = struct_size - struct_field_offset - struct_field_size; | ||
| 1820 | const ptr_stack_offset = off + @intCast(i32, offset_to_field); | 1820 | const ptr_stack_offset = off + @intCast(i32, offset_to_field); |
| 1821 | break :result MCValue{ .ptr_stack_offset = ptr_stack_offset }; | 1821 | break :result MCValue{ .ptr_stack_offset = ptr_stack_offset }; |
| 1822 | }, | 1822 | }, |
| 1823 | .register => |reg| { | 1823 | .register => |reg| { |
| 1824 | const offset_reg = try self.copyToTmpRegister(ptr_ty, .{ | 1824 | const offset_reg = try self.copyToTmpRegister(ptr_ty, .{ |
| 1825 | .immediate = offset_to_field, | 1825 | .immediate = struct_field_offset, |
| 1826 | }); | 1826 | }); |
| 1827 | self.register_manager.freezeRegs(&.{offset_reg}); | 1827 | self.register_manager.freezeRegs(&.{offset_reg}); |
| 1828 | defer self.register_manager.unfreezeRegs(&.{offset_reg}); | 1828 | defer self.register_manager.unfreezeRegs(&.{offset_reg}); |
src/codegen.zig+24-1
| ... | @@ -373,11 +373,24 @@ pub fn generateSymbol( | ... | @@ -373,11 +373,24 @@ pub fn generateSymbol( |
| 373 | }, | 373 | }, |
| 374 | .Struct => { | 374 | .Struct => { |
| 375 | // TODO debug info | 375 | // TODO debug info |
| 376 | // TODO padding of struct members | 376 | const struct_obj = typed_value.ty.castTag(.@"struct").?.data; |
| 377 | if (struct_obj.layout == .Packed) { | ||
| 378 | return Result{ | ||
| 379 | .fail = try ErrorMsg.create( | ||
| 380 | bin_file.allocator, | ||
| 381 | src_loc, | ||
| 382 | "TODO implement generateSymbol for packed struct", | ||
| 383 | .{}, | ||
| 384 | ), | ||
| 385 | }; | ||
| 386 | } | ||
| 387 | |||
| 388 | const struct_begin = code.items.len; | ||
| 377 | const field_vals = typed_value.val.castTag(.@"struct").?.data; | 389 | const field_vals = typed_value.val.castTag(.@"struct").?.data; |
| 378 | for (field_vals) |field_val, index| { | 390 | for (field_vals) |field_val, index| { |
| 379 | const field_ty = typed_value.ty.structFieldType(index); | 391 | const field_ty = typed_value.ty.structFieldType(index); |
| 380 | if (!field_ty.hasRuntimeBits()) continue; | 392 | if (!field_ty.hasRuntimeBits()) continue; |
| 393 | |||
| 381 | switch (try generateSymbol(bin_file, src_loc, .{ | 394 | switch (try generateSymbol(bin_file, src_loc, .{ |
| 382 | .ty = field_ty, | 395 | .ty = field_ty, |
| 383 | .val = field_val, | 396 | .val = field_val, |
| ... | @@ -388,6 +401,16 @@ pub fn generateSymbol( | ... | @@ -388,6 +401,16 @@ pub fn generateSymbol( |
| 388 | }, | 401 | }, |
| 389 | .fail => |em| return Result{ .fail = em }, | 402 | .fail => |em| return Result{ .fail = em }, |
| 390 | } | 403 | } |
| 404 | const unpadded_field_end = code.items.len - struct_begin; | ||
| 405 | |||
| 406 | // Pad struct members if required | ||
| 407 | const target = bin_file.options.target; | ||
| 408 | const padded_field_end = typed_value.ty.structFieldOffset(index + 1, target); | ||
| 409 | const padding = try math.cast(usize, padded_field_end - unpadded_field_end); | ||
| 410 | |||
| 411 | if (padding > 0) { | ||
| 412 | try code.writer().writeByteNTimes(0, padding); | ||
| 413 | } | ||
| 391 | } | 414 | } |
| 392 | 415 | ||
| 393 | return Result{ .appended = {} }; | 416 | return Result{ .appended = {} }; |
test/behavior/struct.zig+33
| ... | @@ -18,6 +18,39 @@ test "top level fields" { | ... | @@ -18,6 +18,39 @@ test "top level fields" { |
| 18 | try expect(@as(i32, 1235) == instance.top_level_field); | 18 | try expect(@as(i32, 1235) == instance.top_level_field); |
| 19 | } | 19 | } |
| 20 | 20 | ||
| 21 | const StructWithFields = struct { | ||
| 22 | a: u8, | ||
| 23 | b: u32, | ||
| 24 | c: u64, | ||
| 25 | d: u32, | ||
| 26 | |||
| 27 | fn first(self: *const StructWithFields) u8 { | ||
| 28 | return self.a; | ||
| 29 | } | ||
| 30 | |||
| 31 | fn second(self: *const StructWithFields) u32 { | ||
| 32 | return self.b; | ||
| 33 | } | ||
| 34 | |||
| 35 | fn third(self: *const StructWithFields) u64 { | ||
| 36 | return self.c; | ||
| 37 | } | ||
| 38 | |||
| 39 | fn fourth(self: *const StructWithFields) u32 { | ||
| 40 | return self.d; | ||
| 41 | } | ||
| 42 | }; | ||
| 43 | |||
| 44 | test "non-packed struct has fields padded out to the required alignment" { | ||
| 45 | if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; | ||
| 46 | |||
| 47 | const foo = StructWithFields{ .a = 5, .b = 1, .c = 10, .d = 2 }; | ||
| 48 | try expect(foo.first() == 5); | ||
| 49 | try expect(foo.second() == 1); | ||
| 50 | try expect(foo.third() == 10); | ||
| 51 | try expect(foo.fourth() == 2); | ||
| 52 | } | ||
| 53 | |||
| 21 | const StructWithNoFields = struct { | 54 | const StructWithNoFields = struct { |
| 22 | fn add(a: i32, b: i32) i32 { | 55 | fn add(a: i32, b: i32) i32 { |
| 23 | return a + b; | 56 | return a + b; |