[Lldb-commits] [lldb] c5cf4b8 - [lldb] Handle missing SBStructuredData copy assignment cases

[Dave Lee via lldb-commits]

Author: Dave Lee
Date: 2021-05-05T15:12:03-07:00
New Revision: c5cf4b8f11cd641560b0cd6e106765721688e74a

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

LOG: [lldb] Handle missing SBStructuredData copy assignment cases

Fix cases that can crash `SBStructuredData::operator=`.

This happened in a case where `rhs` had a null `SBStructuredDataImpl`.

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

Added: 
    lldb/unittests/API/SBStructuredDataTest.cpp

Modified: 
    lldb/source/API/SBStructuredData.cpp
    lldb/unittests/API/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/source/API/SBStructuredData.cpp b/lldb/source/API/SBStructuredData.cpp
index 2ae3005fd8d11..9a37749a64d00 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -29,7 +29,7 @@ SBStructuredData::SBStructuredData() : m_impl_up(new StructuredDataImpl()) {
 }
 
 SBStructuredData::SBStructuredData(const lldb::SBStructuredData &rhs)
-    : m_impl_up(new StructuredDataImpl(*rhs.m_impl_up.get())) {
+    : m_impl_up(new StructuredDataImpl(*rhs.m_impl_up)) {
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData, (const lldb::SBStructuredData &),
                           rhs);
 }
@@ -40,7 +40,7 @@ SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
 }
 
 SBStructuredData::SBStructuredData(lldb_private::StructuredDataImpl *impl)
-    : m_impl_up(impl) {
+    : m_impl_up(impl ? impl : new StructuredDataImpl()) {
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData,
                           (lldb_private::StructuredDataImpl *), impl);
 }
@@ -111,22 +111,19 @@ StructuredDataType SBStructuredData::GetType() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(lldb::StructuredDataType, SBStructuredData,
                                    GetType);
 
-  return (m_impl_up ? m_impl_up->GetType() : eStructuredDataTypeInvalid);
+  return m_impl_up->GetType();
 }
 
 size_t SBStructuredData::GetSize() const {
   LLDB_RECORD_METHOD_CONST_NO_ARGS(size_t, SBStructuredData, GetSize);
 
-  return (m_impl_up ? m_impl_up->GetSize() : 0);
+  return m_impl_up->GetSize();
 }
 
 bool SBStructuredData::GetKeys(lldb::SBStringList &keys) const {
   LLDB_RECORD_METHOD_CONST(bool, SBStructuredData, GetKeys,
                            (lldb::SBStringList &), keys);
 
-  if (!m_impl_up)
-    return false;
-
   if (GetType() != eStructuredDataTypeDictionary)
     return false;
 
@@ -154,9 +151,6 @@ lldb::SBStructuredData SBStructuredData::GetValueForKey(const char *key) const {
   LLDB_RECORD_METHOD_CONST(lldb::SBStructuredData, SBStructuredData,
                            GetValueForKey, (const char *), key);
 
-  if (!m_impl_up)
-    return LLDB_RECORD_RESULT(SBStructuredData());
-
   SBStructuredData result;
   result.m_impl_up->SetObjectSP(m_impl_up->GetValueForKey(key));
   return LLDB_RECORD_RESULT(result);
@@ -166,9 +160,6 @@ lldb::SBStructuredData SBStructuredData::GetItemAtIndex(size_t idx) const {
   LLDB_RECORD_METHOD_CONST(lldb::SBStructuredData, SBStructuredData,
                            GetItemAtIndex, (size_t), idx);
 
-  if (!m_impl_up)
-    return LLDB_RECORD_RESULT(SBStructuredData());
-
   SBStructuredData result;
   result.m_impl_up->SetObjectSP(m_impl_up->GetItemAtIndex(idx));
   return LLDB_RECORD_RESULT(result);
@@ -178,28 +169,28 @@ uint64_t SBStructuredData::GetIntegerValue(uint64_t fail_value) const {
   LLDB_RECORD_METHOD_CONST(uint64_t, SBStructuredData, GetIntegerValue,
                            (uint64_t), fail_value);
 
-  return (m_impl_up ? m_impl_up->GetIntegerValue(fail_value) : fail_value);
+  return m_impl_up->GetIntegerValue(fail_value);
 }
 
 double SBStructuredData::GetFloatValue(double fail_value) const {
   LLDB_RECORD_METHOD_CONST(double, SBStructuredData, GetFloatValue, (double),
                            fail_value);
 
-  return (m_impl_up ? m_impl_up->GetFloatValue(fail_value) : fail_value);
+  return m_impl_up->GetFloatValue(fail_value);
 }
 
 bool SBStructuredData::GetBooleanValue(bool fail_value) const {
   LLDB_RECORD_METHOD_CONST(bool, SBStructuredData, GetBooleanValue, (bool),
                            fail_value);
 
-  return (m_impl_up ? m_impl_up->GetBooleanValue(fail_value) : fail_value);
+  return m_impl_up->GetBooleanValue(fail_value);
 }
 
 size_t SBStructuredData::GetStringValue(char *dst, size_t dst_len) const {
   LLDB_RECORD_CHAR_PTR_METHOD_CONST(size_t, SBStructuredData, GetStringValue,
                                     (char *, size_t), dst, "", dst_len);
 
-  return (m_impl_up ? m_impl_up->GetStringValue(dst, dst_len) : 0);
+  return m_impl_up->GetStringValue(dst, dst_len);
 }
 
 namespace lldb_private {

diff  --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt
index 2f066f26d8aaf..7bdc1e3e87cc1 100644
--- a/lldb/unittests/API/CMakeLists.txt
+++ b/lldb/unittests/API/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(APITests
   SBCommandInterpreterTest.cpp
+  SBStructuredDataTest.cpp
 
   LINK_LIBS
     liblldb

diff  --git a/lldb/unittests/API/SBStructuredDataTest.cpp b/lldb/unittests/API/SBStructuredDataTest.cpp
new file mode 100644
index 0000000000000..a3a33aad9a1ae
--- /dev/null
+++ b/lldb/unittests/API/SBStructuredDataTest.cpp
@@ -0,0 +1,35 @@
+//===-- SBStructuredDataTest.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 "gtest/gtest.h"
+
+#include "lldb/API/SBStringList.h"
+#include "lldb/API/SBStructuredData.h"
+
+#include <cstring>
+#include <string>
+
+using namespace lldb;
+
+class SBStructuredDataTest : public testing::Test {};
+
+TEST_F(SBStructuredDataTest, NullImpl) {
+  SBStructuredData data(nullptr);
+  EXPECT_EQ(data.GetType(), eStructuredDataTypeInvalid);
+  EXPECT_EQ(data.GetSize(), 0ul);
+  SBStringList keys;
+  EXPECT_FALSE(data.GetKeys(keys));
+  EXPECT_EQ(data.GetValueForKey("key").GetType(), eStructuredDataTypeInvalid);
+  EXPECT_EQ(data.GetItemAtIndex(0).GetType(), eStructuredDataTypeInvalid);
+  EXPECT_EQ(data.GetIntegerValue(UINT64_MAX), UINT64_MAX);
+  EXPECT_EQ(data.GetFloatValue(DBL_MAX), DBL_MAX);
+  EXPECT_TRUE(data.GetBooleanValue(true));
+  EXPECT_FALSE(data.GetBooleanValue(false));
+  char dst[1];
+  EXPECT_EQ(data.GetStringValue(dst, sizeof(dst)), 0ul);
+}