From da3efb2ae32c702e68aef06b161f5bbc85f8ab2b Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 17 Jun 2025 15:24:26 +0100 Subject: [PATCH 1/5] [lldb][Expression] Remove IR pointer checker (#144483) Currently when jitting expressions, LLDB scans the IR instructions of the `$__lldb_expr` and will insert a call to a utility function for each load/store instruction. The purpose of the utility funciton is to dereference the load/store operand. If that operand was an invalid pointer the utility function would trap and LLDB asks the IR checker whether it was responsible for the trap, in which case it prints out an error message saying the expression dereferenced an invalid pointer. This is a lot of setup for not much gain. In fact, creating/running this utility expression shows up as ~2% of the expression evaluation time (though we cache them for subsequent expressions). And the error message we get out of it is arguably less useful than if we hadn't instrumented the IR. It was also untested. Before: ``` (lldb) expr int a = *returns_invalid_ptr() error: Execution was interrupted, reason: Attempted to dereference an invalid pointer.. The process has been returned to the state before expression evaluation. ``` After: ``` (lldb) expr int a = *returns_invalid_ptr() error: Expression execution was interrupted: EXC_BAD_ACCESS (code=1, address=0x5). The process has been returned to the state before expression evaluation. ``` This patch removes this IR checker. (cherry picked from commit 0a7b0c844c59189ad4f5072b73d7dfdfd78e76b7) --- .../Clang/IRDynamicChecks.cpp | 107 +----------------- .../ExpressionParser/Clang/IRDynamicChecks.h | 1 - .../lldb-dap/save-core/TestDAP_save_core.py | 6 - 3 files changed, 4 insertions(+), 110 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp index ba946561093eb..3e197826c8983 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp @@ -32,31 +32,16 @@ using namespace lldb_private; static char ID; -#define VALID_POINTER_CHECK_NAME "_$__lldb_valid_pointer_check" #define VALID_OBJC_OBJECT_CHECK_NAME "$__lldb_objc_object_check" -static const char g_valid_pointer_check_text[] = - "extern \"C\" void\n" - "_$__lldb_valid_pointer_check (unsigned char *$__lldb_arg_ptr)\n" - "{\n" - " unsigned char $__lldb_local_val = *$__lldb_arg_ptr;\n" - "}"; - ClangDynamicCheckerFunctions::ClangDynamicCheckerFunctions() : DynamicCheckerFunctions(DCF_Clang) {} ClangDynamicCheckerFunctions::~ClangDynamicCheckerFunctions() = default; -llvm::Error ClangDynamicCheckerFunctions::Install( - DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) { - Expected> utility_fn = - exe_ctx.GetTargetRef().CreateUtilityFunction( - g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME, - lldb::eLanguageTypeC, exe_ctx); - if (!utility_fn) - return utility_fn.takeError(); - m_valid_pointer_check = std::move(*utility_fn); - +llvm::Error +ClangDynamicCheckerFunctions::Install(DiagnosticManager &diagnostic_manager, + ExecutionContext &exe_ctx) { if (Process *process = exe_ctx.GetProcessPtr()) { ObjCLanguageRuntime *objc_language_runtime = ObjCLanguageRuntime::Get(*process); @@ -78,11 +63,7 @@ bool ClangDynamicCheckerFunctions::DoCheckersExplainStop(lldb::addr_t addr, // FIXME: We have to get the checkers to know why they scotched the call in // more detail, // so we can print a better message here. - if (m_valid_pointer_check && m_valid_pointer_check->ContainsAddress(addr)) { - message.Printf("Attempted to dereference an invalid pointer."); - return true; - } else if (m_objc_object_check && - m_objc_object_check->ContainsAddress(addr)) { + if (m_objc_object_check && m_objc_object_check->ContainsAddress(addr)) { message.Printf("Attempted to dereference an invalid ObjC Object or send it " "an unrecognized selector"); return true; @@ -224,29 +205,6 @@ class Instrumenter { return true; } - /// Build a function pointer for a function with signature void - /// (*)(uint8_t*) with a given address - /// - /// \param[in] start_address - /// The address of the function. - /// - /// \return - /// The function pointer, for use in a CallInst. - llvm::FunctionCallee BuildPointerValidatorFunc(lldb::addr_t start_address) { - llvm::Type *param_array[1]; - - param_array[0] = const_cast(GetI8PtrTy()); - - ArrayRef params(param_array, 1); - - FunctionType *fun_ty = FunctionType::get( - llvm::Type::getVoidTy(m_module.getContext()), params, true); - PointerType *fun_ptr_ty = PointerType::getUnqual(fun_ty); - Constant *fun_addr_int = - ConstantInt::get(GetIntptrTy(), start_address, false); - return {fun_ty, ConstantExpr::getIntToPtr(fun_addr_int, fun_ptr_ty)}; - } - /// Build a function pointer for a function with signature void /// (*)(uint8_t*, uint8_t*) with a given address /// @@ -301,53 +259,6 @@ class Instrumenter { IntegerType *m_intptr_ty = nullptr; }; -class ValidPointerChecker : public Instrumenter { -public: - ValidPointerChecker(llvm::Module &module, - std::shared_ptr checker_function) - : Instrumenter(module, checker_function), - m_valid_pointer_check_func(nullptr) {} - - ~ValidPointerChecker() override = default; - -protected: - bool InstrumentInstruction(llvm::Instruction *inst) override { - Log *log = GetLog(LLDBLog::Expressions); - - LLDB_LOGF(log, "Instrumenting load/store instruction: %s\n", - PrintValue(inst).c_str()); - - if (!m_valid_pointer_check_func) - m_valid_pointer_check_func = - BuildPointerValidatorFunc(m_checker_function->StartAddress()); - - llvm::Value *dereferenced_ptr = nullptr; - - if (llvm::LoadInst *li = dyn_cast(inst)) - dereferenced_ptr = li->getPointerOperand(); - else if (llvm::StoreInst *si = dyn_cast(inst)) - dereferenced_ptr = si->getPointerOperand(); - else - return false; - - // Insert an instruction to call the helper with the result - CallInst::Create(m_valid_pointer_check_func, dereferenced_ptr, "", - inst->getIterator()); - - return true; - } - - bool InspectInstruction(llvm::Instruction &i) override { - if (isa(&i) || isa(&i)) - RegisterInstruction(i); - - return true; - } - -private: - llvm::FunctionCallee m_valid_pointer_check_func; -}; - class ObjcObjectChecker : public Instrumenter { public: ObjcObjectChecker(llvm::Module &module, @@ -528,16 +439,6 @@ bool IRDynamicChecks::runOnModule(llvm::Module &M) { return false; } - if (m_checker_functions.m_valid_pointer_check) { - ValidPointerChecker vpc(M, m_checker_functions.m_valid_pointer_check); - - if (!vpc.Inspect(*function)) - return false; - - if (!vpc.Instrument()) - return false; - } - if (m_checker_functions.m_objc_object_check) { ObjcObjectChecker ooc(M, m_checker_functions.m_objc_object_check); diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h index ff20c1f08be0c..f67229afc2152 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h @@ -53,7 +53,6 @@ class ClangDynamicCheckerFunctions bool DoCheckersExplainStop(lldb::addr_t addr, Stream &message) override; - std::shared_ptr m_valid_pointer_check; std::shared_ptr m_objc_object_check; }; diff --git a/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py b/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py index 0bda7ee6e5b83..bfd51a69e7d63 100644 --- a/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py +++ b/lldb/test/API/tools/lldb-dap/save-core/TestDAP_save_core.py @@ -33,13 +33,7 @@ def test_save_core(self): # Getting dap stack trace may trigger __lldb_caller_function JIT module to be created. self.get_stackFrames(startFrame=0) - # Evaluating an expression that cause "_$__lldb_valid_pointer_check" JIT module to be created. - expression = 'printf("this is a test")' - self.dap_server.request_evaluate(expression, context="watch") - - # Verify "_$__lldb_valid_pointer_check" JIT module is created. modules = self.dap_server.get_modules() - self.assertTrue(modules["_$__lldb_valid_pointer_check"]) thread_count = len(self.dap_server.get_threads()) core_stack = self.getBuildArtifact("core.stack.dmp") From d4b4877ea3377b09837c411dffc98cecc4ebc5dc Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 17 Jun 2025 16:33:24 +0100 Subject: [PATCH 2/5] [lldb][Expression] Don't create Objective-C IR checker for pure-C++ targets/frames (#144503) There's no need to create this utility function (and run it) for targets/frames that aren't Objective-C/Objective-C++. (cherry picked from commit 4ced29b8482e3537da7d27d410bf7947b0666b4c) --- .../Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp index 3e197826c8983..8f3e4abc4f095 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp @@ -18,6 +18,7 @@ #include "lldb/Expression/UtilityFunction.h" #include "lldb/Target/ExecutionContext.h" +#include "lldb/Target/Language.h" #include "lldb/Target/Process.h" #include "lldb/Target/StackFrame.h" #include "lldb/Target/Target.h" @@ -46,7 +47,13 @@ ClangDynamicCheckerFunctions::Install(DiagnosticManager &diagnostic_manager, ObjCLanguageRuntime *objc_language_runtime = ObjCLanguageRuntime::Get(*process); - if (objc_language_runtime) { + SourceLanguage lang = process->GetTarget().GetLanguage(); + if (!lang) + if (auto *frame = exe_ctx.GetFramePtr()) + lang = frame->GetLanguage(); + + if (objc_language_runtime && + Language::LanguageIsObjC(lang.AsLanguageType())) { Expected> checker_fn = objc_language_runtime->CreateObjectChecker(VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx); if (!checker_fn) From 3a15ac589ef36604ca70448474311d7b55a3e645 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 17 Jun 2025 17:37:27 +0100 Subject: [PATCH 3/5] [lldb][Formatter] Get element type for unordered_maps from __hash_table::value_type (#144517) https://github.com/llvm/llvm-project/pull/143501 changes usage of `__hash_value_type` in libcxx to an empty tag type. This type will no longer have a definition in DWARF. Currently the LLDB unordered_map formatter deduces the map's `element_type` by looking at the `__cc_` member of `__hash_value_type`. But that will no longer work because we only have its forward declaration. Since what we're really after is the type that `__hash_value_type` is wrapping, we can just look at the `__hash_table::value_type` typedef. With https://github.com/llvm/llvm-project/pull/143501 that will now point to the `std::pair` element type (which used to be what we got from `__cc_`). TBD: need to double-check this works for older layouts. Quick glance at the code makes me suspicious of cases like `unordered_map, int>` (cherry picked from commit 382e3fdbb476a5d5771b315daedcd05a15883fbc) --- .../Language/CPlusPlus/LibCxxUnorderedMap.cpp | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp index 292b96bdc84f7..7791721daeb43 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp @@ -103,15 +103,22 @@ static bool isUnorderedMap(ConstString type_name) { CompilerType lldb_private::formatters::LibcxxStdUnorderedMapSyntheticFrontEnd:: GetElementType(CompilerType table_type) { - auto element_type = table_type.GetTypedefedType().GetTypeTemplateArgument(0); + auto element_type = + table_type.GetDirectNestedTypeWithName("value_type").GetTypedefedType(); + + // In newer unordered_map layouts, the std::pair element type isn't wrapped + // in any helper types. So return it directly. + if (isStdTemplate(element_type.GetTypeName(), "pair")) + return element_type; // This synthetic provider is used for both unordered_(multi)map and - // unordered_(multi)set. For unordered_map, the element type has an - // additional type layer, an internal struct (`__hash_value_type`) - // that wraps a std::pair. Peel away the internal wrapper type - whose - // structure is of no value to users, to expose the std::pair. This - // matches the structure returned by the std::map synthetic provider. - if (isUnorderedMap(m_backend.GetTypeName())) { + // unordered_(multi)set. For older unordered_map layouts, the element type has + // an additional type layer, an internal struct (`__hash_value_type`) that + // wraps a std::pair. Peel away the internal wrapper type - whose structure is + // of no value to users, to expose the std::pair. This matches the structure + // returned by the std::map synthetic provider. + if (isUnorderedMap( + m_backend.GetCompilerType().GetCanonicalType().GetTypeName())) { std::string name; CompilerType field_type = element_type.GetFieldAtIndex(0, name, nullptr, nullptr, nullptr); From 9b4f290cf29cabf15cdecc05566470c3595613fa Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Wed, 18 Jun 2025 11:03:23 +0100 Subject: [PATCH 4/5] [lldb][Format] Update std::deque formatter to account for libc++ naming changes Cherry-picked the LLDB parts from: ``` commit c7df10643bda4acdc9a02406a2eee8aa4ced747f Author: Peng Liu Date: Wed Nov 13 05:08:08 2024 -0500 Unify naming of internal pointer members in std::vector and std::__split_buffer (#115517) ``` Addresses https://github.com/llvm/llvm-project/issues/144555 --- lldb/examples/synthetic/libcxx.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lldb/examples/synthetic/libcxx.py b/lldb/examples/synthetic/libcxx.py index b078a4eb2f639..7a71556e779df 100644 --- a/lldb/examples/synthetic/libcxx.py +++ b/lldb/examples/synthetic/libcxx.py @@ -764,9 +764,10 @@ def update(self): map_.GetChildMemberWithName("__end_cap_") ) else: - map_endcap = map_.GetChildMemberWithName( - "__end_cap_" - ).GetValueAsUnsigned(0) + map_endcap = map_.GetChildMemberWithName("__cap_") + if not map_endcap.IsValid(): + map_endcap = map_.GetChildMemberWithName("__end_cap_") + map_endcap = map_endcap.GetValueAsUnsigned(0) # check consistency if not map_first <= map_begin <= map_end <= map_endcap: From 7f41bdab7d173bd131b987cb50079744fa220b5b Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Fri, 14 Feb 2025 08:51:32 +0100 Subject: [PATCH 5/5] [lldb] Avoid expression evaluation in the std::deque formatter (#127071) It's slower and it can fail in contexts where expression evaluation doesn't work. (cherry picked from commit 0949330669cbd179c3c6e40880b5e1027438648f) --- lldb/examples/synthetic/libcxx.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lldb/examples/synthetic/libcxx.py b/lldb/examples/synthetic/libcxx.py index 7a71556e779df..5abeb3061f4f5 100644 --- a/lldb/examples/synthetic/libcxx.py +++ b/lldb/examples/synthetic/libcxx.py @@ -694,6 +694,13 @@ def get_child_index(self, name): except: return -1 + @staticmethod + def _subscript(ptr: lldb.SBValue, idx: int, name: str) -> lldb.SBValue: + """Access a pointer value as if it was an array. Returns ptr[idx].""" + deref_t = ptr.GetType().GetPointeeType() + offset = idx * deref_t.GetByteSize() + return ptr.CreateChildAtOffset(name, offset, deref_t) + def get_child_at_index(self, index): logger = lldb.formatters.Logger.Logger() logger.write("Fetching child " + str(index)) @@ -703,11 +710,8 @@ def get_child_at_index(self, index): return None try: i, j = divmod(self.start + index, self.block_size) - - return self.first.CreateValueFromExpression( - "[" + str(index) + "]", - "*(*(%s + %d) + %d)" % (self.map_begin.get_expr_path(), i, j), - ) + val = stddeque_SynthProvider._subscript(self.map_begin, i, "") + return stddeque_SynthProvider._subscript(val, j, f"[{index}]") except: return None