[Lldb-commits] [lldb] 047c4b0 - [lldb/Target] Fix `frame recognizer list` crash when registered with nullptr

[Med Ismail Bennani via lldb-commits]

Author: Med Ismail Bennani
Date: 2020-02-07T17:35:29+01:00
New Revision: 047c4b0369f05ba2f96c79151641f07035bec954

URL: https://github.com/llvm/llvm-project/commit/047c4b0369f05ba2f96c79151641f07035bec954
DIFF: https://github.com/llvm/llvm-project/commit/047c4b0369f05ba2f96c79151641f07035bec954.diff

LOG: [lldb/Target] Fix `frame recognizer list` crash when registered with nullptr

One way to register a recognizer is to use RegularExpressionSP for the
module and symbol.

In order to match a symbol regardless of the module, the recognizer can
be registered with a nullptr for the module. However, this cause the
frame recognizer list command to crash because it calls
RegularExpression::GetText without checking if the shared pointer is valid.

This patch adds checks for the symbol and module RegularExpressionSP.

Differential Revision: https://reviews.llvm.org/D74212

Signed-off-by: Med Ismail Bennani <medismail.bennani at gmail.com>

Added: 
    lldb/unittests/Target/StackFrameRecognizerTest.cpp

Modified: 
    lldb/source/Target/StackFrameRecognizer.cpp
    lldb/unittests/Target/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/source/Target/StackFrameRecognizer.cpp b/lldb/source/Target/StackFrameRecognizer.cpp
index 67a320a29712..14cba23b4bfa 100644
--- a/lldb/source/Target/StackFrameRecognizer.cpp
+++ b/lldb/source/Target/StackFrameRecognizer.cpp
@@ -70,9 +70,17 @@ class StackFrameRecognizerManagerImpl {
                          std::string symbol, bool regexp)> const &callback) {
     for (auto entry : m_recognizers) {
       if (entry.is_regexp) {
-        callback(entry.recognizer_id, entry.recognizer->GetName(),
-                 std::string(entry.module_regexp->GetText()),
-                 std::string(entry.symbol_regexp->GetText()), true);
+        std::string module_name;
+        std::string symbol_name;
+
+        if (entry.module_regexp)
+          module_name = entry.module_regexp->GetText().str();
+        if (entry.symbol_regexp)
+          symbol_name = entry.symbol_regexp->GetText().str();
+
+        callback(entry.recognizer_id, entry.recognizer->GetName(), module_name,
+                 symbol_name, true);
+
       } else {
         callback(entry.recognizer_id, entry.recognizer->GetName(), entry.module.GetCString(),
                  entry.symbol.GetCString(), false);

diff  --git a/lldb/unittests/Target/CMakeLists.txt b/lldb/unittests/Target/CMakeLists.txt
index e63b1d077de3..4694139369d3 100644
--- a/lldb/unittests/Target/CMakeLists.txt
+++ b/lldb/unittests/Target/CMakeLists.txt
@@ -3,6 +3,7 @@ add_lldb_unittest(TargetTests
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
   PathMappingListTest.cpp
+  StackFrameRecognizerTest.cpp
 
   LINK_LIBS
       lldbCore
@@ -10,6 +11,7 @@ add_lldb_unittest(TargetTests
       lldbPluginObjectFileELF
       lldbPluginPlatformLinux
       lldbPluginSymbolFileSymtab
+      lldbTarget
       lldbSymbol
       lldbUtility
       lldbUtilityHelpers

diff  --git a/lldb/unittests/Target/StackFrameRecognizerTest.cpp b/lldb/unittests/Target/StackFrameRecognizerTest.cpp
new file mode 100644
index 000000000000..5220796acb4b
--- /dev/null
+++ b/lldb/unittests/Target/StackFrameRecognizerTest.cpp
@@ -0,0 +1,83 @@
+//===-- StackFrameRecognizerTest.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 "lldb/Target/StackFrameRecognizer.h"
+#include "Plugins/Platform/Linux/PlatformLinux.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/Reproducer.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "lldb/lldb-private-enumerations.h"
+#include "lldb/lldb-private.h"
+#include "llvm/Support/FormatVariadic.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+using namespace lldb_private::repro;
+using namespace lldb;
+
+namespace {
+class StackFrameRecognizerTest : public ::testing::Test {
+public:
+  void SetUp() override {
+    llvm::cantFail(Reproducer::Initialize(ReproducerMode::Off, llvm::None));
+    FileSystem::Initialize();
+    HostInfo::Initialize();
+
+    // Pretend Linux is the host platform.
+    platform_linux::PlatformLinux::Initialize();
+    ArchSpec arch("powerpc64-pc-linux");
+    Platform::SetHostPlatform(
+        platform_linux::PlatformLinux::CreateInstance(true, &arch));
+  }
+
+  void TearDown() override {
+    platform_linux::PlatformLinux::Terminate();
+    HostInfo::Terminate();
+    FileSystem::Terminate();
+    Reproducer::Terminate();
+  }
+};
+
+class DummyStackFrameRecognizer : public StackFrameRecognizer {
+public:
+  std::string GetName() override { return "Dummy StackFrame Recognizer"; }
+};
+
+void RegisterDummyStackFrameRecognizer() {
+  static llvm::once_flag g_once_flag;
+
+  llvm::call_once(g_once_flag, []() {
+    RegularExpressionSP module_regex_sp = nullptr;
+    RegularExpressionSP symbol_regex_sp(new RegularExpression("boom"));
+
+    StackFrameRecognizerSP dummy_recognizer_sp(new DummyStackFrameRecognizer());
+
+    StackFrameRecognizerManager::AddRecognizer(
+        dummy_recognizer_sp, module_regex_sp, symbol_regex_sp, false);
+  });
+}
+
+} // namespace
+
+TEST_F(StackFrameRecognizerTest, NullModuleRegex) {
+  DebuggerSP debugger_sp = Debugger::CreateInstance();
+  ASSERT_TRUE(debugger_sp);
+
+  RegisterDummyStackFrameRecognizer();
+
+  bool any_printed = false;
+  StackFrameRecognizerManager::ForEach(
+      [&any_printed](uint32_t recognizer_id, std::string name,
+                     std::string function, std::string symbol,
+                     bool regexp) { any_printed = true; });
+
+  EXPECT_TRUE(any_printed);
+}