Skip to content

Commit

Permalink
Always use 0 for offsets of lib / extern union members (#13305)
Browse files Browse the repository at this point in the history
  • Loading branch information
HertzDevil authored Apr 13, 2023
1 parent 469b5b2 commit 5dc9424
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 17 deletions.
27 changes: 27 additions & 0 deletions spec/compiler/codegen/debug_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,33 @@ describe "Code gen: debug" do
), debug: Crystal::Debug::All)
end

it "codegens lib union (#7335)" do
codegen <<-CRYSTAL, debug: Crystal::Debug::All
lib Foo
union Bar
a : Int32
b : Int16
c : Int8
end
end
x = Foo::Bar.new
CRYSTAL
end

it "codegens extern union (#7335)" do
codegen <<-CRYSTAL, debug: Crystal::Debug::All
@[Extern(union: true)]
struct Foo
@a = uninitialized Int32
@b = uninitialized Int16
@c = uninitialized Int8
end
x = Foo.new
CRYSTAL
end

it "inlines instance var access through getter in debug mode" do
run(%(
struct Bar
Expand Down
17 changes: 17 additions & 0 deletions spec/compiler/codegen/offsetof_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,21 @@ describe "Code gen: offsetof" do

run(code).to_b.should be_true
end

it "returns offset of extern union" do
run(<<-CRYSTAL).to_b.should be_true
@[Extern(union: true)]
struct Foo
@x = 1.0_f32
@y = uninitialized UInt32
def y
@y
end
end
f = Foo.new
(pointerof(f).as(Void*) + offsetof(Foo, @y).to_i64).as(UInt32*).value == f.y
CRYSTAL
end
end
17 changes: 17 additions & 0 deletions spec/debug/extern_unions.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@[Extern(union: true)]
struct Foo
@x : Float32
@y = uninitialized UInt32
@z = uninitialized UInt8[4]

def initialize(@x)
end
end

raise "wrong endianness" unless IO::ByteFormat::SystemEndian == IO::ByteFormat::LittleEndian

x = Foo.new(1.0_f32)
# print: x
# lldb-check: $0 = (x = 1065353216, y = 1, z = "\0\0\x80?")
# gdb-check: $1 = {x = 1065353216, y = 1, z = "\000\000\200?"}
debugger
3 changes: 3 additions & 0 deletions src/compiler/crystal/codegen/codegen.cr
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,13 @@ module Crystal
end

def offset_of(type, element_index)
return 0_u64 if type.extern_union?
llvm_typer.offset_of(llvm_typer.llvm_type(type), element_index)
end

def instance_offset_of(type, element_index)
# extern unions must be value types, which always use the above
# `offset_of` instead
llvm_typer.offset_of(llvm_typer.llvm_struct_type(type), element_index + 1)
end
end
Expand Down
41 changes: 24 additions & 17 deletions src/compiler/crystal/codegen/debug.cr
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,20 @@ module Crystal
@debug_files_per_module[@llvm_mod] ||= {} of DebugFilename => LibLLVM::MetadataRef
end

def current_debug_file
filename = @current_debug_location.try(&.filename) || "??"
debug_files_cache[filename] ||= begin
file, dir = file_and_dir(filename)
di_builder.create_file(file, dir)
end
private def current_debug_file
# These debug files are only used for `DIBuilder#create_union_type`, even
# though they are unneeded here, just as struct types don't need a file;
# LLVM 12 or below produces an assertion failure that is now removed
# (https://github.com/llvm/llvm-project/commit/ad60802a7187aa39b0374536be3fa176fe3d6256)
{% if LibLLVM::IS_LT_130 %}
filename = @current_debug_location.try(&.filename) || "??"
debug_files_cache[filename] ||= begin
file, dir = file_and_dir(filename)
di_builder.create_file(file, dir)
end
{% else %}
Pointer(Void).null.as(LibLLVM::MetadataRef)
{% end %}
end

def get_debug_type(type, original_type : Type)
Expand Down Expand Up @@ -148,21 +156,23 @@ module Crystal
ivars.each_with_index do |(name, ivar), idx|
next if ivar.type.is_a?(NilType)
if (ivar_type = ivar.type?) && (ivar_debug_type = get_debug_type(ivar_type))
offset = @program.target_machine.data_layout.offset_of_element(struct_type, idx &+ (type.struct? ? 0 : 1))
offset = type.extern_union? ? 0_u64 : @program.target_machine.data_layout.offset_of_element(struct_type, idx &+ (type.struct? ? 0 : 1))
size = @program.target_machine.data_layout.size_in_bits(llvm_embedded_type(ivar_type))

# FIXME structs like LibC::PthreadMutexT generate huge offset values
next if offset > UInt64::MAX // 8u64

member = di_builder.create_member_type(nil, name[1..-1], nil, 1, size, size, 8u64 * offset, LLVM::DIFlags::Zero, ivar_debug_type)
element_types << member
end
end

size = @program.target_machine.data_layout.size_in_bits(struct_type)
debug_type = di_builder.create_struct_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, nil, di_builder.get_or_create_type_array(element_types))
unless type.struct?
debug_type = di_builder.create_pointer_type(debug_type, 8u64 * llvm_typer.pointer_size, 8u64 * llvm_typer.pointer_size, original_type.to_s)
elements = di_builder.get_or_create_type_array(element_types)
if type.extern_union?
debug_type = di_builder.create_union_type(nil, original_type.to_s, current_debug_file, 1, size, size, LLVM::DIFlags::Zero, elements)
else
debug_type = di_builder.create_struct_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, nil, elements)
unless type.struct?
debug_type = di_builder.create_pointer_type(debug_type, 8u64 * llvm_typer.pointer_size, 8u64 * llvm_typer.pointer_size, original_type.to_s)
end
end
di_builder.replace_temporary(tmp_debug_type, debug_type)
debug_type
Expand Down Expand Up @@ -257,7 +267,7 @@ module Crystal
if ivar_debug_type = get_debug_type(ivar_type)
offset = @program.target_machine.data_layout.offset_of_element(struct_type, idx &+ (type.struct? ? 0 : 1))
size = @program.target_machine.data_layout.size_in_bits(llvm_embedded_type(ivar_type))
next if offset > UInt64::MAX // 8u64 # TODO: Figure out why it is happening sometimes with offset

member = di_builder.create_member_type(nil, "[#{idx}]", nil, 1, size, size, 8u64 * offset, LLVM::DIFlags::Zero, ivar_debug_type)
element_types << member
end
Expand Down Expand Up @@ -286,9 +296,6 @@ module Crystal
offset = @program.target_machine.data_layout.offset_of_element(struct_type, idx &+ (type.struct? ? 0 : 1))
size = @program.target_machine.data_layout.size_in_bits(llvm_embedded_type(ivar_type))

# FIXME structs like LibC::PthreadMutexT generate huge offset values
next if offset > UInt64::MAX // 8u64

member = di_builder.create_member_type(nil, ivar.name, nil, 1, size, size, 8u64 * offset, LLVM::DIFlags::Zero, ivar_debug_type)
element_types << member
end
Expand Down
2 changes: 2 additions & 0 deletions src/llvm/target_data.cr
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ struct LLVM::TargetData
end

def offset_of_element(struct_type, element)
# element_count = LibLLVM.count_struct_element_types(struct_type)
# raise "Invalid element idx!" unless element < element_count
LibLLVM.offset_of_element(self, struct_type, element)
end

Expand Down

0 comments on commit 5dc9424

Please sign in to comment.