[Lldb-commits] [lldb] [lldb] Guard against null dereference in GetCppObjectPointer (PR #215710)

[Lang Hames via lldb-commits]

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 <lhames at gmail.com>
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 <lhames at gmail.com>
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