From 966a76fee75d805104264468965bec8a98b3d6f0 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 04:41:40 +0800 Subject: [PATCH 1/9] extern unions in debug info --- spec/compiler/codegen/debug_spec.cr | 27 +++++++++++++++++++++++++++ spec/debug/extern_unions.cr | 17 +++++++++++++++++ src/compiler/crystal/codegen/debug.cr | 12 +++++++----- src/llvm/target_data.cr | 2 ++ 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 spec/debug/extern_unions.cr diff --git a/spec/compiler/codegen/debug_spec.cr b/spec/compiler/codegen/debug_spec.cr index 1903d1e58601..4a57056fc7a3 100644 --- a/spec/compiler/codegen/debug_spec.cr +++ b/spec/compiler/codegen/debug_spec.cr @@ -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 diff --git a/spec/debug/extern_unions.cr b/spec/debug/extern_unions.cr new file mode 100644 index 000000000000..66943926dcff --- /dev/null +++ b/spec/debug/extern_unions.cr @@ -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 diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index a363e994d1dd..50f3519fa518 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -148,19 +148,21 @@ 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)) + elements = di_builder.get_or_create_type_array(element_types) + debug_type = if type.extern_union? + di_builder.create_union_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, elements) + else + di_builder.create_struct_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, nil, elements) + end 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 diff --git a/src/llvm/target_data.cr b/src/llvm/target_data.cr index 150e193bc7cb..ed8d0c07ed91 100644 --- a/src/llvm/target_data.cr +++ b/src/llvm/target_data.cr @@ -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 From 7d264fbadc172a23538e6eaa5656cb0dcf362bf4 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 04:45:22 +0800 Subject: [PATCH 2/9] extern unions in `offsetof` --- spec/compiler/codegen/offsetof_spec.cr | 17 +++++++++++++++++ src/compiler/crystal/codegen/codegen.cr | 3 +++ 2 files changed, 20 insertions(+) diff --git a/spec/compiler/codegen/offsetof_spec.cr b/spec/compiler/codegen/offsetof_spec.cr index fa2a0dbe8b37..f7e7971be6b2 100644 --- a/spec/compiler/codegen/offsetof_spec.cr +++ b/spec/compiler/codegen/offsetof_spec.cr @@ -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 diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr index 562e52eaf1ff..3b3a81757bb8 100644 --- a/src/compiler/crystal/codegen/codegen.cr +++ b/src/compiler/crystal/codegen/codegen.cr @@ -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 From 59303c98b151f160a6df2dec95856a46d44f5149 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 05:10:22 +0800 Subject: [PATCH 3/9] remove fixme --- src/compiler/crystal/codegen/debug.cr | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index 50f3519fa518..ced1617706be 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -288,9 +288,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 From fec35d50583e332678a525c4b80ae27960776f69 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 05:12:48 +0800 Subject: [PATCH 4/9] cleanup --- src/compiler/crystal/codegen/debug.cr | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index ced1617706be..57e2d7626df9 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -158,13 +158,13 @@ module Crystal size = @program.target_machine.data_layout.size_in_bits(struct_type) elements = di_builder.get_or_create_type_array(element_types) - debug_type = if type.extern_union? - di_builder.create_union_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, elements) - else - di_builder.create_struct_type(nil, original_type.to_s, nil, 1, size, size, LLVM::DIFlags::Zero, nil, elements) - end - 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) + if type.extern_union? + debug_type = di_builder.create_union_type(nil, original_type.to_s, nil, 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 From ad548c4d2e5b63d405d5369a73073f82becf78fe Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 05:25:53 +0800 Subject: [PATCH 5/9] use first member offset instead for consistency --- src/compiler/crystal/codegen/codegen.cr | 2 +- src/compiler/crystal/codegen/debug.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr index 3b3a81757bb8..450340a85e6b 100644 --- a/src/compiler/crystal/codegen/codegen.cr +++ b/src/compiler/crystal/codegen/codegen.cr @@ -105,7 +105,7 @@ module Crystal end def offset_of(type, element_index) - return 0_u64 if type.extern_union? + element_index = element_index.class.zero if type.extern_union? llvm_typer.offset_of(llvm_typer.llvm_type(type), element_index) end diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index 57e2d7626df9..036fd82b0474 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -148,7 +148,7 @@ 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 = type.extern_union? ? 0_u64 : @program.target_machine.data_layout.offset_of_element(struct_type, idx &+ (type.struct? ? 0 : 1)) + offset = @program.target_machine.data_layout.offset_of_element(struct_type, type.extern_union? ? 0 : idx &+ (type.struct? ? 0 : 1)) size = @program.target_machine.data_layout.size_in_bits(llvm_embedded_type(ivar_type)) member = di_builder.create_member_type(nil, name[1..-1], nil, 1, size, size, 8u64 * offset, LLVM::DIFlags::Zero, ivar_debug_type) From e1f856e9a50ff32cd772ead17b4020135cac1b39 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 05:32:50 +0800 Subject: [PATCH 6/9] Revert "use first member offset instead for consistency" This reverts commit ad548c4d2e5b63d405d5369a73073f82becf78fe. --- src/compiler/crystal/codegen/codegen.cr | 2 +- src/compiler/crystal/codegen/debug.cr | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/crystal/codegen/codegen.cr b/src/compiler/crystal/codegen/codegen.cr index 450340a85e6b..3b3a81757bb8 100644 --- a/src/compiler/crystal/codegen/codegen.cr +++ b/src/compiler/crystal/codegen/codegen.cr @@ -105,7 +105,7 @@ module Crystal end def offset_of(type, element_index) - element_index = element_index.class.zero if type.extern_union? + return 0_u64 if type.extern_union? llvm_typer.offset_of(llvm_typer.llvm_type(type), element_index) end diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index 036fd82b0474..57e2d7626df9 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -148,7 +148,7 @@ 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, type.extern_union? ? 0 : 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)) member = di_builder.create_member_type(nil, name[1..-1], nil, 1, size, size, 8u64 * offset, LLVM::DIFlags::Zero, ivar_debug_type) From 5d43313c87cd227552dd35126f9fab80b2ca38d5 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 06:59:12 +0800 Subject: [PATCH 7/9] add `current_debug_file` --- src/compiler/crystal/codegen/debug.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index 57e2d7626df9..3fc597c290a9 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -159,7 +159,7 @@ module Crystal size = @program.target_machine.data_layout.size_in_bits(struct_type) 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, nil, 1, size, size, LLVM::DIFlags::Zero, elements) + 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? From 3ec35ab4da1e6cbec8314af9681f1919cea1ffbf Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 07:13:35 +0800 Subject: [PATCH 8/9] cleanup2 --- src/compiler/crystal/codegen/debug.cr | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index 3fc597c290a9..be9ea72adaa8 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -83,11 +83,19 @@ module Crystal 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 + # 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) @@ -259,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 From da38e42a66df7bbc2e6e5a6265f4c1f9cf3f2980 Mon Sep 17 00:00:00 2001 From: Quinton Miller Date: Wed, 12 Apr 2023 07:13:57 +0800 Subject: [PATCH 9/9] cleanup2 --- src/compiler/crystal/codegen/debug.cr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/crystal/codegen/debug.cr b/src/compiler/crystal/codegen/debug.cr index be9ea72adaa8..df1b33b23bd8 100644 --- a/src/compiler/crystal/codegen/debug.cr +++ b/src/compiler/crystal/codegen/debug.cr @@ -82,7 +82,7 @@ module Crystal @debug_files_per_module[@llvm_mod] ||= {} of DebugFilename => LibLLVM::MetadataRef end - def current_debug_file + 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