Skip to content

Commit

Permalink
Fix misaligned store in Bool to union upcasts (#14906)
Browse files Browse the repository at this point in the history
The code path for `Nil` looks similar, but it is perfectly fine: it directly stores `[8 x i64] zeroinitializer` to the data field, whose default alignment naturally matches.
  • Loading branch information
HertzDevil authored and straight-shoota committed Aug 20, 2024
1 parent d63d459 commit 77314b0
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
19 changes: 19 additions & 0 deletions spec/compiler/codegen/union_type_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -215,4 +215,23 @@ describe "Code gen: union type" do
Union(Nil, Int32).foo
)).to_string.should eq("TupleLiteral")
end

it "respects union payload alignment when upcasting Bool (#14898)" do
mod = codegen(<<-CRYSTAL)
x = uninitialized Bool | UInt8[64]
x = true
CRYSTAL

str = mod.to_s
{% if LibLLVM::IS_LT_150 %}
str.should contain("store i512 1, i512* %2, align 8")
{% else %}
str.should contain("store i512 1, ptr %1, align 8")
{% end %}

# an i512 store defaults to 16-byte alignment, which is undefined behavior
# as it overestimates the actual alignment of `x`'s data field (x86 in
# particular segfaults on misaligned 16-byte stores)
str.should_not contain("align 16")
end
end
9 changes: 6 additions & 3 deletions src/compiler/crystal/codegen/unions.cr
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,19 @@ module Crystal

def store_bool_in_union(target_type, union_pointer, value)
struct_type = llvm_type(target_type)
union_value_type = struct_type.struct_element_types[1]
store type_id(value, @program.bool), union_type_id(struct_type, union_pointer)

# To store a boolean in a union
# we sign-extend it to the size in bits of the union
union_size = @llvm_typer.size_of(struct_type.struct_element_types[1])
# we zero-extend it to the size in bits of the union
union_size = @llvm_typer.size_of(union_value_type)
int_type = llvm_context.int((union_size * 8).to_i32)

bool_as_extended_int = builder.zext(value, int_type)
casted_value_ptr = pointer_cast(union_value(struct_type, union_pointer), int_type.pointer)
store bool_as_extended_int, casted_value_ptr
inst = store bool_as_extended_int, casted_value_ptr
set_alignment(inst, @llvm_typer.align_of(union_value_type))
inst
end

def store_nil_in_union(target_type, union_pointer)
Expand Down

0 comments on commit 77314b0

Please sign in to comment.