commit 85837de476a919e42da101d42da848a5f181fb38 (tree)
parent 0a2f6632810ceb6a17bffbafd846c5f3aaa24e64
Author: Matthew Lugg <mlugg@mlugg.co.uk>
Date: Tue, 17 Mar 2026 19:38:27 +0000
llvm: solve a bunch of alignment bugs
I went over a bunch of calls to `std.zig.llvm.Builder.{load,store}` and
added correct alignments where we previously passed the default
alignment. This fixes some miscompilations, particularly when using
underaligned pointers or *overaligned* slices. I have definitely missed
some cases, but it's better to get some fixes in than for this to sit in
a `git stash` forever.
Resolves: https://codeberg.org/ziglang/zig/issues/31473
Resolves: https://codeberg.org/ziglang/zig/issues/30566
Diffstat:
| M | src/codegen/llvm.zig | | | 85 | ++++++++++++++++++++++++++++++++++++++++++++----------------------------------- |
1 file changed, 47 insertions(+), 38 deletions(-)
diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig
@@ -1438,8 +1438,7 @@ pub const Object = struct {
const param = wip.arg(llvm_arg_i);
llvm_arg_i += 1;
const field_ptr = try wip.gepStruct(llvm_ty, arg_ptr, field_i, "");
- const alignment =
- Builder.Alignment.fromByteUnits(@divExact(target.ptrBitWidth(), 8));
+ const alignment = Builder.Alignment.fromByteUnits(@divExact(target.ptrBitWidth(), 8));
_ = try wip.store(.normal, param, field_ptr, alignment);
}
@@ -6151,7 +6150,7 @@ pub const FuncGen = struct {
const body = unwrapped_try.else_body;
const err_union_ty = self.typeOf(unwrapped_try.error_union);
const is_unused = self.liveness.isUnused(inst);
- return lowerTry(self, err_union, body, err_union_ty, false, false, is_unused, err_cold);
+ return lowerTry(self, err_union, body, err_union_ty, false, .none, false, is_unused, err_cold);
}
fn airTryPtr(self: *FuncGen, inst: Air.Inst.Index, err_cold: bool) !Builder.Value {
@@ -6159,12 +6158,13 @@ pub const FuncGen = struct {
const unwrapped_try = self.air.unwrapTryPtr(inst);
const err_union_ptr = try self.resolveInst(unwrapped_try.error_union_ptr);
const body = unwrapped_try.else_body;
- const err_union_ty = self.typeOf(unwrapped_try.error_union_ptr).childType(zcu);
+ const err_union_ptr_ty = self.typeOf(unwrapped_try.error_union_ptr);
+ const err_union_ty = err_union_ptr_ty.childType(zcu);
const is_unused = self.liveness.isUnused(inst);
self.maybeMarkAllowZeroAccess(self.typeOf(unwrapped_try.error_union_ptr).ptrInfo(zcu));
- return lowerTry(self, err_union_ptr, body, err_union_ty, true, true, is_unused, err_cold);
+ return lowerTry(self, err_union_ptr, body, err_union_ty, true, err_union_ptr_ty.ptrAlignment(zcu), true, is_unused, err_cold);
}
fn lowerTry(
@@ -6173,6 +6173,7 @@ pub const FuncGen = struct {
body: []const Air.Inst.Index,
err_union_ty: Type,
operand_is_ptr: bool,
+ operand_ptr_align: InternPool.Alignment,
can_elide_load: bool,
is_unused: bool,
err_cold: bool,
@@ -6185,15 +6186,19 @@ pub const FuncGen = struct {
const err_union_llvm_ty = try o.lowerType(pt, err_union_ty);
const error_type = try o.errorIntType(pt);
+ const err_set_align: InternPool.Alignment, const payload_align: InternPool.Alignment = if (operand_is_ptr) .{
+ operand_ptr_align.minStrict(Type.anyerror.abiAlignment(zcu)),
+ operand_ptr_align.minStrict(payload_ty.abiAlignment(zcu)),
+ } else .{ .none, .none };
+
if (!err_union_ty.errorUnionSet(zcu).errorSetIsEmpty(zcu)) {
const loaded = loaded: {
const access_kind: Builder.MemoryAccessKind =
if (err_union_ty.isVolatilePtr(zcu)) .@"volatile" else .normal;
if (!payload_has_bits) {
- // TODO add alignment to this load
break :loaded if (operand_is_ptr)
- try fg.wip.load(access_kind, error_type, err_union, .default, "")
+ try fg.wip.load(access_kind, error_type, err_union, err_set_align.toLlvm(), "")
else
err_union;
}
@@ -6201,12 +6206,11 @@ pub const FuncGen = struct {
if (operand_is_ptr or isByRef(err_union_ty, zcu)) {
const err_field_ptr =
try fg.wip.gepStruct(err_union_llvm_ty, err_union, err_field_index, "");
- // TODO add alignment to this load
break :loaded try fg.wip.load(
if (operand_is_ptr) access_kind else .normal,
error_type,
err_field_ptr,
- .default,
+ err_set_align.toLlvm(),
"",
);
}
@@ -6232,15 +6236,14 @@ pub const FuncGen = struct {
return fg.wip.gepStruct(err_union_llvm_ty, err_union, offset, "");
} else if (isByRef(err_union_ty, zcu)) {
const payload_ptr = try fg.wip.gepStruct(err_union_llvm_ty, err_union, offset, "");
- const payload_alignment = payload_ty.abiAlignment(zcu).toLlvm();
if (isByRef(payload_ty, zcu)) {
if (can_elide_load)
return payload_ptr;
- return fg.loadByRef(payload_ptr, payload_ty, payload_alignment, .normal);
+ return fg.loadByRef(payload_ptr, payload_ty, payload_align.toLlvm(), .normal);
}
const load_ty = err_union_llvm_ty.structFields(&o.builder)[offset];
- return fg.wip.load(.normal, load_ty, payload_ptr, payload_alignment, "");
+ return fg.wip.load(.normal, load_ty, payload_ptr, payload_align.toLlvm(), "");
}
return fg.wip.extractValue(err_union, &.{offset}, "");
}
@@ -6713,20 +6716,20 @@ pub const FuncGen = struct {
const slice_ty = self.typeOf(bin_op.lhs);
const slice = try self.resolveInst(bin_op.lhs);
const index = try self.resolveInst(bin_op.rhs);
- const elem_ty = slice_ty.childType(zcu);
+ const slice_info = slice_ty.ptrInfo(zcu);
+ assert(slice_info.flags.size == .slice);
+ const elem_ty: Type = .fromInterned(slice_info.child);
const llvm_elem_ty = try o.lowerType(pt, elem_ty);
const base_ptr = try self.wip.extractValue(slice, &.{0}, "");
const ptr = try self.wip.gep(.inbounds, llvm_elem_ty, base_ptr, &.{index}, "");
+ const elem_align = slice_ty.ptrAlignment(zcu).min(elem_ty.abiAlignment(zcu));
+ const access_kind: Builder.MemoryAccessKind = if (slice_info.flags.is_volatile) .@"volatile" else .normal;
+ self.maybeMarkAllowZeroAccess(slice_info);
if (isByRef(elem_ty, zcu)) {
- self.maybeMarkAllowZeroAccess(slice_ty.ptrInfo(zcu));
-
- const slice_align = (slice_ty.ptrAlignment(zcu).min(elem_ty.abiAlignment(zcu))).toLlvm();
- return self.loadByRef(ptr, elem_ty, slice_align, if (slice_ty.isVolatilePtr(zcu)) .@"volatile" else .normal);
+ return self.loadByRef(ptr, elem_ty, elem_align.toLlvm(), access_kind);
+ } else {
+ return self.loadTruncate(access_kind, elem_ty, ptr, elem_align.toLlvm());
}
-
- self.maybeMarkAllowZeroAccess(slice_ty.ptrInfo(zcu));
-
- return self.load(ptr, slice_ty);
}
fn airSliceElemPtr(self: *FuncGen, inst: Air.Inst.Index) !Builder.Value {
@@ -7507,7 +7510,7 @@ pub const FuncGen = struct {
if (optional_ty.optionalReprIsPayload(zcu)) {
const loaded = if (operand_is_ptr)
- try self.wip.load(access_kind, optional_llvm_ty, operand, .default, "")
+ try self.wip.load(access_kind, optional_llvm_ty, operand, operand_ty.ptrAlignment(zcu).toLlvm(), "")
else
operand;
if (payload_ty.isSlice(zcu)) {
@@ -7525,7 +7528,7 @@ pub const FuncGen = struct {
if (!payload_ty.hasRuntimeBits(zcu)) {
const loaded = if (operand_is_ptr)
- try self.wip.load(access_kind, optional_llvm_ty, operand, .default, "")
+ try self.wip.load(access_kind, optional_llvm_ty, operand, operand_ty.ptrAlignment(zcu).toLlvm(), "")
else
operand;
return self.wip.icmp(cond, loaded, try o.builder.intValue(.i8, 0), "");
@@ -7568,7 +7571,7 @@ pub const FuncGen = struct {
if (!payload_ty.hasRuntimeBits(zcu)) {
const loaded = if (operand_is_ptr)
- try self.wip.load(access_kind, try o.lowerType(pt, err_union_ty), operand, .default, "")
+ try self.wip.load(access_kind, try o.lowerType(pt, err_union_ty), operand, operand_ty.ptrAlignment(zcu).toLlvm(), "")
else
operand;
return self.wip.icmp(cond, loaded, zero, "");
@@ -7578,9 +7581,13 @@ pub const FuncGen = struct {
const loaded = if (operand_is_ptr or isByRef(err_union_ty, zcu)) loaded: {
const err_union_llvm_ty = try o.lowerType(pt, err_union_ty);
+ const err_alignment = if (operand_is_ptr)
+ operand_ty.ptrAlignment(zcu).minStrict(Type.anyerror.abiAlignment(zcu))
+ else
+ .none;
const err_field_ptr =
try self.wip.gepStruct(err_union_llvm_ty, operand, err_field_index, "");
- break :loaded try self.wip.load(access_kind, error_type, err_field_ptr, .default, "");
+ break :loaded try self.wip.load(access_kind, error_type, err_field_ptr, err_alignment.toLlvm(), "");
} else try self.wip.extractValue(operand, &.{err_field_index}, "");
return self.wip.icmp(cond, loaded, zero, "");
}
@@ -7625,6 +7632,7 @@ pub const FuncGen = struct {
self.maybeMarkAllowZeroAccess(optional_ptr_ty.ptrInfo(zcu));
// We have a pointer to a i8. We need to set it to 1 and then return the same pointer.
+ // Default alignment store because align of the non null bit is 1 anyway.
_ = try self.wip.store(access_kind, non_null_bit, operand, .default);
return operand;
}
@@ -7640,7 +7648,7 @@ pub const FuncGen = struct {
self.maybeMarkAllowZeroAccess(optional_ptr_ty.ptrInfo(zcu));
- // TODO set alignment on this store
+ // Default alignment store because align of the non null bit is 1 anyway.
_ = try self.wip.store(access_kind, non_null_bit, non_null_ptr, .default);
// Then return the payload pointer (only if it's used).
@@ -7728,7 +7736,7 @@ pub const FuncGen = struct {
self.maybeMarkAllowZeroAccess(operand_ty.ptrInfo(zcu));
- return self.wip.load(access_kind, error_type, operand, .default, "");
+ return self.wip.load(access_kind, error_type, operand, operand_ty.ptrAlignment(zcu).toLlvm(), "");
}
const offset = try errUnionErrorOffset(payload_ty, pt);
@@ -7752,6 +7760,7 @@ pub const FuncGen = struct {
const operand = try self.resolveInst(ty_op.operand);
const err_union_ptr_ty = self.typeOf(ty_op.operand);
const err_union_ty = err_union_ptr_ty.childType(zcu);
+ const err_union_ptr_align = err_union_ptr_ty.ptrAlignment(zcu);
const payload_ty = err_union_ty.errorUnionPayload(zcu);
const non_error_val = try o.builder.intValue(try o.errorIntType(pt), 0);
@@ -7761,8 +7770,7 @@ pub const FuncGen = struct {
if (!payload_ty.hasRuntimeBits(zcu)) {
self.maybeMarkAllowZeroAccess(err_union_ptr_ty.ptrInfo(zcu));
-
- _ = try self.wip.store(access_kind, non_error_val, operand, .default);
+ _ = try self.wip.store(access_kind, non_error_val, operand, err_union_ptr_align.toLlvm());
return operand;
}
const err_union_llvm_ty = try o.lowerType(pt, err_union_ty);
@@ -7770,7 +7778,7 @@ pub const FuncGen = struct {
self.maybeMarkAllowZeroAccess(err_union_ptr_ty.ptrInfo(zcu));
const err_int_ty = try pt.errorIntType();
- const error_alignment = err_int_ty.abiAlignment(zcu).toLlvm();
+ const error_alignment = err_int_ty.abiAlignment(zcu).minStrict(err_union_ptr_align).toLlvm();
const error_offset = try errUnionErrorOffset(payload_ty, pt);
// First set the non-error value.
const non_null_ptr = try self.wip.gepStruct(err_union_llvm_ty, operand, error_offset, "");
@@ -8451,9 +8459,8 @@ pub const FuncGen = struct {
_ = try self.wip.store(.normal, result_val, field_ptr, result_alignment);
}
{
- const overflow_alignment = comptime Builder.Alignment.fromByteUnits(1);
const field_ptr = try self.wip.gepStruct(llvm_inst_ty, alloca_inst, overflow_index, "");
- _ = try self.wip.store(.normal, overflow_bit, field_ptr, overflow_alignment);
+ _ = try self.wip.store(.normal, overflow_bit, field_ptr, comptime .fromByteUnits(1));
}
return alloca_inst;
@@ -8813,9 +8820,8 @@ pub const FuncGen = struct {
_ = try self.wip.store(.normal, result, field_ptr, result_alignment);
}
{
- const field_alignment = comptime Builder.Alignment.fromByteUnits(1);
const field_ptr = try self.wip.gepStruct(llvm_dest_ty, alloca_inst, overflow_index, "");
- _ = try self.wip.store(.normal, overflow_bit, field_ptr, field_alignment);
+ _ = try self.wip.store(.normal, overflow_bit, field_ptr, comptime .fromByteUnits(1));
}
return alloca_inst;
}
@@ -9971,15 +9977,18 @@ pub const FuncGen = struct {
const union_ptr = try self.resolveInst(bin_op.lhs);
const new_tag = try self.resolveInst(bin_op.rhs);
+ const union_ptr_align = un_ptr_ty.ptrAlignment(zcu);
if (layout.payload_size == 0) {
- // TODO alignment on this store
- _ = try self.wip.store(access_kind, new_tag, union_ptr, .default);
+ _ = try self.wip.store(access_kind, new_tag, union_ptr, union_ptr_align.toLlvm());
return .none;
}
const tag_index = @intFromBool(layout.tag_align.compare(.lt, layout.payload_align));
const tag_field_ptr = try self.wip.gepStruct(try o.lowerType(pt, un_ty), union_ptr, tag_index, "");
- // TODO alignment on this store
- _ = try self.wip.store(access_kind, new_tag, tag_field_ptr, .default);
+ const tag_ptr_align: InternPool.Alignment = switch (layout.tagOffset()) {
+ 0 => union_ptr_align,
+ else => |off| .minStrict(union_ptr_align, .fromLog2Units(@ctz(off))),
+ };
+ _ = try self.wip.store(access_kind, new_tag, tag_field_ptr, tag_ptr_align.toLlvm());
return .none;
}
