Skip to content

Commit

Permalink
[lldb][ClangExpressionParser] Implement ExternalSemaSource::ReadUndef…
Browse files Browse the repository at this point in the history
…inedButUsed (#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 #104712
  • Loading branch information
Michael137 authored Aug 20, 2024
1 parent ddaa828 commit 8056d92
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 0 deletions.
19 changes: 19 additions & 0 deletions lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaConsumer.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include "llvm/Support/Casting.h"
#include <optional>

namespace clang {
Expand Down Expand Up @@ -138,6 +139,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 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" 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.
///
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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);
}

Expand Down
22 changes: 22 additions & 0 deletions lldb/test/Shell/Expr/TestAnonNamespaceParamFunc.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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

// 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);
}

0 comments on commit 8056d92

Please sign in to comment.