[lldb] Fix double free in CommandPluginInterfaceImplementation

[JDevlieghere]

The class was taking ownership of the SBCommandPluginInterface pointer it was passed in, by wrapping it in a shared pointer. This causes a double free in the unit test when the object is destroyed and the same pointer gets freed once when the SBCommandPluginInterface goes away and then again when the shared pointer hits a zero refcount.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

The class was taking ownership of the SBCommandPluginInterface pointer it was passed in, by wrapping it in a shared pointer. This causes a double free in the unit test when the object is destroyed and the same pointer gets freed once when the SBCommandPluginInterface goes away and then again when the shared pointer hits a zero refcount.

---

Full diff: https://github.com/llvm/llvm-project/pull/131658.diff

2 Files Affected:

• (modified) lldb/source/API/SBCommandInterpreter.cpp (+1-1)
• (modified) lldb/unittests/API/SBCommandInterpreterTest.cpp (+11-7)

diff --git a/lldb/source/API/SBCommandInterpreter.cpp b/lldb/source/API/SBCommandInterpreter.cpp
index d153e2acdf853..de22a9dd96bd8 100644
--- a/lldb/source/API/SBCommandInterpreter.cpp
+++ b/lldb/source/API/SBCommandInterpreter.cpp
@@ -77,7 +77,7 @@ class CommandPluginInterfaceImplementation : public CommandObjectParsed {
     SBDebugger debugger_sb(m_interpreter.GetDebugger().shared_from_this());
     m_backend->DoExecute(debugger_sb, command.GetArgumentVector(), sb_return);
   }
-  std::shared_ptr<lldb::SBCommandPluginInterface> m_backend;
+  lldb::SBCommandPluginInterface *m_backend;
   std::optional<std::string> m_auto_repeat_command;
 };
 } // namespace lldb_private
diff --git a/lldb/unittests/API/SBCommandInterpreterTest.cpp b/lldb/unittests/API/SBCommandInterpreterTest.cpp
index 941b738e84ac8..5651e1c3dc63f 100644
--- a/lldb/unittests/API/SBCommandInterpreterTest.cpp
+++ b/lldb/unittests/API/SBCommandInterpreterTest.cpp
@@ -6,24 +6,28 @@
 //
 //===----------------------------------------------------------------------===/
 
-#include "gtest/gtest.h"
-
 // Use the umbrella header for -Wdocumentation.
 #include "lldb/API/LLDB.h"
 
+#include "TestingSupport/SubsystemRAII.h"
+#include "lldb/API/SBDebugger.h"
+#include "gtest/gtest.h"
 #include <cstring>
 #include <string>
 
 using namespace lldb;
+using namespace lldb_private;
 
 class SBCommandInterpreterTest : public testing::Test {
 protected:
   void SetUp() override {
-    SBDebugger::Initialize();
-    m_dbg = SBDebugger::Create(/*source_init_files=*/false);
+    debugger = SBDebugger::Create(/*source_init_files=*/false);
   }
 
-  SBDebugger m_dbg;
+  void TearDown() override { SBDebugger::Destroy(debugger); }
+
+  SubsystemRAII<lldb::SBDebugger> subsystems;
+  SBDebugger debugger;
 };
 
 class DummyCommand : public SBCommandPluginInterface {
@@ -44,7 +48,7 @@ class DummyCommand : public SBCommandPluginInterface {
 TEST_F(SBCommandInterpreterTest, SingleWordCommand) {
   // We first test a command without autorepeat
   DummyCommand dummy("It worked");
-  SBCommandInterpreter interp = m_dbg.GetCommandInterpreter();
+  SBCommandInterpreter interp = debugger.GetCommandInterpreter();
   interp.AddCommand("dummy", &dummy, /*help=*/nullptr);
   {
     SBCommandReturnObject result;
@@ -78,7 +82,7 @@ TEST_F(SBCommandInterpreterTest, SingleWordCommand) {
 }
 
 TEST_F(SBCommandInterpreterTest, MultiWordCommand) {
-  SBCommandInterpreter interp = m_dbg.GetCommandInterpreter();
+  SBCommandInterpreter interp = debugger.GetCommandInterpreter();
   auto command = interp.AddMultiwordCommand("multicommand", /*help=*/nullptr);
   // We first test a subcommand without autorepeat
   DummyCommand subcommand("It worked again");

[bulbazord]

Dastardly!

[alexfh]

We started seeing assertion failures in SBCommandInterpreterTest after this commit:

[ RUN      ] SBCommandInterpreterTest.SingleWordCommand
[       OK ] SBCommandInterpreterTest.SingleWordCommand (245 ms)
[ RUN      ] SBCommandInterpreterTest.MultiWordCommand
assertion failed at llvm-project/lldb/source/Utility/Log.cpp:223 in static void lldb_private::Log::Register(llvm::StringRef, Channel &): iter.second == true
    @     0x7f62501313f4  __assert_fail
    @     0x7f62783bbac7  lldb_private::Log::Register()
    @     0x7f62a1fff376  lldb_private::SystemInitializerCommon::Initialize()
    @     0x7f62bb4e4ce5  lldb_private::SystemInitializerFull::Initialize()
    @     0x7f62a1fff784  lldb_private::SystemLifetimeManager::Initialize()
    @     0x7f62bb3ee26f  lldb::SBDebugger::InitializeWithErrorHandling()
    @     0x7f62bb3ee144  lldb::SBDebugger::Initialize()
    @     0x7f62bc5db0be  testing::internal::TestFactoryImpl<>::CreateTest()
    @     0x7f62644d1903  testing::TestInfo::Run()
    @     0x7f62644d2ab3  testing::TestSuite::Run()
    @     0x7f62644e5a30  testing::internal::UnitTestImpl::RunAllTests()
    @     0x7f62644e5420  testing::UnitTest::Run()
    @     0x7f6264d0dc1d  main
    @     0x7f629230b3d4  __libc_start_main
    @     0x556100b3476a  _start

Apart from that, we see msan reports

[ RUN      ] SBCommandInterpreterTest.SingleWordCommand
[       OK ] SBCommandInterpreterTest.SingleWordCommand (501 ms)
[ RUN      ] SBCommandInterpreterTest.MultiWordCommand
==398==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7fb86f56529a in lldb_private::Diagnostics::AddCallback(std::__msan::function<llvm::Error (lldb_private::FileSpec const&)>) llvm-project/lldb/source/Utility/Diagnostics.cpp:51:1
    #1 0x7fb88edaca57 in lldb_private::Debugger::Debugger(void (*)(char const*, void*), void*) llvm-project/lldb/source/Core/Debugger.cpp:967:57
    #2 0x7fb88eda9fb2 in lldb_private::Debugger::CreateInstance(void (*)(char const*, void*), void*) llvm-project/lldb/source/Core/Debugger.cpp:773:30
    #3 0x7fb8b4c400ce in lldb::SBDebugger::Create(bool, void (*)(char const*, void*), void*) llvm-project/lldb/source/API/SBDebugger.cpp:291:18
    #4 0x7fb8b4c403af in lldb::SBDebugger::Create(bool) llvm-project/lldb/source/API/SBDebugger.cpp:272:10
    #5 0x7fb8b5eb39ff in SBCommandInterpreterTest::SetUp() llvm-project/lldb/unittests/API/SBCommandInterpreterTest.cpp:24:16
    #6 0x7fb8595a12f5 in HandleExceptionsInMethodIfSupported<testing::Test, void> llvm-project/third-party/unittest/googletest/src/gtest.cc
...

as well as leak reports from asan.

[alexfh]

@labath