Fix debug info for closured variables (#16393)

Declares the debug variables as they are created in `#setup_closure_vars`, not later when the `alloca` block is being populated. The `DW_OP_plus_uconst` is necessary to resolve to the correct variable address during debug time, attaching a debug variable to a `getelementptr` instruction will not work.

Author: HertzDevil

spec/compiler/codegen/debug_spec.cr

@@ -331,4 +331,18 @@ describe "Code gen: debug" do
       Int32
       CRYSTAL
   end
+
+  it "doesn't fail if Proc self is closured (#16382)" do
+    codegen <<-CRYSTAL, debug: Crystal::Debug::All
+      struct Proc
+        def partial
+          -> do
+            self
+          end
+        end
+      end
+
+      -> { }.partial.call
+      CRYSTAL
+  end
 end

src/compiler/crystal/codegen/debug.cr

@@ -321,9 +321,9 @@ module Crystal
       end
     end
 
-    def declare_variable(var_name, var_type, alloca, location, basic_block : LLVM::BasicBlock? = nil)
+    def declare_variable(var_name, var_type, alloca, location, basic_block : LLVM::BasicBlock? = nil, offset : Int = 0)
       return false unless @debug.variables?
-      declare_local(var_type, alloca, location, basic_block) do |scope, file, line_number, debug_type|
+      declare_local(var_type, alloca, location, basic_block, offset) do |scope, file, line_number, debug_type|
         di_builder.create_auto_variable scope, var_name, file, line_number, debug_type, align_of(var_type)
       end
     end
@@ -332,7 +332,11 @@ module Crystal
       @program.target_machine.data_layout.abi_alignment(llvm_type(type)) * 8
     end
 
-    private def declare_local(type, alloca, location, basic_block : LLVM::BasicBlock? = nil, &)
+    # see also the other DWARF enums in `crystal/dwarf/abbrev.cr` (note that
+    # LLVM defines several custom opcodes outside the user extension range)
+    DW_OP_plus_uconst = 0x23
+
+    private def declare_local(type, alloca, location, basic_block : LLVM::BasicBlock? = nil, offset : Int = 0, &)
       location = location.try &.expanded_location
       return false unless location
 
@@ -346,7 +350,18 @@ module Crystal
       return false unless scope
 
       var = yield scope, file, location.line_number, debug_type
-      expr = di_builder.create_expression(nil, 0)
+
+      if offset != 0
+        expr =
+          {% if LibLLVM::IS_LT_140 %}
+            di_builder.create_expression(Int64.static_array(DW_OP_plus_uconst, offset), 2)
+          {% else %}
+            di_builder.create_expression(UInt64.static_array(DW_OP_plus_uconst, offset), 2)
+          {% end %}
+      else
+        expr = di_builder.create_expression(nil, 0)
+      end
+
       if basic_block
         block = basic_block
       else
@@ -355,16 +370,6 @@ module Crystal
       old_debug_location = @current_debug_location
       set_current_debug_location location
       if builder.current_debug_location != llvm_nil && (ptr = alloca)
-        # FIXME: When debug records are used instead of debug intrinsics, it
-        # seems inserting them into an empty BasicBlock will instead place them
-        # in a totally different (next?) function where the variable doesn't
-        # exist, leading to a "function-local metadata used in wrong function"
-        # validation error. This might happen when e.g. all variables inside a
-        # block are closured. Ideally every debug record should immediately
-        # follow the variable it declares.
-        {% unless LibLLVM::IS_LT_190 %}
-          call(do_nothing_fun) if block.instructions.empty?
-        {% end %}
         di_builder.insert_declare_at_end(ptr, var, expr, builder.current_debug_location_metadata, block)
         set_current_debug_location old_debug_location
         true

src/compiler/crystal/codegen/fun.cr

@@ -117,7 +117,7 @@ class Crystal::CodeGenVisitor
           void_ptr = context.fun.params.first
           closure_type = llvm_typer.copy_type(context.closure_type.not_nil!)
           closure_ptr = pointer_cast void_ptr, closure_type.pointer
-          setup_closure_vars target_def.vars, context.closure_vars.not_nil!, self.context, closure_type, closure_ptr
+          setup_closure_vars target_def, context.closure_vars.not_nil!, self.context, closure_type, closure_ptr
         else
           context.reset_closure
         end
@@ -459,11 +459,12 @@ class Crystal::CodeGenVisitor
     context.fun.add_attribute LLVM::Attribute::NoInline if target_def.no_inline?
   end
 
-  def setup_closure_vars(def_vars, closure_vars, context, closure_type, closure_ptr)
+  def setup_closure_vars(target_def, closure_vars, context, closure_type, closure_ptr)
     if context.closure_skip_parent
       parent_context = context.closure_parent_context.not_nil!
-      setup_closure_vars(def_vars, parent_context.closure_vars.not_nil!, parent_context, closure_type, closure_ptr)
+      setup_closure_vars(target_def, parent_context.closure_vars.not_nil!, parent_context, closure_type, closure_ptr)
     else
+      def_vars = target_def.vars
       closure_vars.each_with_index do |var, i|
         # A closured var in this context might have the same name as
         # a local var in another context, for example if the local var
@@ -472,20 +473,33 @@ class Crystal::CodeGenVisitor
         def_var = def_vars.try &.[var.name]?
         next if def_var && !def_var.closured?
 
-        self.context.vars[var.name] = LLVMVar.new(gep(closure_type, closure_ptr, 0, i, var.name), var.type)
+        if context.fun.naked?
+          debug_variable_created = false
+        else
+          var_offset = llvm_typer.offset_of(closure_type, i)
+          debug_variable_created = declare_variable(var.name, var.type, closure_ptr, target_def.location, offset: var_offset)
+        end
+        var_ptr = gep(closure_type, closure_ptr, 0, i, var.name)
+        self.context.vars[var.name] = LLVMVar.new(var_ptr, var.type, debug_variable_created: debug_variable_created)
       end
 
       if (closure_parent_context = context.closure_parent_context) &&
          (parent_vars = closure_parent_context.closure_vars)
         parent_closure_type = llvm_typer.copy_type(closure_parent_context.closure_type.not_nil!)
         parent_closure_ptr = gep(closure_type, closure_ptr, 0, closure_vars.size, "parent_ptr")
         parent_closure = load(parent_closure_type.pointer, parent_closure_ptr, "parent")
-        setup_closure_vars(def_vars, parent_vars, closure_parent_context, parent_closure_type, parent_closure)
+        setup_closure_vars(target_def, parent_vars, closure_parent_context, parent_closure_type, parent_closure)
       elsif closure_self = context.closure_self
-        offset = context.closure_parent_context ? 1 : 0
-        self_value = gep(closure_type, closure_ptr, 0, closure_vars.size + offset, "self")
-        self_value = load(llvm_type(closure_self), self_value) unless context.type.passed_by_value?
-        self.context.vars["self"] = LLVMVar.new(self_value, closure_self, true)
+        self_index = closure_vars.size + (context.closure_parent_context ? 1 : 0)
+        if context.fun.naked?
+          debug_variable_created = false
+        else
+          self_offset = llvm_typer.offset_of(closure_type, self_index)
+          debug_variable_created = declare_variable("self", closure_self, closure_ptr, target_def.location, offset: self_offset)
+        end
+        self_value = gep(closure_type, closure_ptr, 0, self_index, "self")
+        self_value = load(llvm_type(closure_self), self_value) unless closure_self.passed_by_value?
+        self.context.vars["self"] = LLVMVar.new(self_value, closure_self, true, debug_variable_created: debug_variable_created)
       end
     end
   end