From a859f94644cb362d84d3f9d72bc02b00a75fc32a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 14 Mar 2022 16:21:11 -0700 Subject: [PATCH 1/2] stage2: LLVM codegen of arrays should use type length, not value length It is possible for the value length to be longer than the type because we allow in-memory coercing of types such as `[5:0]u8` to `[5]u8`. In such a case, the value length is 6 but the type length if 5. The `.repeated` value type already got this right, so this is extending similar logic out to `.aggregate` and `.bytes`. Both scenarios are tested in behavior tests. Fixes #11165 --- src/codegen/llvm.zig | 7 +++--- test/behavior.zig | 1 + test/behavior/bugs/11165.zig | 42 ++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 test/behavior/bugs/11165.zig diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 4aba6de5a3..786f77d946 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -1507,7 +1507,7 @@ pub const DeclGen = struct { const bytes = tv.val.castTag(.bytes).?.data; return dg.context.constString( bytes.ptr, - @intCast(c_uint, bytes.len), + @intCast(c_uint, tv.ty.arrayLenIncludingSentinel()), .True, // don't null terminate. bytes has the sentinel, if any. ); }, @@ -1515,10 +1515,11 @@ pub const DeclGen = struct { const elem_vals = tv.val.castTag(.aggregate).?.data; const elem_ty = tv.ty.elemType(); const gpa = dg.gpa; - const llvm_elems = try gpa.alloc(*const llvm.Value, elem_vals.len); + const len = @intCast(usize, tv.ty.arrayLenIncludingSentinel()); + const llvm_elems = try gpa.alloc(*const llvm.Value, len); defer gpa.free(llvm_elems); var need_unnamed = false; - for (elem_vals) |elem_val, i| { + for (elem_vals[0..len]) |elem_val, i| { llvm_elems[i] = try dg.genTypedValue(.{ .ty = elem_ty, .val = elem_val }); need_unnamed = need_unnamed or dg.isUnnamedType(elem_ty, llvm_elems[i]); } diff --git a/test/behavior.zig b/test/behavior.zig index 7efe687c75..03d15645fa 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -62,6 +62,7 @@ test { _ = @import("behavior/bugs/11100.zig"); _ = @import("behavior/bugs/10970.zig"); _ = @import("behavior/bugs/11046.zig"); + _ = @import("behavior/bugs/11165.zig"); _ = @import("behavior/call.zig"); _ = @import("behavior/cast.zig"); _ = @import("behavior/comptime_memory.zig"); diff --git a/test/behavior/bugs/11165.zig b/test/behavior/bugs/11165.zig new file mode 100644 index 0000000000..d253c17b67 --- /dev/null +++ b/test/behavior/bugs/11165.zig @@ -0,0 +1,42 @@ +const builtin = @import("builtin"); + +test "bytes" { + const S = struct { + a: u32, + c: [5]u8, + }; + + const U = union { + s: S, + }; + + const s_1 = S{ + .a = undefined, + .c = "12345".*, // this caused problems + }; + _ = s_1; + + const u_2 = U{ .s = s_1 }; + _ = u_2; +} + +test "aggregate" { + const S = struct { + a: u32, + c: [5]u8, + }; + + const U = union { + s: S, + }; + + const c = [5:0]u8{ 1, 2, 3, 4, 5 }; + const s_1 = S{ + .a = undefined, + .c = c, // this caused problems + }; + _ = s_1; + + const u_2 = U{ .s = s_1 }; + _ = u_2; +} From 67647154c1c307dcf34413d013e6cd4a1df81945 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 14 Mar 2022 20:00:17 -0700 Subject: [PATCH 2/2] stage2: apply fix for #11165 to codegen.zig for native backends Co-authored-by: Cody Tapscott --- src/codegen.zig | 25 +++++++------------------ test/behavior/bugs/11165.zig | 10 ++++++++-- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/codegen.zig b/src/codegen.zig index 1a30738b07..6d5e140dca 100644 --- a/src/codegen.zig +++ b/src/codegen.zig @@ -207,29 +207,18 @@ pub fn generateSymbol( .bytes => { // TODO populate .debug_info for the array const payload = typed_value.val.castTag(.bytes).?; - if (typed_value.ty.sentinel()) |sentinel| { - try code.ensureUnusedCapacity(payload.data.len + 1); - code.appendSliceAssumeCapacity(payload.data); - switch (try generateSymbol(bin_file, src_loc, .{ - .ty = typed_value.ty.elemType(), - .val = sentinel, - }, code, debug_output, reloc_info)) { - .appended => return Result{ .appended = {} }, - .externally_managed => |slice| { - code.appendSliceAssumeCapacity(slice); - return Result{ .appended = {} }; - }, - .fail => |em| return Result{ .fail = em }, - } - } else { - return Result{ .externally_managed = payload.data }; - } + const len = @intCast(usize, typed_value.ty.arrayLenIncludingSentinel()); + // The bytes payload already includes the sentinel, if any + try code.ensureUnusedCapacity(len); + code.appendSliceAssumeCapacity(payload.data[0..len]); + return Result{ .appended = {} }; }, .aggregate => { // TODO populate .debug_info for the array const elem_vals = typed_value.val.castTag(.aggregate).?.data; const elem_ty = typed_value.ty.elemType(); - for (elem_vals) |elem_val| { + const len = @intCast(usize, typed_value.ty.arrayLenIncludingSentinel()); + for (elem_vals[0..len]) |elem_val| { switch (try generateSymbol(bin_file, src_loc, .{ .ty = elem_ty, .val = elem_val, diff --git a/test/behavior/bugs/11165.zig b/test/behavior/bugs/11165.zig index d253c17b67..cf2f9698f9 100644 --- a/test/behavior/bugs/11165.zig +++ b/test/behavior/bugs/11165.zig @@ -1,6 +1,9 @@ const builtin = @import("builtin"); test "bytes" { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + const S = struct { a: u32, c: [5]u8, @@ -16,11 +19,14 @@ test "bytes" { }; _ = s_1; - const u_2 = U{ .s = s_1 }; + var u_2 = U{ .s = s_1 }; _ = u_2; } test "aggregate" { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + const S = struct { a: u32, c: [5]u8, @@ -37,6 +43,6 @@ test "aggregate" { }; _ = s_1; - const u_2 = U{ .s = s_1 }; + var u_2 = U{ .s = s_1 }; _ = u_2; }