Skip to content

Commit 542d52b

Browse files
authored
[lldb][Expression] Allow specifying a preferred ModuleList for lookup during expression evaluation (llvm#129733)
The `TestMemoryHistory.py`/`TestReportData.py` are currently failing on the x86 macOS CI (started after we upgraded the Xcode SDK on that machien). The LLDB ASAN utility expression is failing to run with following error: ``` (lldb) image lookup -n __asan_get_alloc_stack 1 match found in /usr/lib/system/libsystem_sanitizers.dylib: Address: libsystem_sanitizers.dylib[0x00007ffd11e673f7] (libsystem_sanitizers.dylib.__TEXT.__text + 11287) Summary: libsystem_sanitizers.dylib`__asan_get_alloc_stack 1 match found in /Users/michaelbuch/Git/lldb-build-main-no-modules/lib/clang/21/lib/darwin/libclang_rt.asan_osx_dynamic.dylib: Address: libclang_rt.asan_osx_dynamic.dylib[0x0000000000009ec0] (libclang_rt.asan_osx_dynamic.dylib.__TEXT.__text + 34352) Summary: libclang_rt.asan_osx_dynamic.dylib`::__asan_get_alloc_stack(__sanitizer::uptr, __sanitizer::uptr *, __sanitizer::uptr, __sanitizer::u32 *) at asan_debugging.cpp:132 (lldb) memory history 'pointer' Assertion failed: ((uintptr_t)addr == report.access.address), function __asan_get_alloc_stack, file debugger_abi.cpp, line 62. warning: cannot evaluate AddressSanitizer expression: error: Expression execution was interrupted: signal SIGABRT. The process has been returned to the state before expression evaluation. ``` The reason for this is that the system sanitizer dylib and the locally built libclang_rt contain the same symbol `__asan_get_alloc_stack`, and depending on the order in which they're loaded, we may pick the one from the wrong dylib (this probably changed during the buildbot upgrade and is why it only now started failing). Based on discussion with @wrotki we always want to pick the one that's in the libclang_rt dylib if it was loaded, and libsystem_sanitizers otherwise. This patch addresses this by adding a "preferred lookup context list" to the expression evaluator. Currently this is only exposed in the `EvaluateExpressionOptions`. We make it a `SymbolContextList` in case we want the lookup contexts to be contexts other than modules (e.g., source files, etc.). In `IRExecutionUnit` we make it a `ModuleList` because it makes the symbol lookup implementation simpler and we only do module lookups here anyway. If we ever need it to be a `SymbolContext`, that transformation shouldn't be too difficult.
1 parent 9a0e652 commit 542d52b

File tree

9 files changed

+134
-5
lines changed

9 files changed

+134
-5
lines changed

Diff for: lldb/include/lldb/Expression/IRExecutionUnit.h

+12
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "llvm/ExecutionEngine/SectionMemoryManager.h"
1818
#include "llvm/IR/Module.h"
1919

20+
#include "lldb/Core/ModuleList.h"
2021
#include "lldb/Expression/IRMemoryMap.h"
2122
#include "lldb/Expression/ObjectFileJIT.h"
2223
#include "lldb/Symbol/SymbolContext.h"
@@ -161,6 +162,12 @@ class IRExecutionUnit : public std::enable_shared_from_this<IRExecutionUnit>,
161162
return m_jitted_global_variables;
162163
}
163164

165+
void AppendPreferredSymbolContexts(SymbolContextList const &contexts) {
166+
for (auto const &ctx : contexts)
167+
if (ctx.module_sp)
168+
m_preferred_modules.Append(ctx.module_sp);
169+
}
170+
164171
private:
165172
/// Look up the object in m_address_map that contains a given address, find
166173
/// where it was copied to, and return the remote address at the same offset
@@ -396,6 +403,11 @@ class IRExecutionUnit : public std::enable_shared_from_this<IRExecutionUnit>,
396403
///< defining no functions using that variable, would do this.) If this
397404
///< is true, any allocations need to be committed immediately -- no
398405
///< opportunity for relocation.
406+
407+
///< Any Module in this list will be used for symbol/function lookup
408+
///< before any other module (except for the module corresponding to the
409+
///< current frame).
410+
ModuleList m_preferred_modules;
399411
};
400412

401413
} // namespace lldb_private

Diff for: lldb/include/lldb/Target/Target.h

+14
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "lldb/Core/UserSettingsController.h"
2626
#include "lldb/Expression/Expression.h"
2727
#include "lldb/Host/ProcessLaunchInfo.h"
28+
#include "lldb/Symbol/SymbolContext.h"
2829
#include "lldb/Symbol/TypeSystem.h"
2930
#include "lldb/Target/ExecutionContextScope.h"
3031
#include "lldb/Target/PathMappingList.h"
@@ -332,6 +333,14 @@ class EvaluateExpressionOptions {
332333
m_language = SourceLanguage(language_type);
333334
}
334335

336+
void SetPreferredSymbolContexts(SymbolContextList contexts) {
337+
m_preferred_lookup_contexts = std::move(contexts);
338+
}
339+
340+
const SymbolContextList &GetPreferredSymbolContexts() const {
341+
return m_preferred_lookup_contexts;
342+
}
343+
335344
/// Set the language using a pair of language code and version as
336345
/// defined by the DWARF 6 specification.
337346
/// WARNING: These codes may change until DWARF 6 is finalized.
@@ -500,6 +509,11 @@ class EvaluateExpressionOptions {
500509
// originates
501510
mutable std::string m_pound_line_file;
502511
mutable uint32_t m_pound_line_line = 0;
512+
513+
///< During expression evaluation, any SymbolContext in this list will be
514+
///< used for symbol/function lookup before any other context (except for
515+
///< the module corresponding to the current frame).
516+
SymbolContextList m_preferred_lookup_contexts;
503517
};
504518

505519
// Target

Diff for: lldb/source/Expression/IRExecutionUnit.cpp

+26-5
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ IRExecutionUnit::IRExecutionUnit(std::unique_ptr<llvm::LLVMContext> &context_up,
5252
m_cpu_features(cpu_features), m_name(name), m_sym_ctx(sym_ctx),
5353
m_did_jit(false), m_function_load_addr(LLDB_INVALID_ADDRESS),
5454
m_function_end_load_addr(LLDB_INVALID_ADDRESS),
55-
m_reported_allocations(false) {}
55+
m_reported_allocations(false), m_preferred_modules() {}
5656

5757
lldb::addr_t IRExecutionUnit::WriteNow(const uint8_t *bytes, size_t size,
5858
Status &error) {
@@ -782,8 +782,11 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
782782
}
783783

784784
ModuleList non_local_images = target->GetImages();
785-
// We'll process module_sp separately, before the other modules.
785+
// We'll process module_sp and any preferred modules separately, before the
786+
// other modules.
786787
non_local_images.Remove(sc.module_sp);
788+
for (size_t i = 0; i < m_preferred_modules.GetSize(); ++i)
789+
non_local_images.Remove(m_preferred_modules.GetModuleAtIndex(i));
787790

788791
LoadAddressResolver resolver(target, symbol_was_missing_weak);
789792

@@ -794,9 +797,11 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
794797
for (const ConstString &name : names) {
795798
// The lookup order here is as follows:
796799
// 1) Functions in `sc.module_sp`
797-
// 2) Functions in the other modules
798-
// 3) Symbols in `sc.module_sp`
799-
// 4) Symbols in the other modules
800+
// 2) Functions in the preferred modules list
801+
// 3) Functions in the other modules
802+
// 4) Symbols in `sc.module_sp`
803+
// 5) Symbols in the preferred modules list
804+
// 6) Symbols in the other modules
800805
if (sc.module_sp) {
801806
SymbolContextList sc_list;
802807
sc.module_sp->FindFunctions(name, CompilerDeclContext(),
@@ -806,6 +811,14 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
806811
return *load_addr;
807812
}
808813

814+
{
815+
SymbolContextList sc_list;
816+
m_preferred_modules.FindFunctions(name, lldb::eFunctionNameTypeFull,
817+
function_options, sc_list);
818+
if (auto load_addr = resolver.Resolve(sc_list))
819+
return *load_addr;
820+
}
821+
809822
{
810823
SymbolContextList sc_list;
811824
non_local_images.FindFunctions(name, lldb::eFunctionNameTypeFull,
@@ -822,6 +835,14 @@ IRExecutionUnit::FindInSymbols(const std::vector<ConstString> &names,
822835
return *load_addr;
823836
}
824837

838+
{
839+
SymbolContextList sc_list;
840+
m_preferred_modules.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,
841+
sc_list);
842+
if (auto load_addr = resolver.Resolve(sc_list))
843+
return *load_addr;
844+
}
845+
825846
{
826847
SymbolContextList sc_list;
827848
non_local_images.FindSymbolsWithNameAndType(name, lldb::eSymbolTypeAny,

Diff for: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -1539,6 +1539,10 @@ lldb_private::Status ClangExpressionParser::DoPrepareForExecution(
15391539
function_name, exe_ctx.GetTargetSP(), sc,
15401540
m_compiler->getTargetOpts().Features);
15411541

1542+
if (auto *options = m_expr.GetOptions())
1543+
execution_unit_sp->AppendPreferredSymbolContexts(
1544+
options->GetPreferredSymbolContexts());
1545+
15421546
ClangExpressionHelper *type_system_helper =
15431547
dyn_cast<ClangExpressionHelper>(m_expr.GetTypeSystemHelper());
15441548
ClangExpressionDeclMap *decl_map =

Diff for: lldb/source/Plugins/InstrumentationRuntime/Utility/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
add_lldb_library(lldbPluginInstrumentationRuntimeUtility
22
ReportRetriever.cpp
3+
Utility.cpp
34

45
LINK_LIBS
56
lldbBreakpoint

Diff for: lldb/source/Plugins/InstrumentationRuntime/Utility/ReportRetriever.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "ReportRetriever.h"
10+
#include "Utility.h"
1011

1112
#include "lldb/Breakpoint/StoppointCallbackContext.h"
1213
#include "lldb/Core/Debugger.h"
@@ -82,6 +83,12 @@ ReportRetriever::RetrieveReportData(const ProcessSP process_sp) {
8283
options.SetAutoApplyFixIts(false);
8384
options.SetLanguage(eLanguageTypeObjC_plus_plus);
8485

86+
if (auto m = GetPreferredAsanModule(process_sp->GetTarget())) {
87+
SymbolContextList sc_list;
88+
sc_list.Append(SymbolContext(std::move(m)));
89+
options.SetPreferredSymbolContexts(std::move(sc_list));
90+
}
91+
8592
ValueObjectSP return_value_sp;
8693
ExecutionContext exe_ctx;
8794
frame_sp->CalculateExecutionContext(exe_ctx);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===-- Utility.cpp -------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "Utility.h"
10+
11+
#include "lldb/Core/Module.h"
12+
#include "lldb/Target/Target.h"
13+
14+
namespace lldb_private {
15+
16+
///< On Darwin, if LLDB loaded libclang_rt, it's coming from a locally built
17+
///< compiler-rt, and we should prefer it in favour of the system sanitizers.
18+
///< This helper searches the target for such a dylib. Returns nullptr if no
19+
///< such dylib was found.
20+
lldb::ModuleSP GetPreferredAsanModule(const Target &target) {
21+
lldb::ModuleSP module;
22+
llvm::Regex pattern(R"(libclang_rt\.asan_.*_dynamic\.dylib)");
23+
target.GetImages().ForEach([&](const lldb::ModuleSP &m) {
24+
if (pattern.match(m->GetFileSpec().GetFilename().GetStringRef())) {
25+
module = m;
26+
return false;
27+
}
28+
29+
return true;
30+
});
31+
32+
return module;
33+
}
34+
35+
} // namespace lldb_private
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//===-- Utility.h -----------------------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLDB_SOURCE_PLUGINS_INSTRUMENTATIONRUNTIME_UTILITY_UTILITY_H
10+
#define LLDB_SOURCE_PLUGINS_INSTRUMENTATIONRUNTIME_UTILITY_UTILITY_H
11+
12+
#include "lldb/lldb-forward.h"
13+
14+
namespace lldb_private {
15+
16+
class Target;
17+
18+
///< On Darwin, if LLDB loaded libclang_rt, it's coming from a locally built
19+
///< compiler-rt, and we should prefer it in favour of the system sanitizers
20+
///< when running InstrumentationRuntime utility expressions that use symbols
21+
///< from the sanitizer libraries. This helper searches the target for such a
22+
///< dylib. Returns nullptr if no such dylib was found.
23+
lldb::ModuleSP GetPreferredAsanModule(const Target &target);
24+
25+
} // namespace lldb_private
26+
27+
#endif // LLDB_SOURCE_PLUGINS_INSTRUMENTATIONRUNTIME_UTILITY_UTILITY_H

Diff for: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp

+8
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88

99
#include "MemoryHistoryASan.h"
1010

11+
#include "lldb/Symbol/SymbolContext.h"
1112
#include "lldb/Target/MemoryHistory.h"
1213

14+
#include "Plugins/InstrumentationRuntime/Utility/Utility.h"
1315
#include "Plugins/Process/Utility/HistoryThread.h"
1416
#include "lldb/Core/Debugger.h"
1517
#include "lldb/Core/Module.h"
@@ -174,6 +176,12 @@ HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) {
174176
options.SetAutoApplyFixIts(false);
175177
options.SetLanguage(eLanguageTypeObjC_plus_plus);
176178

179+
if (auto m = GetPreferredAsanModule(process_sp->GetTarget())) {
180+
SymbolContextList sc_list;
181+
sc_list.Append(SymbolContext(std::move(m)));
182+
options.SetPreferredSymbolContexts(std::move(sc_list));
183+
}
184+
177185
ExpressionResults expr_result = UserExpression::Evaluate(
178186
exe_ctx, options, expr.GetString(), "", return_value_sp);
179187
if (expr_result != eExpressionCompleted) {

0 commit comments

Comments
 (0)