[lldb] Ensure SBStructuredData::m_impl_up is always non-null

[kastiglione]

Summary

By ensuring that SBStructuredData::m_impl_up always has a value, it prevents crashes that can happen when the SBStructuredDataImpl is null.

This fixes a case that caused SBStructuredData::operator= to crash. This happened when rhs had a null SBStructuredDataImpl.

[shafik]

LGTM, it seems like there a few places where we are not consistently checking m_impl_up is valid and other places we are using the conditional operator to check if it is set or not.

[kastiglione]

I should expand this to fix other unchecked dereferences.

[kastiglione]

Ensure m_impl_up is always non-null

[kastiglione]

I changed direction, to ensure the impl pointer is always non-null.

lldb/source/API/SBStructuredData.cpp

43

This is the key change, all else is cleanup.

[teemperor]

LGTM. I'm going to be that guy and ask if you could test this (there is an API unittest directory with an example test, so copying that and just exercising the nullptr SBStructuredData is good enough).

[augusto2112]

LGTM!

[kastiglione]

Add unittest

lldb/source/API/SBStructuredData.cpp

 //===-- SBStructuredData.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/API/SBStructuredData.h"
 #include "SBReproducerPrivate.h"
 
 #include "lldb/API/SBStream.h"
 #include "lldb/API/SBStringList.h"
 #include "lldb/Core/StructuredDataImpl.h"
 #include "lldb/Target/StructuredDataPlugin.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/Status.h"

 }
 
 SBStructuredData::SBStructuredData(const lldb::EventSP &event_sp)
     : m_impl_up(new StructuredDataImpl(event_sp)) {
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData, (const lldb::EventSP &), event_sp);
 }
 
 SBStructuredData::SBStructuredData(lldb_private::StructuredDataImpl *impl)
     : m_impl_up(impl) {
   LLDB_RECORD_CONSTRUCTOR(SBStructuredData,
                           (lldb_private::StructuredDataImpl *), impl);
 }
 
 SBStructuredData::~SBStructuredData() = default;
 
 SBStructuredData &SBStructuredData::
 operator=(const lldb::SBStructuredData &rhs) {
   LLDB_RECORD_METHOD(
       lldb::SBStructuredData &,
       SBStructuredData, operator=,(const lldb::SBStructuredData &), rhs);
 
+  if (m_impl_up && rhs.m_impl_up)
     *m_impl_up = *rhs.m_impl_up;
+  else if (m_impl_up && !rhs.m_impl_up)
+    m_impl_up.release();
+  else if (!m_impl_up && rhs.m_impl_up)
+    m_impl_up = std::make_unique<StructuredDataImpl>(*rhs.m_impl_up);
+
   return LLDB_RECORD_RESULT(*this);
 }
 
 lldb::SBError SBStructuredData::SetFromJSON(lldb::SBStream &stream) {
   LLDB_RECORD_METHOD(lldb::SBError, SBStructuredData, SetFromJSON,
                      (lldb::SBStream &), stream);
 
   lldb::SBError error;