[Lldb-commits] [lldb] [lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed (PR #104817)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Tue Aug 20 10:42:24 PDT 2024


https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/104817

>From 4bb7f6495a2fbf46e01b9f01c46d335d2b680afd Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 19 Aug 2024 17:39:44 +0100
Subject: [PATCH 1/3] [lldb][ClangExpressionParser] Implement
 ExternalSemaSource::ReadUndefinedButUsed

While parsing an expression, Clang tries to diagnose usage of decls
(with possibly non-external linkage) for which it hasn't been provided
with a definition. This is the case, e.g., for functions with parameters
that live in an anonymous namespace (those will have `UniqueExternal`
linkage). Before diagnosing such situations, Clang calls
`ExternalSemaSource::ReadUndefinedButUsed`. The intended use of this
API is to extend the set of "used but not defined" decls with additional
ones that the external source knows about. However, in LLDB's case,
we never provide `FunctionDecl`s with a definition, and instead
rely on the expression parser to resolve those symbols by linkage name.
Thus, to avoid the Clang parser from erroring out in these situations,
this patch implements `ReadUndefinedButUsed` which just removes the
"undefined" non-external `FunctionDecl`s that Clang found.

Fixes https://github.com/llvm/llvm-project/issues/104712
---
 .../Plugins/ExpressionParser/Clang/ASTUtils.h | 19 +++++++++++++++++++
 .../Clang/ClangExpressionParser.cpp           |  3 +++
 .../Shell/Expr/TestAnonNamespaceParamFunc.cpp | 19 +++++++++++++++++++
 3 files changed, 41 insertions(+)
 create mode 100644 lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..9710f7dd4e983c 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
+#include "llvm/Support/Casting.h"
 #include <optional>
 
 namespace clang {
@@ -135,6 +136,24 @@ class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
     return m_Source->layoutRecordType(Record, Size, Alignment, FieldOffsets,
                                       BaseOffsets, VirtualBaseOffsets);
   }
+
+  /// This gets called when Sema is reconciling undefined but used functions.
+  /// For LLDB's use-case, we never provide Clang with function definitions,
+  /// instead we rely on linkage names and symbol resolution to call the
+  /// correct funcitons during JITting. So this implementation clears
+  /// any "undefined" functions Clang found while parsing.
+  ///
+  /// \param[in,out] Undefined A set of used decls for which Clang has not
+  ///                          been provided a definition with.
+  ///
+  void ReadUndefinedButUsed(
+      llvm::MapVector<clang::NamedDecl *, clang::SourceLocation> &Undefined)
+      override {
+    Undefined.remove_if([](auto const &decl_loc_pair) {
+      const clang::NamedDecl *ND = decl_loc_pair.first;
+      return llvm::isa_and_present<clang::FunctionDecl>(ND);
+    });
+  }
 };
 
 /// Wraps an ASTConsumer into an SemaConsumer. Doesn't take ownership of the
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b23e851f547ce9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1223,6 +1223,8 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager,
 
     clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
 
+    auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
+
     if (ast_context.getExternalSource()) {
       auto module_wrapper =
           new ExternalASTSourceWrapper(ast_context.getExternalSource());
@@ -1236,6 +1238,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager,
     } else {
       ast_context.setExternalSource(ast_source);
     }
+    m_compiler->getSema().addExternalSource(ast_source_wrapper);
     decl_map->InstallASTContext(*m_ast_context);
   }
 
diff --git a/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
new file mode 100644
index 00000000000000..750c81c0db6b32
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
@@ -0,0 +1,19 @@
+// TODO: Test that ...
+
+// RUN: %build %s -o %t
+// RUN: %lldb %t -o run -o "expression func(a)" -o exit | FileCheck %s
+
+// CHECK: expression func(a)
+// CHECK: (int) $0 = 15
+
+namespace {
+struct InAnon {};
+} // namespace
+
+int func(InAnon a) { return 15; }
+
+int main() {
+  InAnon a;
+  __builtin_debugtrap();
+  return func(a);
+}

>From 4bdb9e1763d133f1b0712675a4e135cb95e5e50b Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 19 Aug 2024 18:08:34 +0100
Subject: [PATCH 2/3] fixup! adjust comment

---
 lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 9710f7dd4e983c..136706f1565964 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -137,11 +137,11 @@ class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
                                       BaseOffsets, VirtualBaseOffsets);
   }
 
-  /// This gets called when Sema is reconciling undefined but used functions.
+  /// This gets called when Sema is reconciling undefined but used decls.
   /// For LLDB's use-case, we never provide Clang with function definitions,
   /// instead we rely on linkage names and symbol resolution to call the
   /// correct funcitons during JITting. So this implementation clears
-  /// any "undefined" functions Clang found while parsing.
+  /// any "undefined" FunctionDecls that Clang found while parsing.
   ///
   /// \param[in,out] Undefined A set of used decls for which Clang has not
   ///                          been provided a definition with.

>From eab8a56dd5a25f0514d7a110e6fd06b54f20a25e Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Tue, 20 Aug 2024 00:42:47 +0100
Subject: [PATCH 3/3] fixup! fix test comment

---
 lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
index 750c81c0db6b32..1882be1b31a899 100644
--- a/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
+++ b/lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
@@ -1,4 +1,7 @@
-// TODO: Test that ...
+// Tests that we can evaluate functions that Clang
+// classifies as having clang::Linkage::UniqueExternal
+// linkage. In this case, a function whose argument
+// is not legally usable outside this TU.
 
 // RUN: %build %s -o %t
 // RUN: %lldb %t -o run -o "expression func(a)" -o exit | FileCheck %s



More information about the lldb-commits mailing list