https://github.com/lhames updated https://github.com/llvm/llvm-project/pull/215710
>From a7a6de58241b2fb1cf92532d109e68157f4701d5 Mon Sep 17 00:00:00 2001 From: Lang Hames <[email protected]> Date: Wed, 12 Aug 2026 12:18:42 +1000 Subject: [PATCH 1/2] [lldb] Guard against null dereference in GetCppObjectPointer ClangUserExpression::GetCppObjectPointer dereferenced the ValueObjectSP returned by GetObjectPointerValueObject before checking it for null. Fix by moving the existing check above the child lookups. --- .../Plugins/ExpressionParser/Clang/ClangUserExpression.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp index b7ebb6dfd5551..62652e200baca 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp @@ -854,6 +854,9 @@ lldb::addr_t ClangUserExpression::GetCppObjectPointer( auto valobj_sp = GetObjectPointerValueObject(std::move(frame_sp), object_name, err); + if (!err.Success() || !valobj_sp) + return LLDB_INVALID_ADDRESS; + // We're inside a C++ class method. This could potentially be an unnamed // lambda structure. If the lambda captured a "this", that should be // the object pointer. @@ -862,9 +865,6 @@ lldb::addr_t ClangUserExpression::GetCppObjectPointer( else if (auto cv_this_child_sp = valobj_sp->GetChildMemberWithName("__this")) valobj_sp = cv_this_child_sp; - if (!err.Success() || !valobj_sp.get()) - return LLDB_INVALID_ADDRESS; - lldb::addr_t ret = valobj_sp->GetValueAsUnsigned(LLDB_INVALID_ADDRESS); if (ret == LLDB_INVALID_ADDRESS) { >From 88154238ffede58a71acd06072a23c138d335751 Mon Sep 17 00:00:00 2001 From: Lang Hames <[email protected]> Date: Wed, 12 Aug 2026 15:03:56 +1000 Subject: [PATCH 2/2] Add a unit test. --- .../Clang/ClangUserExpression.h | 9 +++- lldb/unittests/Expression/CMakeLists.txt | 1 + .../Expression/ClangUserExpressionTest.cpp | 43 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 lldb/unittests/Expression/ClangUserExpressionTest.cpp diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h index fa2bc04924c63..f12b9dd397074 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h @@ -43,6 +43,9 @@ class ClangUserExpression : public LLVMUserExpression { // LLVM RTTI support static char ID; + /// Grants the unit test access to private members. + friend class ClangUserExpressionTest; + public: bool isA(const void *ClassID) const override { return ClassID == &ID || LLVMUserExpression::isA(ClassID); @@ -209,8 +212,10 @@ class ClangUserExpression : public LLVMUserExpression { std::vector<std::string> modules_to_import, bool for_completion); - lldb::addr_t GetCppObjectPointer(lldb::StackFrameSP frame, - llvm::StringRef object_name, Status &err); + /// Get the object pointer ("this") for a C++ method context. + static lldb::addr_t GetCppObjectPointer(lldb::StackFrameSP frame, + llvm::StringRef object_name, + Status &err); void FixupCVRParseErrorDiagnostics(DiagnosticManager &diagnostic_manager) const; diff --git a/lldb/unittests/Expression/CMakeLists.txt b/lldb/unittests/Expression/CMakeLists.txt index 252e2ffa32398..40d10d5dc0726 100644 --- a/lldb/unittests/Expression/CMakeLists.txt +++ b/lldb/unittests/Expression/CMakeLists.txt @@ -6,6 +6,7 @@ endif() add_lldb_unittest(ExpressionTests ClangParserTest.cpp ClangExpressionDeclMapTest.cpp + ClangUserExpressionTest.cpp DiagnosticManagerTest.cpp DWARFExpressionTest.cpp CppModuleConfigurationTest.cpp diff --git a/lldb/unittests/Expression/ClangUserExpressionTest.cpp b/lldb/unittests/Expression/ClangUserExpressionTest.cpp new file mode 100644 index 0000000000000..c91a995531166 --- /dev/null +++ b/lldb/unittests/Expression/ClangUserExpressionTest.cpp @@ -0,0 +1,43 @@ +//===-- ClangUserExpressionTest.cpp ---------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Plugins/ExpressionParser/Clang/ClangUserExpression.h" +#include "lldb/Utility/Status.h" +#include "lldb/lldb-defines.h" +#include "gtest/gtest.h" + +namespace lldb_private { + +/// ClangUserExpression declares this fixture a friend so that the tests below +/// can reach its private helpers. +/// +/// TEST_F derives from this fixture and friendship isn't inherited, so the +/// private access has to happen in a member of the fixture itself rather than +/// in a test body. +class ClangUserExpressionTest : public testing::Test { +protected: + static lldb::addr_t CallGetCppObjectPointer(lldb::StackFrameSP frame, + llvm::StringRef object_name, + Status &err) { + return ClangUserExpression::GetCppObjectPointer(std::move(frame), + object_name, err); + } +}; + +// GetCppObjectPointer must check the ValueObjectSP it gets back from +// GetObjectPointerValueObject before dereferencing it to look for a captured +// "this": that function returns a null SP when it can't find the object. A null +// frame is the cheapest way to make it do so. +TEST_F(ClangUserExpressionTest, GetCppObjectPointerWithoutFrame) { + Status err; + EXPECT_EQ(CallGetCppObjectPointer(nullptr, "this", err), + LLDB_INVALID_ADDRESS); + EXPECT_FALSE(err.Success()); +} + +} // namespace lldb_private _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
