Improve debugging support (#8538)

* Rudimentary debug support is here. It allows LLDB to show Crystal variables properly in C lang style. To work properly some LLDB patching is needed due to the dereferencing bug in DWARF Expression Evaluator.

* Rolled back enforced NoInline in debug mode.

* Added comment to explain why we have to use C++ Language ID

* Added debug information for union and nullable types.

* Fixed some of the debug type glitches (MixedUnionType notoriously) as well as fixed line linfo for blocks.

* Refactored debug information logging as per bcardiff comments

* Added array types debugging with DWARF support.

* Some extra features was added to llvm module so now it is possible to access to any block or any instruction in the LLVM function.
Also some refactoring as done to show properly locations of block variables.

* Added debug support for Tuples, NamedTuples, TypeDefs and partial support for Symbols.

* Removed unnecessary debug logs and did some format tidy up.

* More of code tidy up.

* Moving LLVM functions to the class they belong to.

* Neating up the code as per comments of reviewers.

* Code cleanup as per Sija's comments.

* Moved lldb crystal formatter into etc/lldb
Added get_or_create_array_subrange_variable method to LLVM module.

* Rolled back previously  set_current_debug_location as they are breaking unit tests and not really beneficial.

* Fixes as per Sija's suggestions as well as fix for failed unit test due to the missed filename of the unit test

* autoformated source code

* Removed location code that is not needed and was redundant and also was breaking the unit tests.

* Code clean up as per Sija's comments.

* Typo fix! Thanks a lot, Sija!

* Some clean up to trigger the build to re-test Alpine Linux.

* Re-trigger build wth comment commit.

* Fixed formatting issues (who knew that ### is not allowed! )

* initial refactor to use proper debug alloca

* fixing of alloca to be on alloca block

* formatted code

* Bug fixing for proper debug implementation. It should work properly now for most of variables and "self" variable.

* Fix of code formatting.

* Code fixes and avoid generating debug info for naked functions

* Fixed function name typo that @Sija found

* Addressed PR comments and refactored back to Type for debug_type_hash.

* changed the order of 'if' branch execution for setup_fun when --debug flag is used as per PR comments from @RX14

* Removed method attributes irrelevant for debugging as per @RX14

* Refactored code as per suggestions from @Asterlite and @Sija

* Code refactoring as per @Sija suggestions

* Removed all unused methods that were implemented during debug support implementeation.

* Initial debug specs

Generate a lldb script from magic comments and check the output using FileCheck

* Cherrypicked debug driver from @bcardiff

Co-authored-by: Sergey Kuznetsov <[email protected]>
Co-authored-by: Brian J. Cardiff <[email protected]>

etc/lldb/crystal_formatters.py

@@ -0,0 +1,62 @@
+import lldb
+
+class CrystalArraySyntheticProvider:
+    def __init__(self, valobj, internal_dict):
+        self.valobj = valobj
+        self.buffer = None
+        self.size = 0
+
+    def update(self):
+        if self.valobj.type.is_pointer:
+            self.valobj = self.valobj.Dereference()
+        self.size = int(self.valobj.child[0].value)
+        self.type = self.valobj.type
+        self.buffer = self.valobj.child[2]
+
+    def num_children(self):
+        size = 0 if self.size is None else self.size
+        return size
+
+    def get_child_index(self, name):
+        try:
+            return int(name.lstrip('[').rstrip(']'))
+        except:
+            return -1
+
+    def get_child_at_index(self,index):
+        if index >= self.size:
+            return None        
+        try:
+            elementType = self.buffer.type.GetPointeeType()
+            offset = elementType.size * index
+            return self.buffer.CreateChildAtOffset('[' + str(index) + ']', offset, elementType)
+        except Exception as e:
+            print('Got exception %s' % (str(e)))
+            return None
+
+def findType(name, module):
+    cachedTypes = module.GetTypes()
+    for idx in range(cachedTypes.GetSize()):
+        type = cachedTypes.GetTypeAtIndex(idx)
+        if type.name == name:
+            return type
+    return None
+
+
+def CrystalString_SummaryProvider(value, dict):
+    error = lldb.SBError()
+    if value.TypeIsPointerType():
+        value = value.Dereference()
+    process = value.GetTarget().GetProcess()
+    byteSize = int(value.child[0].value)
+    len = int(value.child[1].value)
+    len = byteSize or len
+    strAddr = value.child[2].load_addr
+    val = process.ReadCStringFromMemory(strAddr, len + 1, error)
+    return '"%s"' % val
+
+
+def __lldb_init_module(debugger, dict):
+    debugger.HandleCommand('type synthetic add -l crystal_formatters.CrystalArraySyntheticProvider -x "^Array\(.+\)(\s*\**)?" -w Crystal')
+    debugger.HandleCommand('type summary add -F crystal_formatters.CrystalString_SummaryProvider -x "^(String|\(String \| Nil\))(\s*\**)?$" -w Crystal')
+    debugger.HandleCommand('type category enable Crystal')

spec/debug/driver.cr

@@ -0,0 +1,58 @@
+def each_line1(file)
+  File.read_lines(file).each_with_index do |line, index0|
+    yield line, index0 + 1
+  end
+end
+
+repo_base_dir = "#{__DIR__}/../../"
+tmp_build_dir = File.join(repo_base_dir, ".build")
+Dir.mkdir_p(tmp_build_dir)
+
+input = ARGV[0]
+
+bin = File.join(tmp_build_dir, "debug_test_case")
+debugger_script = File.join(tmp_build_dir, "./debugger.script")
+
+`#{repo_base_dir}/bin/crystal build --debug #{input} -o #{bin}`
+
+File.open(debugger_script, "w") do |script|
+  lldb_crystal_formatters = File.expand_path(File.join(repo_base_dir, "etc", "lldb", "crystal_formatters.py"))
+  script.puts "version"
+  script.puts "command script import #{lldb_crystal_formatters}"
+
+  each_line1(input) do |line, line_number|
+    if line.match(/# break\b/)
+      script.puts "breakpoint set --file #{input} --line #{line_number}"
+    end
+  end
+
+  script.puts "run"
+
+  each_line1(input) do |line|
+    if md = line.match(/# lldb-command: (.*)/)
+      script.puts md[1]
+    end
+  end
+end
+
+session_output_dir = File.join(repo_base_dir, "tmp", "debug")
+Dir.mkdir_p(session_output_dir)
+
+session_log = File.join(session_output_dir, File.basename(input, File.extname(input)) + ".lldb-session")
+session_assert = File.join(session_output_dir, File.basename(input, File.extname(input)) + ".lldb-assert")
+
+File.open(session_assert, "w") do |assert|
+  each_line1(input) do |line|
+    if md = line.match(/# lldb-command: (.*)/)
+      assert.puts "CHECK: (lldb) #{md[1]}"
+    elsif md = line.match(/# lldb-check: (.*)/)
+      assert.puts "CHECK-NEXT: #{md[1]}"
+    end
+  end
+end
+
+`/usr/bin/lldb -b --source #{debugger_script} #{bin} > #{session_log}`
+
+`FileCheck #{session_assert} < #{session_log}`
+
+exit $?.exit_code

spec/debug/strings.cr

@@ -0,0 +1,6 @@
+# NOTE: breakpoint on line 1 + next does not work
+a = "hello world" # break
+# lldb-command: n
+# lldb-command: print a
+# lldb-check: (String *) $0 = {{0x[0-9a-f]+}} "hello world"
+b = 0

spec/debug/test.sh

@@ -0,0 +1,34 @@
+#!/bin/sh
+
+# This file can be executed from the root of the working copy
+#
+# $ ./spec/debug/test.sh
+#
+# It will use the ./spec/debug/driver.cr program to execute
+# the files explicitly listed at the end of this file.
+#
+# Those files have magic comments to build a script for an lldb session
+# and a FileCheck file with assertions over that session.
+#
+# In ./tmp/debug you can find a dump of the session and the assertion file.
+#
+# The magic comments interpreted by the driver are:
+#
+#   * # break
+#   * # lldb-command:
+#   * # lldb-check:
+#
+
+set -euo pipefail
+
+SCRIPT_PATH="$(realpath "$0")"
+SCRIPT_ROOT="$(dirname "$SCRIPT_PATH")"
+
+BUILD_DIR=$SCRIPT_ROOT/../../.build
+crystal=$SCRIPT_ROOT/../../bin/crystal
+driver=$BUILD_DIR/debug_driver
+mkdir -p $BUILD_DIR
+$crystal build $SCRIPT_ROOT/driver.cr -o $driver
+
+$driver $SCRIPT_ROOT/top_level.cr
+$driver $SCRIPT_ROOT/strings.cr

spec/debug/top_level.cr

@@ -0,0 +1,8 @@
+# NOTE: breakpoint on line 1 + next does not work
+a = 42 # break
+# lldb-command: print a
+# lldb-check: (int) $0 = 0
+# lldb-command: n
+# lldb-command: print a
+# lldb-check: (int) $1 = 42
+b = 0

spec/spec_helper.cr

@@ -161,9 +161,13 @@ def assert_macro_internal(program, sub_node, macro_args, macro_body, expected, e
   result_pragmas.should eq(expected_pragmas) if expected_pragmas
 end
 
-def codegen(code, inject_primitives = true, debug = Crystal::Debug::None)
+def codegen(code, inject_primitives = true, debug = Crystal::Debug::None, filename = __FILE__)
   code = inject_primitives(code) if inject_primitives
-  node = parse code
+  parser = Parser.new(code)
+  parser.filename = filename
+  parser.wants_doc = false
+  node = parser.parse
+
   result = semantic node
   result.program.codegen(result.node, single_module: false, debug: debug)[""].mod
 end

src/compiler/crystal/codegen/codegen.cr

@@ -135,8 +135,9 @@ module Crystal
       # llvm value, so in a way it's "already loaded".
       # This field is true if that's the case.
       getter already_loaded : Bool
+      getter debug_variable_created : Bool
 
-      def initialize(@pointer, @type, @already_loaded = false)
+      def initialize(@pointer, @type, @already_loaded = false, @debug_variable_created = false)
       end
     end
 
@@ -1348,7 +1349,17 @@ module Crystal
     end
 
     def declare_var(var)
-      context.vars[var.name] ||= LLVMVar.new(var.no_returns? ? llvm_nil : alloca(llvm_type(var.type), var.name), var.type)
+      context.vars[var.name] ||= begin
+        pointer = var.no_returns? ? llvm_nil : alloca(llvm_type(var.type), var.name)
+        debug_variable_created =
+          if context.fun.naked?
+            # Naked functions must not have debug info associated with them
+            false
+          else
+            declare_variable(var.name, var.type, pointer, var.location)
+          end
+        LLVMVar.new(pointer, var.type, debug_variable_created: debug_variable_created)
+      end
     end
 
     def declare_lib_var(name, type, thread_local)
@@ -1715,8 +1726,20 @@ module Crystal
             is_arg = args.try &.any? { |arg| arg.name == var.name }
             next if is_arg
 
-            ptr = builder.alloca llvm_type(var_type), name
-            context.vars[name] = LLVMVar.new(ptr, var_type)
+            ptr = alloca llvm_type(var_type), name
+
+            location = var.location
+            if location.nil? && obj.is_a?(ASTNode)
+              location = obj.location
+            end
+
+            debug_variable_created =
+              if location && !context.fun.naked?
+                declare_variable name, var_type, ptr, location, alloca_block
+              else
+                false
+              end
+            context.vars[name] = LLVMVar.new(ptr, var_type, debug_variable_created: debug_variable_created)
 
             # Assign default nil for variables that are bound to the nil variable
             if bound_to_mod_nil?(var)

src/compiler/crystal/codegen/context.cr

@@ -3,6 +3,7 @@ require "./codegen"
 class Crystal::CodeGenVisitor
   class Context
     property fun : LLVM::Function
+    property fun_debug_params = [] of LibLLVMExt::Metadata
     property type : Type
     property vars : Hash(String, LLVMVar)
     property return_type : Type?
@@ -56,6 +57,10 @@ class Crystal::CodeGenVisitor
       context.closure_self = @closure_self
       context
     end
+
+    def add_fun_debug_param(debug_type)
+      fun_debug_params << debug_type if debug_type
+    end
   end
 
   def with_cloned_context(new_context = @context)