Skip to content

Commit 57a7907

Browse files
authored
[lldb][Expression] Add structor variant to LLDB's function call labels (#149827)
Depends on * #148877 * #155483 * #155485 * #154137 * #154142 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 is 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 ABI-tagged structors 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. So when searching for the definition DIE, LLDB will include the structor variant in its heuristic for determining a match. 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 at 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 in #155485. 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 I added a heuristic in `SymbolFileDWARF` where we pick `C2` if we asked for `C1` but it doesn't exist. This may not always be correct (e.g., if the compiler decided to drop `C1` for other reasons). 3. In #154142 Clang will emit `C4`/`D4` variants of ctors/dtors on declarations. When resolving the `FunctionCallLabel` we will now substitute the actual variant that Clang told us we need to call into the mangled name. We do this using LLDB's `ManglingSubstitutor`. That way we find the definition DIE exactly the same way we do for regular function calls. 4. In cases where declarations and definitions live in separate modules, the DIE ID encoded in the function call label may not be enough to find the definition DIE in the encoded module ID. For those cases we fall back to how LLDB used to work: look up in all images of the target. To make sure we don't use the unified mangled name for the fallback lookup, we change the lookup name to whatever mangled name the FunctionCallLabel resolved to. rdar://104968288
1 parent 06d202b commit 57a7907

File tree

18 files changed

+479
-79
lines changed

18 files changed

+479
-79
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/include/lldb/Symbol/SymbolFile.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,12 @@ class SymbolFile : public PluginInterface {
332332
/// Resolves the function corresponding to the specified LLDB function
333333
/// call \c label.
334334
///
335-
/// \param[in] label The FunctionCallLabel to be resolved.
335+
/// \param[in,out] label The FunctionCallLabel to be resolved.
336336
///
337337
/// \returns An llvm::Error if the specified \c label couldn't be resolved.
338338
/// Returns the resolved function (as a SymbolContext) otherwise.
339339
virtual llvm::Expected<SymbolContext>
340-
ResolveFunctionCallLabel(const FunctionCallLabel &label) {
340+
ResolveFunctionCallLabel(FunctionCallLabel &label) {
341341
return llvm::createStringError("Not implemented");
342342
}
343343

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/Expression/IRExecutionUnit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ class LoadAddressResolver {
723723
/// Returns address of the function referred to by the special function call
724724
/// label \c label.
725725
static llvm::Expected<lldb::addr_t>
726-
ResolveFunctionCallLabel(const FunctionCallLabel &label,
726+
ResolveFunctionCallLabel(FunctionCallLabel &label,
727727
const lldb_private::SymbolContext &sc,
728728
bool &symbol_was_missing_weak) {
729729
symbol_was_missing_weak = false;

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ static unsigned GetCXXMethodCVQuals(const DWARFDIE &subprogram,
252252
}
253253

254254
static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) {
255-
char const *name = die.GetMangledName(/*substitute_name_allowed*/ false);
255+
const char *name = die.GetMangledName(/*substitute_name_allowed*/ false);
256256
if (!name)
257257
return {};
258258

@@ -286,7 +286,9 @@ static std::string MakeLLDBFuncAsmLabel(const DWARFDIE &die) {
286286
if (die_id == LLDB_INVALID_UID)
287287
return {};
288288

289-
return FunctionCallLabel{/*module_id=*/module_id,
289+
// Note, discriminator is added by Clang during mangling.
290+
return FunctionCallLabel{/*discriminator=*/{},
291+
/*module_id=*/module_id,
290292
/*symbol_id=*/die_id,
291293
/*.lookup_name=*/name}
292294
.toString();

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

Lines changed: 145 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,14 @@
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/ADT/StringRef.h"
1214
#include "llvm/DebugInfo/DWARF/DWARFAddressRange.h"
1315
#include "llvm/DebugInfo/DWARF/DWARFDebugLoc.h"
1416
#include "llvm/Support/Casting.h"
17+
#include "llvm/Support/Error.h"
1518
#include "llvm/Support/FileUtilities.h"
1619
#include "llvm/Support/FormatAdapters.h"
1720
#include "llvm/Support/Threading.h"
@@ -23,6 +26,7 @@
2326
#include "lldb/Core/Progress.h"
2427
#include "lldb/Core/Section.h"
2528
#include "lldb/Core/Value.h"
29+
#include "lldb/Expression/Expression.h"
2630
#include "lldb/Utility/ArchSpec.h"
2731
#include "lldb/Utility/LLDBLog.h"
2832
#include "lldb/Utility/RegularExpression.h"
@@ -79,6 +83,7 @@
7983

8084
#include "llvm/DebugInfo/DWARF/DWARFContext.h"
8185
#include "llvm/DebugInfo/DWARF/DWARFDebugAbbrev.h"
86+
#include "llvm/Demangle/Demangle.h"
8287
#include "llvm/Support/FileSystem.h"
8388
#include "llvm/Support/FormatVariadic.h"
8489

@@ -2484,34 +2489,148 @@ bool SymbolFileDWARF::ResolveFunction(const DWARFDIE &orig_die,
24842489
return false;
24852490
}
24862491

2487-
DWARFDIE
2488-
SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label) {
2489-
DWARFDIE definition;
2490-
Module::LookupInfo info(ConstString(label.lookup_name),
2491-
lldb::eFunctionNameTypeFull,
2492-
lldb::eLanguageTypeUnknown);
2493-
2494-
m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) {
2495-
if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0))
2496-
return IterationAction::Continue;
2492+
static llvm::StringRef ClangToItaniumCtorKind(clang::CXXCtorType kind) {
2493+
switch (kind) {
2494+
case clang::CXXCtorType::Ctor_Complete:
2495+
return "C1";
2496+
case clang::CXXCtorType::Ctor_Base:
2497+
return "C2";
2498+
case clang::CXXCtorType::Ctor_Unified:
2499+
return "C4";
2500+
case clang::CXXCtorType::Ctor_CopyingClosure:
2501+
case clang::CXXCtorType::Ctor_DefaultClosure:
2502+
case clang::CXXCtorType::Ctor_Comdat:
2503+
llvm_unreachable("Unexpected constructor kind.");
2504+
}
2505+
}
24972506

2498-
// We don't check whether the specification DIE for this function
2499-
// corresponds to the declaration DIE because the declaration might be in
2500-
// a type-unit but the definition in the compile-unit (and it's
2501-
// specifcation would point to the declaration in the compile-unit). We
2502-
// rely on the mangled name within the module to be enough to find us the
2503-
// unique definition.
2504-
definition = entry;
2505-
return IterationAction::Stop;
2506-
});
2507+
static llvm::StringRef ClangToItaniumDtorKind(clang::CXXDtorType kind) {
2508+
switch (kind) {
2509+
case clang::CXXDtorType::Dtor_Deleting:
2510+
return "D0";
2511+
case clang::CXXDtorType::Dtor_Complete:
2512+
return "D1";
2513+
case clang::CXXDtorType::Dtor_Base:
2514+
return "D2";
2515+
case clang::CXXDtorType::Dtor_Unified:
2516+
return "D4";
2517+
case clang::CXXDtorType::Dtor_Comdat:
2518+
llvm_unreachable("Unexpected destructor kind.");
2519+
}
2520+
}
2521+
2522+
static llvm::StringRef
2523+
GetItaniumCtorDtorVariant(llvm::StringRef discriminator) {
2524+
const bool is_ctor = discriminator.consume_front("C");
2525+
if (!is_ctor && !discriminator.consume_front("D"))
2526+
return {};
2527+
2528+
uint64_t structor_kind;
2529+
if (!llvm::to_integer(discriminator, structor_kind))
2530+
return {};
2531+
2532+
if (is_ctor) {
2533+
if (structor_kind > clang::CXXCtorType::Ctor_Unified)
2534+
return {};
2535+
2536+
return ClangToItaniumCtorKind(
2537+
static_cast<clang::CXXCtorType>(structor_kind));
2538+
}
2539+
2540+
if (structor_kind > clang::CXXDtorType::Dtor_Unified)
2541+
return {};
2542+
2543+
return ClangToItaniumDtorKind(static_cast<clang::CXXDtorType>(structor_kind));
2544+
}
2545+
2546+
llvm::Expected<DWARFDIE>
2547+
SymbolFileDWARF::FindFunctionDefinition(const FunctionCallLabel &label,
2548+
const DWARFDIE &declaration) {
2549+
auto do_lookup = [this](llvm::StringRef lookup_name) -> DWARFDIE {
2550+
DWARFDIE found;
2551+
Module::LookupInfo info(ConstString(lookup_name),
2552+
lldb::eFunctionNameTypeFull,
2553+
lldb::eLanguageTypeUnknown);
2554+
2555+
m_index->GetFunctions(info, *this, {}, [&](DWARFDIE entry) {
2556+
if (entry.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0))
2557+
return IterationAction::Continue;
2558+
2559+
found = entry;
2560+
return IterationAction::Stop;
2561+
});
2562+
2563+
return found;
2564+
};
2565+
2566+
DWARFDIE definition = do_lookup(label.lookup_name);
2567+
if (definition.IsValid())
2568+
return definition;
2569+
2570+
// This is not a structor lookup. Nothing else to be done here.
2571+
if (label.discriminator.empty())
2572+
return llvm::createStringError(
2573+
"no definition DIE found in this SymbolFile");
2574+
2575+
// We're doing a structor lookup. Maybe we didn't find the structor variant
2576+
// because the complete object structor was aliased to the base object
2577+
// structor. Try finding the alias instead.
2578+
//
2579+
// TODO: there are other reasons for why a subprogram definition might be
2580+
// missing. Ideally DWARF would tell us more details about which structor
2581+
// variant a DIE corresponds to and whether it's an alias.
2582+
auto subst_or_err =
2583+
CPlusPlusLanguage::SubstituteStructorAliases_ItaniumMangle(
2584+
label.lookup_name);
2585+
if (!subst_or_err)
2586+
return subst_or_err.takeError();
2587+
2588+
definition = do_lookup(*subst_or_err);
2589+
2590+
if (!definition.IsValid())
2591+
return llvm::createStringError(
2592+
"failed to find definition DIE for structor alias in fallback lookup");
25072593

25082594
return definition;
25092595
}
25102596

25112597
llvm::Expected<SymbolContext>
2512-
SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) {
2598+
SymbolFileDWARF::ResolveFunctionCallLabel(FunctionCallLabel &label) {
25132599
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
25142600

2601+
if (!label.discriminator.empty()) {
2602+
llvm::StringRef from = label.discriminator[0] == 'C' ? "C4" : "D4";
2603+
2604+
llvm::StringRef variant = GetItaniumCtorDtorVariant(label.discriminator);
2605+
if (variant.empty())
2606+
return llvm::createStringError(
2607+
"failed to get Itanium variant for discriminator");
2608+
2609+
if (from == variant)
2610+
return llvm::createStringError(
2611+
"tried substituting unified structor variant into label");
2612+
2613+
// If we failed to substitute unified mangled name, don't try to do a lookup
2614+
// using the unified name because there may be multiple definitions for it
2615+
// in the index, and we wouldn't know which one to choose.
2616+
auto subst_or_err = CPlusPlusLanguage::SubstituteStructor_ItaniumMangle(
2617+
label.lookup_name, from, variant);
2618+
if (!subst_or_err)
2619+
return llvm::joinErrors(
2620+
llvm::createStringError(llvm::formatv(
2621+
"failed to substitute {0} for {1} in mangled name {2}:", from,
2622+
variant, label.lookup_name)),
2623+
subst_or_err.takeError());
2624+
2625+
if (!*subst_or_err)
2626+
return llvm::createStringError(
2627+
llvm::formatv("got invalid substituted mangled named (substituted "
2628+
"{0} for {1} in mangled name {2})",
2629+
from, variant, label.lookup_name));
2630+
2631+
label.lookup_name = subst_or_err->GetStringRef();
2632+
}
2633+
25152634
DWARFDIE die = GetDIE(label.symbol_id);
25162635
if (!die.IsValid())
25172636
return llvm::createStringError(
@@ -2520,11 +2639,13 @@ SymbolFileDWARF::ResolveFunctionCallLabel(const FunctionCallLabel &label) {
25202639
// Label was created using a declaration DIE. Need to fetch the definition
25212640
// to resolve the function call.
25222641
if (die.GetAttributeValueAsUnsigned(llvm::dwarf::DW_AT_declaration, 0)) {
2523-
auto definition = FindFunctionDefinition(label);
2524-
if (!definition)
2525-
return llvm::createStringError("failed to find definition DIE");
2642+
auto die_or_err = FindFunctionDefinition(label, die);
2643+
if (!die_or_err)
2644+
return llvm::joinErrors(
2645+
llvm::createStringError("failed to find definition DIE:"),
2646+
die_or_err.takeError());
25262647

2527-
die = std::move(definition);
2648+
die = std::move(*die_or_err);
25282649
}
25292650

25302651
SymbolContextList sc_list;

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,9 @@ class SymbolFileDWARF : public SymbolFileCommon {
378378
/// SymbolFile.
379379
///
380380
/// \returns A valid definition DIE on success.
381-
DWARFDIE FindFunctionDefinition(const FunctionCallLabel &label);
381+
llvm::Expected<DWARFDIE>
382+
FindFunctionDefinition(const FunctionCallLabel &label,
383+
const DWARFDIE &declaration);
382384

383385
protected:
384386
SymbolFileDWARF(const SymbolFileDWARF &) = delete;
@@ -445,7 +447,7 @@ class SymbolFileDWARF : public SymbolFileCommon {
445447
DIEArray &&variable_dies);
446448

447449
llvm::Expected<SymbolContext>
448-
ResolveFunctionCallLabel(const FunctionCallLabel &label) override;
450+
ResolveFunctionCallLabel(FunctionCallLabel &label) override;
449451

450452
// Given a die_offset, figure out the symbol context representing that die.
451453
bool ResolveFunction(const DWARFDIE &die, bool include_inlines,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1603,8 +1603,8 @@ void SymbolFileDWARFDebugMap::GetCompileOptions(
16031603
});
16041604
}
16051605

1606-
llvm::Expected<SymbolContext> SymbolFileDWARFDebugMap::ResolveFunctionCallLabel(
1607-
const FunctionCallLabel &label) {
1606+
llvm::Expected<SymbolContext>
1607+
SymbolFileDWARFDebugMap::ResolveFunctionCallLabel(FunctionCallLabel &label) {
16081608
const uint64_t oso_idx = GetOSOIndexFromUserID(label.symbol_id);
16091609
SymbolFileDWARF *oso_dwarf = GetSymbolFileByOSOIndex(oso_idx);
16101610
if (!oso_dwarf)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class SymbolFileDWARFDebugMap : public SymbolFileCommon {
145145
GetCompileOptions(std::unordered_map<lldb::CompUnitSP, Args> &args) override;
146146

147147
llvm::Expected<SymbolContext>
148-
ResolveFunctionCallLabel(const FunctionCallLabel &label) override;
148+
ResolveFunctionCallLabel(FunctionCallLabel &label) override;
149149

150150
protected:
151151
enum { kHaveInitializedOSOs = (1 << 0), kNumFlags };
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

0 commit comments

Comments
 (0)