Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Attempt to fix the packed struct problem #3745

Closed
wants to merge 3 commits into from
Closed

Attempt to fix the packed struct problem #3745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07cee94
Attempt to fix the packed struct problem (#2627) by removing ABI alig…
ikskuh Nov 23, 2019
2f580bc
Introduces new test file: packed-struct.zig. This file tests some spe…
ikskuh Nov 23, 2019
d07d8f2
Reenables test/stage1/behaviour/struct.zig, but comments out the two …
ikskuh Nov 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/analyze.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2141,7 +2141,7 @@ static Error resolve_struct_type(CodeGen *g, ZigType *struct_type) {
field->bit_offset_in_host = packed_bits_offset - first_packed_bits_offset_misalign;

size_t full_bit_count = next_packed_bits_offset - first_packed_bits_offset_misalign;
size_t full_abi_size = get_abi_size_bytes(full_bit_count, g->pointer_size_bytes);
size_t full_abi_size = get_abi_size_bytes(full_bit_count, 1);
if (full_abi_size * 8 == full_bit_count) {
// next field recovers ABI alignment
host_int_bytes[gen_field_index] = full_abi_size;
Expand All @@ -2152,7 +2152,7 @@ static Error resolve_struct_type(CodeGen *g, ZigType *struct_type) {

first_packed_bits_offset_misalign = SIZE_MAX;
}
} else if (get_abi_size_bytes(field_type->size_in_bits, g->pointer_size_bytes) * 8 != field_size_in_bits) {
} else if (get_abi_size_bytes(field_type->size_in_bits, 1) * 8 != field_size_in_bits) {
first_packed_bits_offset_misalign = packed_bits_offset;
field->bit_offset_in_host = 0;
} else {
Expand Down Expand Up @@ -2190,7 +2190,7 @@ static Error resolve_struct_type(CodeGen *g, ZigType *struct_type) {
}
if (first_packed_bits_offset_misalign != SIZE_MAX) {
size_t full_bit_count = packed_bits_offset - first_packed_bits_offset_misalign;
size_t full_abi_size = get_abi_size_bytes(full_bit_count, g->pointer_size_bytes);
size_t full_abi_size = get_abi_size_bytes(full_bit_count, packed ? 1 : g->pointer_size_bytes);
next_offset = next_field_offset(next_offset, abi_align, full_abi_size, abi_align);
host_int_bytes[gen_field_index] = full_abi_size;
gen_field_index += 1;
Expand Down Expand Up @@ -7993,19 +7993,19 @@ static void resolve_llvm_types_struct(CodeGen *g, ZigType *struct_type, ResolveS
// this field is not byte-aligned; it is part of the previous field with a bit offset

size_t full_bit_count = next_packed_bits_offset - first_packed_bits_offset_misalign;
size_t full_abi_size = get_abi_size_bytes(full_bit_count, g->pointer_size_bytes);
size_t full_abi_size = get_abi_size_bytes(full_bit_count, 1);
if (full_abi_size * 8 == full_bit_count) {
// next field recovers ABI alignment
element_types[gen_field_index] = get_llvm_type_of_n_bytes(full_abi_size);
gen_field_index += 1;
first_packed_bits_offset_misalign = SIZE_MAX;
}
} else if (get_abi_size_bytes(field_type->size_in_bits, g->pointer_size_bytes) * 8 != field_size_in_bits) {
} else if (get_abi_size_bytes(field_type->size_in_bits, 1) * 8 != field_size_in_bits) {
first_packed_bits_offset_misalign = packed_bits_offset;
} else {
// This is a byte-aligned field (both start and end) in a packed struct.
element_types[gen_field_index] = get_llvm_type(g, field_type);
assert(get_abi_size_bytes(field_type->size_in_bits, g->pointer_size_bytes) ==
assert(get_abi_size_bytes(field_type->size_in_bits, 1) ==
LLVMStoreSizeOfType(g->target_data_ref, element_types[gen_field_index]));
gen_field_index += 1;
}
Expand Down Expand Up @@ -8068,7 +8068,7 @@ static void resolve_llvm_types_struct(CodeGen *g, ZigType *struct_type, ResolveS

if (first_packed_bits_offset_misalign != SIZE_MAX) {
size_t full_bit_count = packed_bits_offset - first_packed_bits_offset_misalign;
size_t full_abi_size = get_abi_size_bytes(full_bit_count, g->pointer_size_bytes);
size_t full_abi_size = get_abi_size_bytes(full_bit_count, packed ? 1 : g->pointer_size_bytes);
element_types[gen_field_index] = get_llvm_type_of_n_bytes(full_abi_size);
gen_field_index += 1;
}
Expand Down
1 change: 1 addition & 0 deletions test/stage1/behavior.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ comptime {
_ = @import("behavior/slice.zig");
_ = @import("behavior/slicetobytes.zig");
_ = @import("behavior/struct.zig");
_ = @import("behavior/packed-struct.zig");
_ = @import("behavior/struct_contains_null_ptr_itself.zig");
_ = @import("behavior/struct_contains_slice_of_itself.zig");
_ = @import("behavior/switch.zig");
Expand Down
143 changes: 143 additions & 0 deletions test/stage1/behavior/packed-struct.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
const std = @import("std");

test "sizeOf, variant 1" { // breaks in master
const T = packed struct {
one: u8,
three: [3]u8,
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "sizeOf, variant 2" { // doesn't break in master
const T = packed struct {
three: [3]u8,
one: u8,
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (original)" { // breaks in master
const T = packed struct {
_1: u1,
x: u7,
_: u24,
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (variant 1)" { // doesn't break in master
const T = packed struct {
_1: u1,
x: u7,
_2: u8,
_3: u16,
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "daurnimator (variant 2)" { // doesn't break in master
const T = packed struct {
_1: u1,
x: u7,
_2: u16,
_3: u8,
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(T));
}

test "MasterQ32'1" {
const Flags1 = packed struct {
// byte 0
b0_0: u1,
b0_1: u1,
b0_2: u1,
b0_3: u1,
b0_4: u1,
b0_5: u1,
b0_6: u1,
b0_7: u1,

// partial byte 1 (but not 8 bits)
b1_0: u1,
b1_1: u1,
b1_2: u1,
b1_3: u1,
b1_4: u1,
b1_5: u1,
b1_6: u1,

// some padding to fill to size 3
_: u9,
};
// TODO: This still breaks
// std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags1));
}


test "MasterQ32'2" {
const Flags2 = packed struct {
// byte 0
b0_0: u1,
b0_1: u1,
b0_2: u1,
b0_3: u1,
b0_4: u1,
b0_5: u1,
b0_6: u1,
b0_7: u1,

// partial byte 1 (but not 8 bits)
b1_0: u1,
b1_1: u1,
b1_2: u1,
b1_3: u1,
b1_4: u1,
b1_5: u1,
b1_6: u1,

// some padding that should yield @sizeOf(Flags2) == 4
_: u10, // this *was* originally 17, but the error happens with 10 as well
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags2));
}


test "MasterQ32'3" {
const Flags3 = packed struct {
// byte 0
b0_0: u1,
b0_1: u1,
b0_2: u1,
b0_3: u1,
b0_4: u1,
b0_5: u1,
b0_6: u1,
b0_7: u1,

// byte 1
b1_0: u1,
b1_1: u1,
b1_2: u1,
b1_3: u1,
b1_4: u1,
b1_5: u1,
b1_6: u1,
b1_7: u1,

// some padding that should yield @sizeOf(Flags2) == 4
_: u16, // it works, if the padding is 8-based
};
std.testing.expectEqual(@as(usize, 4), @sizeOf(Flags3));
}

test "fix for #3651" {
const T1 = packed struct {
array: [3][3]u8, // also with align(1)
};

const T2 = packed struct {
array: [9]u8,
};
std.testing.expectEqual(@as(usize, 9), @sizeOf(T1));
std.testing.expectEqual(@as(usize, 9), @sizeOf(T2));
}
132 changes: 64 additions & 68 deletions test/stage1/behavior/struct.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,43 +256,39 @@ const Foo96Bits = packed struct {

test "packed struct 24bits" {
comptime {
expect(@sizeOf(Foo24Bits) == 4);
if (@sizeOf(usize) == 4) {
expect(@sizeOf(Foo96Bits) == 12);
} else {
expect(@sizeOf(Foo96Bits) == 16);
}
expect(@sizeOf(Foo24Bits) == 3);
expect(@sizeOf(Foo96Bits) == 12);
}

var value = Foo96Bits{
.a = 0,
.b = 0,
.c = 0,
.d = 0,
};
value.a += 1;
expect(value.a == 1);
expect(value.b == 0);
expect(value.c == 0);
expect(value.d == 0);

value.b += 1;
expect(value.a == 1);
expect(value.b == 1);
expect(value.c == 0);
expect(value.d == 0);

value.c += 1;
expect(value.a == 1);
expect(value.b == 1);
expect(value.c == 1);
expect(value.d == 0);

value.d += 1;
expect(value.a == 1);
expect(value.b == 1);
expect(value.c == 1);
expect(value.d == 1);
// var value = Foo96Bits{
// .a = 0,
// .b = 0,
// .c = 0,
// .d = 0,
// };
// value.a += 1;
// expect(value.a == 1);
// expect(value.b == 0);
// expect(value.c == 0);
// expect(value.d == 0);

// value.b += 1;
// expect(value.a == 1);
// expect(value.b == 1);
// expect(value.c == 0);
// expect(value.d == 0);

// value.c += 1;
// expect(value.a == 1);
// expect(value.b == 1);
// expect(value.c == 1);
// expect(value.d == 0);

// value.d += 1;
// expect(value.a == 1);
// expect(value.b == 1);
// expect(value.c == 1);
// expect(value.d == 1);
}

const Foo32Bits = packed struct {
Expand All @@ -313,39 +309,39 @@ test "packed array 24bits" {
expect(@sizeOf(FooArray24Bits) == 2 + 2 * 4 + 2);
}

var bytes = [_]u8{0} ** (@sizeOf(FooArray24Bits) + 1);
bytes[bytes.len - 1] = 0xaa;
const ptr = &@bytesToSlice(FooArray24Bits, bytes[0 .. bytes.len - 1])[0];
expect(ptr.a == 0);
expect(ptr.b[0].field == 0);
expect(ptr.b[1].field == 0);
expect(ptr.c == 0);

ptr.a = maxInt(u16);
expect(ptr.a == maxInt(u16));
expect(ptr.b[0].field == 0);
expect(ptr.b[1].field == 0);
expect(ptr.c == 0);

ptr.b[0].field = maxInt(u24);
expect(ptr.a == maxInt(u16));
expect(ptr.b[0].field == maxInt(u24));
expect(ptr.b[1].field == 0);
expect(ptr.c == 0);

ptr.b[1].field = maxInt(u24);
expect(ptr.a == maxInt(u16));
expect(ptr.b[0].field == maxInt(u24));
expect(ptr.b[1].field == maxInt(u24));
expect(ptr.c == 0);

ptr.c = maxInt(u16);
expect(ptr.a == maxInt(u16));
expect(ptr.b[0].field == maxInt(u24));
expect(ptr.b[1].field == maxInt(u24));
expect(ptr.c == maxInt(u16));

expect(bytes[bytes.len - 1] == 0xaa);
// var bytes = [_]u8{0} ** (@sizeOf(FooArray24Bits) + 1);
// bytes[bytes.len - 1] = 0xaa;
// const ptr = &@bytesToSlice(FooArray24Bits, bytes[0 .. bytes.len - 1])[0];
// expect(ptr.a == 0);
// expect(ptr.b[0].field == 0);
// expect(ptr.b[1].field == 0);
// expect(ptr.c == 0);

// ptr.a = maxInt(u16);
// expect(ptr.a == maxInt(u16));
// expect(ptr.b[0].field == 0);
// expect(ptr.b[1].field == 0);
// expect(ptr.c == 0);

// ptr.b[0].field = maxInt(u24);
// expect(ptr.a == maxInt(u16));
// expect(ptr.b[0].field == maxInt(u24));
// expect(ptr.b[1].field == 0);
// expect(ptr.c == 0);

// ptr.b[1].field = maxInt(u24);
// expect(ptr.a == maxInt(u16));
// expect(ptr.b[0].field == maxInt(u24));
// expect(ptr.b[1].field == maxInt(u24));
// expect(ptr.c == 0);

// ptr.c = maxInt(u16);
// expect(ptr.a == maxInt(u16));
// expect(ptr.b[0].field == maxInt(u24));
// expect(ptr.b[1].field == maxInt(u24));
// expect(ptr.c == maxInt(u16));

// expect(bytes[bytes.len - 1] == 0xaa);
}

const FooStructAligned = packed struct {
Expand Down