Skip to content

Commit 07ca5c9

Browse files
committed
[lldb][Expression] Encode structor variant into FunctionCallLabel
This patch is an implementation of [this discussion](https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816/7) about handling ABI-tagged structors during expression evaluation. **Motivation** LLDB encodes the mangled name of a `DW_TAG_subprogram` into `AsmLabel`s on function and method Clang AST nodes. This means that when calls to these functions get lowered into IR (when running JITted expressions), the address resolver can locate the appropriate symbol by mangled name (and it's guaranteed to find the symbol because we got the mangled name from debug-info, instead of letting Clang mangle it based on AST structure). However, we don't do this for `CXXConstructorDecl`s/`CXXDestructorDecl`s because these structor declarations in DWARF don't have a linkage name. This is because there can be multiple variants of a structor, each with a distinct mangling in the Itanium ABI. Each structor variant has its own definition `DW_TAG_subprogram`. So LLDB doesn't know which mangled name to put into the `AsmLabel`. Currently this means using an ABI-tagged constructor in LLDB expressions won't work (see [this RFC](https://discourse.llvm.org/t/rfc-lldb-handling-abi-tagged-constructors-destructors-in-expression-evaluator/82816) for concrete examples). **Proposed Solution** The `FunctionCallLabel` encoding that we put into `AsmLabel`s already supports stuffing more info about a DIE into it. So this patch extends the `FunctionCallLabel` to contain an optional discriminator (a sequence of bytes) which the `SymbolFileDWARF` plugin interprets as the constructor/destructor variant of that DIE. There's a few subtleties here: 1. At the point at which LLDB first constructs the label, it has no way of knowing (just by looking the debug-info declaration), which structor variant the expression evaluator is supposed to call. That's something that gets decided when compiling the expression. So we let the Clang mangler inject the correct structor variant into the `AsmLabel` during JITing. I adjusted the `AsmLabelAttr` mangling for this. An option would've been to create a new Clang attribute which behaved like an `AsmLabel` but with these special semantics for LLDB. My main concern there is that we'd have to adjust all the `AsmLabelAttr` checks around Clang to also now account for this new attribute. 2. The compiler is free to omit the `C1` variant of a constructor if the `C2` variant is sufficient. In that case it may alias `C1` to `C2`, leaving us with only the `C2` `DW_TAG_subprogram` in the object file. Linux is one of the platforms where this occurs. For those cases there's heuristic in `SymbolFileDWARF` where we pick `C2` if we asked for `C1` but it doesn't exist. This may not always be correct (if for some reason the compiler decided to drop `C1` for other reasons).
1 parent 180c030 commit 07ca5c9

File tree

10 files changed

+291
-55
lines changed

10 files changed

+291
-55
lines changed

lldb/include/lldb/Expression/Expression.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,15 @@ class Expression {
103103
///
104104
/// The format being:
105105
///
106-
/// <prefix>:<module uid>:<symbol uid>:<name>
106+
/// <prefix>:<discriminator>:<module uid>:<symbol uid>:<name>
107107
///
108108
/// The label string needs to stay valid for the entire lifetime
109109
/// of this object.
110110
struct FunctionCallLabel {
111+
/// Arbitrary string which language plugins can interpret for their
112+
/// own needs.
113+
llvm::StringRef discriminator;
114+
111115
/// Unique identifier of the lldb_private::Module
112116
/// which contains the symbol identified by \c symbol_id.
113117
lldb::user_id_t module_id;
@@ -133,7 +137,7 @@ struct FunctionCallLabel {
133137
///
134138
/// The representation roundtrips through \c fromString:
135139
/// \code{.cpp}
136-
/// llvm::StringRef encoded = "$__lldb_func:0x0:0x0:_Z3foov";
140+
/// llvm::StringRef encoded = "$__lldb_func:blah:0x0:0x0:_Z3foov";
137141
/// FunctionCallLabel label = *fromString(label);
138142
///
139143
/// assert (label.toString() == encoded);

lldb/source/Expression/Expression.cpp

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,21 @@ Expression::Expression(ExecutionContextScope &exe_scope)
3434

3535
llvm::Expected<FunctionCallLabel>
3636
lldb_private::FunctionCallLabel::fromString(llvm::StringRef label) {
37-
llvm::SmallVector<llvm::StringRef, 4> components;
38-
label.split(components, ":", /*MaxSplit=*/3);
37+
llvm::SmallVector<llvm::StringRef, 5> components;
38+
label.split(components, ":", /*MaxSplit=*/4);
3939

40-
if (components.size() != 4)
40+
if (components.size() != 5)
4141
return llvm::createStringError("malformed function call label.");
4242

4343
if (components[0] != FunctionCallLabelPrefix)
4444
return llvm::createStringError(llvm::formatv(
4545
"expected function call label prefix '{0}' but found '{1}' instead.",
4646
FunctionCallLabelPrefix, components[0]));
4747

48-
llvm::StringRef module_label = components[1];
49-
llvm::StringRef die_label = components[2];
48+
llvm::StringRef discriminator = components[1];
49+
llvm::StringRef module_label = components[2];
50+
llvm::StringRef die_label = components[3];
51+
llvm::StringRef lookup_name = components[4];
5052

5153
lldb::user_id_t module_id = 0;
5254
if (!llvm::to_integer(module_label, module_id))
@@ -58,20 +60,23 @@ lldb_private::FunctionCallLabel::fromString(llvm::StringRef label) {
5860
return llvm::createStringError(
5961
llvm::formatv("failed to parse symbol ID from '{0}'.", die_label));
6062

61-
return FunctionCallLabel{/*.module_id=*/module_id,
63+
return FunctionCallLabel{/*.discriminator=*/discriminator,
64+
/*.module_id=*/module_id,
6265
/*.symbol_id=*/die_id,
63-
/*.lookup_name=*/components[3]};
66+
/*.lookup_name=*/lookup_name};
6467
}
6568

6669
std::string lldb_private::FunctionCallLabel::toString() const {
67-
return llvm::formatv("{0}:{1:x}:{2:x}:{3}", FunctionCallLabelPrefix,
68-
module_id, symbol_id, lookup_name)
70+
return llvm::formatv("{0}:{1}:{2:x}:{3:x}:{4}", FunctionCallLabelPrefix,
71+
discriminator, module_id, symbol_id, lookup_name)
6972
.str();
7073
}
7174

7275
void llvm::format_provider<FunctionCallLabel>::format(
7376
const FunctionCallLabel &label, raw_ostream &OS, StringRef Style) {
74-
OS << llvm::formatv("FunctionCallLabel{ module_id: {0:x}, symbol_id: {1:x}, "
75-
"lookup_name: {2} }",
76-
label.module_id, label.symbol_id, label.lookup_name);
77+
OS << llvm::formatv("FunctionCallLabel{ discriminator: {0}, module_id: "
78+
"{1:x}, symbol_id: {2:x}, "
79+
"lookup_name: {3} }",
80+
label.discriminator, label.module_id, label.symbol_id,
81+
label.lookup_name);
7782
}

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,11 +251,52 @@ static unsigned GetCXXMethodCVQuals(const DWARFDIE &subprogram,
251251
return cv_quals;
252252
}
253253

254+
static const char *GetMangledOrStructorName(const DWARFDIE &die) {
255+
const char *name = die.GetMangledName(/*substitute_name_allowed*/ false);
256+
if (name)
257+
return name;
258+
259+
name = die.GetName();
260+
if (!name)
261+
return nullptr;
262+
263+
DWARFDIE parent = die.GetParent();
264+
if (!parent.IsStructUnionOrClass())
265+
return nullptr;
266+
267+
const char *parent_name = parent.GetName();
268+
if (!parent_name)
269+
return nullptr;
270+
271+
// Constructor.
272+
if (::strcmp(parent_name, name) == 0)
273+
return name;
274+
275+
// Destructor.
276+
if (name[0] == '~' && ::strcmp(parent_name, name + 1) == 0)
277+
return name;
278+
279+
return nullptr;
280+
}
281+
254282
static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) {
255-
char const *name = die.GetMangledName(/*substitute_name_allowed*/ false);
283+
char const *name = GetMangledOrStructorName(die);
256284
if (!name)
257285
return {};
258286

287+
auto *cu = die.GetCU();
288+
if (!cu)
289+
return {};
290+
291+
// FIXME: When resolving function call labels, we check that
292+
// that the definition's DW_AT_specification points to the
293+
// declaration that we encoded into the label here. But if the
294+
// declaration came from a type-unit (and the definition from
295+
// .debug_info), that check won't work. So for now, don't use
296+
// function call labels for declaration DIEs from type-units.
297+
if (cu->IsTypeUnit())
298+
return {};
299+
259300
SymbolFileDWARF *dwarf = die.GetDWARF();
260301
if (!dwarf)
261302
return {};
@@ -286,7 +327,9 @@ static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) {
286327
if (die_id == LLDB_INVALID_UID)
287328
return {};
288329

289-
return FunctionCallLabel{/*module_id=*/module_id,
330+
// Note, discriminator is added by Clang during mangling.
331+
return FunctionCallLabel{/*discriminator=*/{},
332+
/*module_id=*/module_id,
290333
/*symbol_id=*/die_id,
291334
/*.lookup_name=*/name}
292335
.toString();

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Lines changed: 121 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "SymbolFileDWARF.h"
10+
#include "clang/Basic/ABI.h"
1011
#include "llvm/ADT/STLExtras.h"
12+
#include "llvm/ADT/StringExtras.h"
1113
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
1214
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
1315
#include "llvm/Support/Casting.h"
@@ -78,6 +80,7 @@
7880

7981
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
8082
#include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
83+
#include "llvm/Demangle/Demangle.h"
8184
#include "llvm/Support/FileSystem.h"
8285
#include "llvm/Support/FormatVariadic.h"
8386

@@ -2483,28 +2486,132 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die,
24832486
return false;
24842487
}
24852488

2486-
DWARFDIE
2487-
SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label) {
2489+
static int ClangToItaniumCtorKind(clang::CXXCtorType kind) {
2490+
switch (kind) {
2491+
case clang::CXXCtorType::Ctor_Complete:
2492+
return 1;
2493+
case clang::CXXCtorType::Ctor_Base:
2494+
return 2;
2495+
case clang::CXXCtorType::Ctor_CopyingClosure:
2496+
case clang::CXXCtorType::Ctor_DefaultClosure:
2497+
case clang::CXXCtorType::Ctor_Comdat:
2498+
llvm_unreachable("Unexpected constructor kind.");
2499+
}
2500+
}
2501+
2502+
static int ClangToItaniumDtorKind(clang::CXXDtorType kind) {
2503+
switch (kind) {
2504+
case clang::CXXDtorType::Dtor_Deleting:
2505+
return 0;
2506+
case clang::CXXDtorType::Dtor_Complete:
2507+
return 1;
2508+
case clang::CXXDtorType::Dtor_Base:
2509+
return 2;
2510+
case clang::CXXDtorType::Dtor_Comdat:
2511+
llvm_unreachable("Unexpected destructor kind.");
2512+
}
2513+
}
2514+
2515+
static std::optional<int>
2516+
GetItaniumCtorDtorVariant(llvm::StringRef discriminator) {
2517+
const bool is_ctor = discriminator.consume_front("C");
2518+
if (!is_ctor && !discriminator.consume_front("D"))
2519+
return std::nullopt;
2520+
2521+
uint64_t structor_kind;
2522+
if (!llvm::to_integer(discriminator, structor_kind))
2523+
return std::nullopt;
2524+
2525+
if (is_ctor) {
2526+
if (structor_kind > clang::CXXCtorType::Ctor_DefaultClosure)
2527+
return std::nullopt;
2528+
2529+
return ClangToItaniumCtorKind(
2530+
static_cast<clang::CXXCtorType>(structor_kind));
2531+
}
2532+
2533+
if (structor_kind > clang::CXXDtorType::Dtor_Comdat)
2534+
return std::nullopt;
2535+
2536+
return ClangToItaniumDtorKind(static_cast<clang::CXXDtorType>(structor_kind));
2537+
}
2538+
2539+
DWARFDIE SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label,
2540+
const DWARFDIE &declaration) {
24882541
DWARFDIE definition;
2542+
llvm::DenseMap<int, DWARFDIE> structor_variant_to_die;
2543+
2544+
// eFunctionNameTypeFull for mangled name lookup.
2545+
// eFunctionNameTypeMethod is required for structor lookups (since we look
2546+
// those up by DW_AT_name).
24892547
Module::LookupInfo info(ConstString(label.lookup_name),
2490-
lldb::eFunctionNameTypeFull,
2548+
lldb::eFunctionNameTypeFull |
2549+
lldb::eFunctionNameTypeMethod,
24912550
lldb::eLanguageTypeUnknown);
24922551

24932552
m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) {
24942553
if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0))
24952554
return IterationAction::Continue;
24962555

2497-
// We don't check whether the specification DIE for this function
2498-
// corresponds to the declaration DIE because the declaration might be in
2499-
// a type-unit but the definition in the compile-unit (and it's
2500-
// specifcation would point to the declaration in the compile-unit). We
2501-
// rely on the mangled name within the module to be enough to find us the
2502-
// unique definition.
2503-
definition = entry;
2504-
return IterationAction::Stop;
2556+
auto spec = entry.GetAttributeValueAsReferenceDIE(DW_AT_specification);
2557+
if (!spec)
2558+
return IterationAction::Continue;
2559+
2560+
if (spec != declaration)
2561+
return IterationAction::Continue;
2562+
2563+
// We're not picking a specific structor variant DIE, so we're done here.
2564+
if (label.discriminator.empty()) {
2565+
definition = entry;
2566+
return IterationAction::Stop;
2567+
}
2568+
2569+
const char *mangled =
2570+
entry.GetMangledName(/*substitute_name_allowed=*/false);
2571+
if (!mangled)
2572+
return IterationAction::Continue;
2573+
2574+
llvm::ItaniumPartialDemangler D;
2575+
if (D.partialDemangle(mangled))
2576+
return IterationAction::Continue;
2577+
2578+
auto structor_variant = D.getCtorOrDtorVariant();
2579+
if (!structor_variant)
2580+
return IterationAction::Continue;
2581+
2582+
auto [_, inserted] = structor_variant_to_die.try_emplace(*structor_variant,
2583+
std::move(entry));
2584+
assert(inserted);
2585+
2586+
// The compiler may choose to alias the constructor variants
2587+
// (notably this happens on Linux), so we might not have a definition
2588+
// DIE for some structor variants. Hence we iterate over all variants
2589+
// and pick the most appropriate one out of those.
2590+
return IterationAction::Continue;
25052591
});
25062592

2507-
return definition;
2593+
if (definition.IsValid())
2594+
return definition;
2595+
2596+
auto label_variant = GetItaniumCtorDtorVariant(label.discriminator);
2597+
if (!label_variant)
2598+
return {};
2599+
2600+
auto it = structor_variant_to_die.find(*label_variant);
2601+
2602+
// Found the exact variant.
2603+
if (it != structor_variant_to_die.end())
2604+
return it->getSecond();
2605+
2606+
// We need a C1 constructor. If debug-info only contains a DIE for C2,
2607+
// assume C1 was aliased to C2.
2608+
if (!label.lookup_name.starts_with("~") && label_variant == 1) {
2609+
if (auto it = structor_variant_to_die.find(2);
2610+
it != structor_variant_to_die.end())
2611+
return it->getSecond();
2612+
}
2613+
2614+
return {};
25082615
}
25092616

25102617
llvm::Expected<SymbolContext>
@@ -2519,11 +2626,9 @@ SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) {
25192626
// Label was created using a declaration DIE. Need to fetch the definition
25202627
// to resolve the function call.
25212628
if (die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) {
2522-
auto definition = FindFunctionDefinition(label);
2523-
if (!definition)
2629+
die = FindFunctionDefinition(label, die);
2630+
if (!die.IsValid())
25242631
return llvm::createStringError("failed to find definition DIE");
2525-
2526-
die = std::move(definition);
25272632
}
25282633

25292634
SymbolContextList sc_list;

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,8 @@ class SymbolFileDWARF : public SymbolFileCommon {
378378
/// SymbolFile.
379379
///
380380
/// \returns A valid definition DIE on success.
381-
DWARFDIE FindFunctionDefinition(const FunctionCallLabel &label);
381+
DWARFDIE FindFunctionDefinition(const FunctionCallLabel &label,
382+
const DWARFDIE &declaration);
382383

383384
protected:
384385
SymbolFileDWARF(const SymbolFileDWARF &) = delete;
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CXX_SOURCES := main.cpp
2+
3+
include Makefile.rules
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
"""
2+
Test that we can call structors/destructors
3+
annotated (and thus mangled) with ABI tags.
4+
"""
5+
6+
import lldb
7+
from lldbsuite.test.decorators import *
8+
from lldbsuite.test.lldbtest import *
9+
from lldbsuite.test import lldbutil
10+
11+
12+
class AbiTagStructorsTestCase(TestBase):
13+
def test_abi_tag_lookup(self):
14+
self.build()
15+
lldbutil.run_to_source_breakpoint(
16+
self, "Break here", lldb.SBFileSpec("main.cpp", False)
17+
)
18+
19+
self.expect_expr("Tagged()", result_type="Tagged")
20+
self.expect_expr("t1 = t2", result_type="Tagged")
21+
22+
self.expect("expr Tagged t3(t1)", error=False)
23+
self.expect("expr t1.~Tagged()", error=False)
24+
25+
# Calls to deleting and base object destructor variants (D0 and D2 in Itanium ABI)
26+
self.expect_expr(
27+
"struct D : public HasVirtualDtor {}; D d; d.func()",
28+
result_type="int",
29+
result_value="10",
30+
)

0 commit comments

Comments
 (0)