[lldb][TypeSystemClang] Create FileIDs/SourceLocations from DWARF

Author: Michael137

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

@@ -11,6 +11,7 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/ExprCXX.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
 #include "llvm/Support/FormatAdapters.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/SmallVectorMemoryBuffer.h"
@@ -9945,3 +9946,35 @@ void TypeSystemClang::LogCreation() const {
     LLDB_LOG(log, "Created new TypeSystem for (ASTContext*){0:x} '{1}'",
              &getASTContext(), getDisplayName());
 }
+
+clang::SourceLocation TypeSystemClang::GetLocForDecl(const Declaration &decl) {
+  // If the Declaration is invalid there is nothing to do.
+  if (!decl.IsValid())
+    return {};
+
+  clang::SourceManager &sm = getASTContext().getSourceManager();
+  clang::FileManager &fm = sm.getFileManager();
+
+  auto fe = fm.getFileRef(decl.GetFile().GetPath());
+  if (!fe) {
+    llvm::consumeError(fe.takeError());
+    return {};
+  }
+
+  clang::FileID fid = sm.translateFile(*fe);
+  if (fid.isInvalid()) {
+    // We see the file for the first time, so create a dummy file for it now.
+
+    // Connect the new dummy file to the main file via some fake include
+    // location. This is necessary as all file's in the SourceManager need to be
+    // reachable via an include chain from the main file.
+    SourceLocation ToIncludeLocOrFakeLoc;
+    assert(sm.getMainFileID().isValid());
+    ToIncludeLocOrFakeLoc = sm.getLocForStartOfFile(sm.getMainFileID());
+    fid = sm.createFileID(*fe, ToIncludeLocOrFakeLoc, clang::SrcMgr::C_User);
+  }
+
+  // Clang requires column numbers to be >= 1..
+  return sm.translateLineCol(fid, decl.GetLine(),
+                             std::max<uint16_t>(decl.GetColumn(), 1));
+}

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

@@ -1165,6 +1165,15 @@ class TypeSystemClang : public TypeSystem {
 
   bool SetDeclIsForcefullyCompleted(const clang::TagDecl *td);
 
+  /// Turns the given \c decl into a \c clang::SourceLocation.
+  ///
+  /// Will create a \c FileID in this \c DWARFASTParserClang's \c ASTContext
+  /// if necessary.
+  ///
+  /// If no \c FileID could be found/created, returns an empty \c
+  /// SourceLocation.
+  clang::SourceLocation GetLocForDecl(const lldb_private::Declaration &decl);
+
 private:
   /// Returns the PrintingPolicy used when generating the internal type names.
   /// These type names are mostly used for the formatter selection.
@@ -1189,19 +1198,6 @@ class TypeSystemClang : public TypeSystem {
   std::optional<uint64_t> GetObjCBitSize(clang::QualType qual_type,
                                          ExecutionContextScope *exe_scope);
 
-  /// Turns the given \c decl into a \c clang::SourceLocation.
-  ///
-  /// Will create a \c FileID in this \c DWARFASTParserClang's \c ASTContext
-  /// if necessary.
-  ///
-  /// If no \c FileID could be found/created, returns an empty \c
-  /// SourceLocation.
-  ///
-  /// FIXME: currently a no-op.
-  clang::SourceLocation GetLocForDecl(const lldb_private::Declaration &decl) {
-    return {};
-  }
-
   // Classes that inherit from TypeSystemClang can see and modify these
   std::string m_target_triple;
   std::unique_ptr<clang::ASTContext> m_ast_up;

lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

@@ -15,6 +15,8 @@ def setUp(self):
 
         self.main_source = "main.cpp"
         self.main_source_spec = lldb.SBFileSpec(self.main_source)
+        self.main_source_diag_path = os.path.join("diagnostics", "main.cpp")
+        self.header_source_diag_path = os.path.join("diagnostics", "lib.h")
 
     def test_source_and_caret_printing(self):
         """Test that the source and caret positions LLDB prints are correct"""
@@ -113,9 +115,6 @@ def test_source_and_caret_printing(self):
             value.GetError().GetCString(),
         )
 
-        # Declarations from the debug information currently have no debug information. It's not clear what
-        # we should do in this case, but we should at least not print anything that's wrong.
-        # In the future our declarations should have valid source locations.
         value = frame.EvaluateExpression("struct FooBar { double x };", top_level_opts)
         self.assertFalse(value.GetError().Success())
         self.assertIn(
@@ -140,10 +139,13 @@ def test_source_and_caret_printing(self):
             """
     1 | foo(1, 2)
       | ^~~
-note: candidate function not viable: requires single argument 'x', but 2 arguments were provided
 """,
             value.GetError().GetCString(),
         )
+        self.assertIn(
+            "candidate function not viable: requires single argument 'x', but 2 arguments were provided",
+            value.GetError().GetCString(),
+        )
 
         # Redefine something that we defined in a user-expression. We should use the previous expression file name
         # for the original decl.
@@ -260,3 +262,116 @@ def check_error(diags):
         self.assertEqual(err_ty.GetIntegerValue(), lldb.eErrorTypeExpression)
         diags = data.GetValueForKey("errors").GetItemAtIndex(0)
         check_error(diags)
+
+    def test_source_locations_from_debug_information(self):
+        """Test that the source locations from debug information are correct"""
+        self.build()
+
+        (_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+            self, "// Break here", self.main_source_spec
+        )
+
+        frame = thread.GetFrameAtIndex(0)
+
+        # Test source locations of functions
+        value = frame.EvaluateExpression("foo(1, 2)")
+        self.assertFalse(value.GetError().Success())
+        error_msg = value.GetError().GetCString()
+        self.assertIn(
+            """:1:1: no matching function for call to 'foo'
+    1 | foo(1, 2)
+      | ^~~
+""",
+            error_msg,
+        )
+
+        self.assertIn(
+            """
+    1 | void foo() {}
+      | ^
+""",
+            error_msg,
+        )
+
+        self.assertIn(
+            """
+    2 | void foo(char, char const *x) {}
+      | ^
+""",
+            error_msg,
+        )
+
+        self.assertIn(
+            """
+    3 | void foo(int x) {}
+      | ^
+""",
+            error_msg,
+        )
+
+        self.assertIn(f"{self.header_source_diag_path}:", error_msg)
+        self.assertIn(f"{self.main_source_diag_path}:", error_msg)
+
+        top_level_opts = lldb.SBExpressionOptions()
+        top_level_opts.SetTopLevel(True)
+
+        # Test source locations of records
+        value = frame.EvaluateExpression("struct FooBar { double x; };", top_level_opts)
+        self.assertFalse(value.GetError().Success())
+        error_msg = value.GetError().GetCString()
+        self.assertIn(
+            """:1:8: redefinition of 'FooBar'
+    1 | struct FooBar { double x; };
+""",
+            error_msg,
+        )
+        self.assertIn(
+            """previous definition is here
+    5 | struct FooBar {
+      | ^
+""",
+            error_msg,
+        )
+        self.assertIn(f"{self.main_source_diag_path}:", error_msg)
+
+        # Test source locations of enums
+        value = frame.EvaluateExpression("enum class EnumInSource {};", top_level_opts)
+        self.assertFalse(value.GetError().Success())
+        error_msg = value.GetError().GetCString()
+        self.assertIn(
+            """previous declaration is here
+    9 | enum class EnumInSource { A, B, C };
+      | ^
+""",
+            error_msg,
+        )
+        self.assertIn(f"{self.main_source_diag_path}:", error_msg)
+
+    @skipIf(
+        debug_info=no_match("dsym"),
+        bugnumber="Template function decl can only be found via dsym",
+    )
+    def test_source_locations_from_debug_information_template_func(self):
+        """Test that the source locations from debug information are correct
+        for template functions"""
+        self.build()
+
+        (_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+            self, "// Break here", self.main_source_spec
+        )
+
+        frame = thread.GetFrameAtIndex(0)
+
+        # Test source locations of template functions
+        value = frame.EvaluateExpression("templateFunc<int>(1)")
+        self.assertFalse(value.GetError().Success())
+        error_msg = value.GetError().GetCString()
+        self.assertIn(
+            """11:1: non-template declaration found by name lookup
+   11 | template <typename T> void templateFunc() {}
+      | ^
+""",
+            error_msg,
+        )
+
+        self.assertIn(f"{self.main_source_diag_path}:", error_msg)

lldb/test/API/commands/expression/diagnostics/lib.h

@@ -0,0 +1,2 @@
+void foo() {}
+void foo(char, char const *x) {}

lldb/test/API/commands/expression/diagnostics/main.cpp

@@ -1,11 +1,22 @@
+#include "lib.h"
+
 void foo(int x) {}
 
 struct FooBar {
   int i;
 };
 
+enum class EnumInSource { A, B, C };
+
+template <typename T> void templateFunc() {}
+
 int main() {
   FooBar f;
   foo(1);
+  foo('c', "abc");
+  foo();
+  EnumInSource e = EnumInSource::A;
+  templateFunc<int>();
+
   return 0; // Break here
 }

lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py

@@ -10,11 +10,12 @@
 
 
 class ExprInsideLambdaTestCase(TestBase):
-    def expectExprError(self, expr: str, expected: str):
+    def expectExprError(self, expr: str, expected_errors: str):
         frame = self.thread.GetFrameAtIndex(0)
         value = frame.EvaluateExpression(expr)
         errmsg = value.GetError().GetCString()
-        self.assertIn(expected, errmsg)
+        for expected in expected_errors:
+            self.assertIn(expected, errmsg)
 
     def test_expr_inside_lambda(self):
         """Test that lldb evaluating expressions inside lambda expressions works correctly."""
@@ -76,7 +77,7 @@ def test_expr_inside_lambda(self):
         self.expect_expr("local_var", result_type="int", result_value="137")
 
         # Check access to variable in previous frame which we didn't capture
-        self.expectExprError("local_var_copy", "use of undeclared identifier")
+        self.expectExprError("local_var_copy", ["use of undeclared identifier"])
 
         lldbutil.continue_to_breakpoint(process, bkpt)
 
@@ -110,19 +111,17 @@ def test_expr_inside_lambda(self):
         # Check access to outer top-level structure's members
         self.expectExprError(
             "class_var",
-            ("use of non-static data member" " 'class_var' of 'Foo' from nested type"),
+            ["use of non-static data member 'class_var' of 'Foo' from nested type"],
         )
 
-        self.expectExprError(
-            "base_var", ("use of non-static data member" " 'base_var'")
-        )
+        self.expectExprError("base_var", ["use of non-static data member 'base_var'"])
 
         self.expectExprError(
             "local_var",
-            (
-                "use of non-static data member 'local_var'"
-                " of '(unnamed class)' from nested type 'LocalLambdaClass'"
-            ),
+            [
+                "use of non-static data member 'local_var' of '(unnamed class at",
+                "from nested type 'LocalLambdaClass'",
+            ],
         )
 
         # Inside non_capturing_method
@@ -133,5 +132,5 @@ def test_expr_inside_lambda(self):
 
         self.expectExprError(
             "class_var",
-            ("use of non-static data member" " 'class_var' of 'Foo' from nested type"),
+            ["use of non-static data member 'class_var' of 'Foo' from nested type"],
         )

lldb/test/API/lang/cpp/function-local-class/TestCppFunctionLocalClass.py

@@ -34,16 +34,8 @@ def test(self):
             result_type="TypedefUnnamed2",
             result_children=[ValueCheck(name="b", value="3")],
         )
-        self.expect_expr(
-            "unnamed",
-            result_type="(unnamed struct)",
-            result_children=[ValueCheck(name="i", value="4")],
-        )
-        self.expect_expr(
-            "unnamed2",
-            result_type="(unnamed struct)",
-            result_children=[ValueCheck(name="j", value="5")],
-        )
+        self.expect("expression unnamed", substrs=["(unnamed struct at", "i = 4"])
+        self.expect("expression unnamed2", substrs=["(unnamed struct at", "j = 5"])
 
         # Try a class that is only forward declared.
         self.expect_expr("fwd", result_type="Forward *")

lldb/test/Shell/Settings/TestFrameFormatName.test

@@ -16,7 +16,7 @@ run
 c
 # NAME_WITH_ARGS: frame int ns::foo<int ()>(str="bar")
 c
-# NAME_WITH_ARGS: frame int ns::foo<(anonymous namespace)::$_0>(t=(anonymous namespace)::(unnamed class) @ {{.*}})
+# NAME_WITH_ARGS: frame int ns::foo<(anonymous namespace)::$_0>(t=(anonymous namespace)::(unnamed class at {{.*}}) @ {{.*}})
 c
 # NAME_WITH_ARGS: frame int ns::foo<int (*)()>(t=({{.*}}`(anonymous namespace)::anon_bar() at {{.*}}))
 c

lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp

@@ -15,7 +15,7 @@ struct A {
   } C;
 } a;
 
-// CHECK: A::(anonymous struct)
+// CHECK: A::(anonymous struct at
 // CHECK: |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal
-// CHECK: A::(unnamed struct)
+// CHECK: A::(unnamed struct at
 // CHECK: |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal

lldb/unittests/Symbol/CMakeLists.txt

@@ -27,5 +27,8 @@ add_lldb_unittest(SymbolTests
 
 set(test_inputs
   inlined-functions.yaml
+  dummy.cpp
+  dummy_header1.h
+  dummy_header2.h
   )
 add_unittest_inputs(SymbolTests "${test_inputs}")