Skip to content

Commit f5858a8

Browse files
committed
[lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndefinedButUsed (llvm#104817)
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, this is computed [here in computeTypeLinkageInfo](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/AST/Type.cpp#L4647-L4653)). 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. We also had to add an `ExternalSemaSource` to the `clang::Sema` instance LLDB creates. We previously didn't have any source on `Sema`. Because we add the `ExternalASTSourceWrapper` here, that means we'd also technically be adding the `ClangExpressionDeclMap` as an `ExternalASTSource` to `Sema`, which is fine because `Sema` will only be calling into the `ExternalSemaSource` APIs (though nothing currently strictly enforces this, which is a bit worrying). Note, the decision for whether to put a function into `UndefinedButUsed` is done in [Sema::MarkFunctionReferenced](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/Sema/SemaExpr.cpp#L18083-L18087). The `UniqueExternal` linkage computation is done in [getLVForNamespaceScopeDecl](https://github.com/llvm/llvm-project/blob/ea8bb4d633683f5cbfd82491620be3056f347a02/clang/lib/AST/Decl.cpp#L821-L833). Fixes llvm#104712 (cherry picked from commit 8056d92)
1 parent 1538d04 commit f5858a8

File tree

3 files changed

+44
-0
lines changed

3 files changed

+44
-0
lines changed

lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "clang/Sema/Sema.h"
1717
#include "clang/Sema/SemaConsumer.h"
1818
#include "llvm/ADT/IntrusiveRefCntPtr.h"
19+
#include "llvm/Support/Casting.h"
1920
#include <optional>
2021

2122
namespace clang {
@@ -139,6 +140,24 @@ class ExternalASTSourceWrapper : public ImporterBackedASTSource {
139140
return m_Source->layoutRecordType(Record, Size, Alignment, FieldOffsets,
140141
BaseOffsets, VirtualBaseOffsets);
141142
}
143+
144+
/// This gets called when Sema is reconciling undefined but used decls.
145+
/// For LLDB's use-case, we never provide Clang with function definitions,
146+
/// instead we rely on linkage names and symbol resolution to call the
147+
/// correct funcitons during JITting. So this implementation clears
148+
/// any "undefined" FunctionDecls that Clang found while parsing.
149+
///
150+
/// \param[in,out] Undefined A set of used decls for which Clang has not
151+
/// been provided a definition with.
152+
///
153+
void ReadUndefinedButUsed(
154+
llvm::MapVector<clang::NamedDecl *, clang::SourceLocation> &Undefined)
155+
override {
156+
Undefined.remove_if([](auto const &decl_loc_pair) {
157+
const clang::NamedDecl *ND = decl_loc_pair.first;
158+
return llvm::isa_and_present<clang::FunctionDecl>(ND);
159+
});
160+
}
142161
};
143162

144163
/// Wraps an ASTConsumer into an SemaConsumer. Doesn't take ownership of the

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,8 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager,
11211121

11221122
clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
11231123

1124+
auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
1125+
11241126
if (ast_context.getExternalSource()) {
11251127
auto *module_wrapper =
11261128
new ExternalASTSourceWrapper(ast_context.getExternalSource());
@@ -1134,6 +1136,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager &diagnostic_manager,
11341136
} else {
11351137
ast_context.setExternalSource(ast_source);
11361138
}
1139+
m_compiler->getSema().addExternalSource(ast_source_wrapper);
11371140
decl_map->InstallASTContext(*m_ast_context);
11381141
}
11391142

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Tests that we can evaluate functions that Clang
2+
// classifies as having clang::Linkage::UniqueExternal
3+
// linkage. In this case, a function whose argument
4+
// is not legally usable outside this TU.
5+
6+
// RUN: %build %s -o %t
7+
// RUN: %lldb %t -o run -o "expression func(a)" -o exit | FileCheck %s
8+
9+
// CHECK: expression func(a)
10+
// CHECK: (int) $0 = 15
11+
12+
namespace {
13+
struct InAnon {};
14+
} // namespace
15+
16+
int func(InAnon a) { return 15; }
17+
18+
int main() {
19+
InAnon a;
20+
__builtin_debugtrap();
21+
return func(a);
22+
}

0 commit comments

Comments
 (0)