[lldb] Eliminate SupportFileSP nullptr derefs (#168624)

This patch fixes and eliminates the possibility of SupportFileSP ever
being nullptr. The support file was originally treated like a value
type, but became a polymorphic type and therefore has to be stored and
passed around as a pointer.

To avoid having all the callers check the validity of the pointer, I
introduced the invariant that SupportFileSP is never null and always
default constructed. However, without enforcement at the type level,
that's fragile and indeed, we already identified two crashes where
someone accidentally broke that invariant.

This PR introduces a NonNullSharedPtr to prevent that. NonNullSharedPtr
is a smart pointer wrapper around std::shared_ptr that guarantees the
pointer is never null. If default-constructed, it creates a
default-constructed instance of the contained type. Note that I'm using
private inheritance because you shouldn't inherit from standard library
classes due to the lack of virtual destructor. So while the new
abstraction looks like a `std::shared_ptr`, it is in fact **not** a
shared pointer. Given that our destructor is trivial, we could use
public inheritance, but currently there's no need for it.

rdar://164989579

Author: JDevlieghere

lldb/include/lldb/Core/SourceManager.h

@@ -11,6 +11,7 @@
 
 #include "lldb/Utility/Checksum.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/SupportFile.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-forward.h"
 
@@ -38,8 +39,8 @@ class SourceManager {
                            const SourceManager::File &rhs);
 
   public:
-    File(lldb::SupportFileSP support_file_sp, lldb::TargetSP target_sp);
-    File(lldb::SupportFileSP support_file_sp, lldb::DebuggerSP debugger_sp);
+    File(SupportFileNSP support_file_nsp, lldb::TargetSP target_sp);
+    File(SupportFileNSP support_file_nsp, lldb::DebuggerSP debugger_sp);
 
     bool ModificationTimeIsStale() const;
     bool PathRemappingIsStale() const;
@@ -57,9 +58,9 @@ class SourceManager {
 
     bool LineIsValid(uint32_t line);
 
-    lldb::SupportFileSP GetSupportFile() const {
-      assert(m_support_file_sp && "SupportFileSP must always be valid");
-      return m_support_file_sp;
+    SupportFileNSP GetSupportFile() const {
+      assert(m_support_file_nsp && "SupportFileNSP must always be valid");
+      return m_support_file_nsp;
     }
 
     uint32_t GetSourceMapModificationID() const { return m_source_map_mod_id; }
@@ -80,13 +81,13 @@ class SourceManager {
 
   protected:
     /// Set file and update modification time.
-    void SetSupportFile(lldb::SupportFileSP support_file_sp);
+    void SetSupportFile(SupportFileNSP support_file_nsp);
 
     bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
 
     /// The support file. If the target has source mappings, this might be
     /// different from the original support file passed to the constructor.
-    lldb::SupportFileSP m_support_file_sp;
+    SupportFileNSP m_support_file_nsp;
 
     /// Keep track of the on-disk checksum.
     Checksum m_checksum;
@@ -107,9 +108,9 @@ class SourceManager {
     lldb::TargetWP m_target_wp;
 
   private:
-    void CommonInitializer(lldb::SupportFileSP support_file_sp,
+    void CommonInitializer(SupportFileNSP support_file_nsp,
                            lldb::TargetSP target_sp);
-    void CommonInitializerImpl(lldb::SupportFileSP support_file_sp,
+    void CommonInitializerImpl(SupportFileNSP support_file_nsp,
                                lldb::TargetSP target_sp);
   };
 
@@ -156,13 +157,13 @@ class SourceManager {
 
   ~SourceManager();
 
-  FileSP GetLastFile() { return GetFile(m_last_support_file_sp); }
+  FileSP GetLastFile() { return GetFile(m_last_support_file_nsp); }
   bool AtLastLine(bool reverse) {
     return m_last_line == UINT32_MAX || (reverse && m_last_line == 1);
   }
 
   size_t DisplaySourceLinesWithLineNumbers(
-      lldb::SupportFileSP support_file_sp, uint32_t line, uint32_t column,
+      SupportFileNSP support_file_nsp, uint32_t line, uint32_t column,
       uint32_t context_before, uint32_t context_after,
       const char *current_line_cstr, Stream *s,
       const SymbolContextList *bp_locs = nullptr);
@@ -176,31 +177,30 @@ class SourceManager {
   size_t DisplayMoreWithLineNumbers(Stream *s, uint32_t count, bool reverse,
                                     const SymbolContextList *bp_locs = nullptr);
 
-  bool SetDefaultFileAndLine(lldb::SupportFileSP support_file_sp,
-                             uint32_t line);
+  bool SetDefaultFileAndLine(SupportFileNSP support_file_nsp, uint32_t line);
 
   struct SupportFileAndLine {
-    lldb::SupportFileSP support_file_sp;
+    SupportFileNSP support_file_nsp;
     uint32_t line;
-    SupportFileAndLine(lldb::SupportFileSP support_file_sp, uint32_t line)
-        : support_file_sp(support_file_sp), line(line) {}
+    SupportFileAndLine(SupportFileNSP support_file_nsp, uint32_t line)
+        : support_file_nsp(support_file_nsp), line(line) {}
   };
 
   std::optional<SupportFileAndLine> GetDefaultFileAndLine();
 
   bool DefaultFileAndLineSet() {
-    return (GetFile(m_last_support_file_sp).get() != nullptr);
+    return (GetFile(m_last_support_file_nsp).get() != nullptr);
   }
 
-  void FindLinesMatchingRegex(lldb::SupportFileSP support_file_sp,
+  void FindLinesMatchingRegex(SupportFileNSP support_file_nsp,
                               RegularExpression &regex, uint32_t start_line,
                               uint32_t end_line,
                               std::vector<uint32_t> &match_lines);
 
-  FileSP GetFile(lldb::SupportFileSP support_file_sp);
+  FileSP GetFile(SupportFileNSP support_file_nsp);
 
 protected:
-  lldb::SupportFileSP m_last_support_file_sp;
+  SupportFileNSP m_last_support_file_nsp;
   uint32_t m_last_line;
   uint32_t m_last_count;
   bool m_default_set;

lldb/include/lldb/Symbol/CompileUnit.h

@@ -93,7 +93,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
   /// \param[in] user_data
   ///     User data where the SymbolFile parser can store data.
   ///
-  /// \param[in] support_file_sp
+  /// \param[in] support_file_nsp
   ///     The file specification for the source file of this compile
   ///     unit.
   ///
@@ -118,7 +118,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
   ///     An rvalue list of already parsed support files.
   /// \see lldb::LanguageType
   CompileUnit(const lldb::ModuleSP &module_sp, void *user_data,
-              lldb::SupportFileSP support_file_sp, lldb::user_id_t uid,
+              SupportFileNSP support_file_nsp, lldb::user_id_t uid,
               lldb::LanguageType language, lldb_private::LazyBool is_optimized,
               SupportFileList &&support_files = {});
 
@@ -230,12 +230,12 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
 
   /// Return the primary source spec associated with this compile unit.
   const FileSpec &GetPrimaryFile() const {
-    return m_primary_support_file_sp->GetSpecOnly();
+    return m_primary_support_file_nsp->GetSpecOnly();
   }
 
   /// Return the primary source file associated with this compile unit.
-  lldb::SupportFileSP GetPrimarySupportFile() const {
-    return m_primary_support_file_sp;
+  SupportFileNSP GetPrimarySupportFile() const {
+    return m_primary_support_file_nsp;
   }
 
   /// Get the line table for the compile unit.
@@ -430,7 +430,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
   /// compile unit.
   std::vector<SourceModule> m_imported_modules;
   /// The primary file associated with this compile unit.
-  lldb::SupportFileSP m_primary_support_file_sp;
+  SupportFileNSP m_primary_support_file_nsp;
   /// Files associated with this compile unit's line table and declarations.
   SupportFileList m_support_files;
   /// Line table that will get parsed on demand.

lldb/include/lldb/Symbol/Function.h

@@ -469,12 +469,12 @@ class Function : public UserID, public SymbolContextScope {
   ///
   /// \param[out] line_no
   ///     The line number.
-  void GetStartLineSourceInfo(lldb::SupportFileSP &source_file_sp,
+  void GetStartLineSourceInfo(SupportFileNSP &source_file_sp,
                               uint32_t &line_no);
 
   using SourceRange = Range<uint32_t, uint32_t>;
   /// Find the file and line number range of the function.
-  llvm::Expected<std::pair<lldb::SupportFileSP, SourceRange>> GetSourceInfo();
+  llvm::Expected<std::pair<SupportFileNSP, SourceRange>> GetSourceInfo();
 
   /// Get the outgoing call edges from this function, sorted by their return
   /// PC addresses (in increasing order).

lldb/include/lldb/Symbol/LineEntry.h

@@ -137,10 +137,10 @@ struct LineEntry {
   AddressRange range;
 
   /// The source file, possibly mapped by the target.source-map setting.
-  lldb::SupportFileSP file_sp;
+  SupportFileNSP file_sp;
 
   /// The original source file, from debug info.
-  lldb::SupportFileSP original_file_sp;
+  SupportFileNSP original_file_sp;
 
   /// The source line number, or LLDB_INVALID_LINE_NUMBER if there is no line
   /// number information.

lldb/include/lldb/Utility/FileSpecList.h

@@ -41,7 +41,7 @@ class SupportFileList {
   bool AppendIfUnique(const FileSpec &file);
   size_t GetSize() const { return m_files.size(); }
   const FileSpec &GetFileSpecAtIndex(size_t idx) const;
-  lldb::SupportFileSP GetSupportFileAtIndex(size_t idx) const;
+  SupportFileNSP GetSupportFileAtIndex(size_t idx) const;
   size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const;
   /// Find a compatible file index.
   ///

lldb/include/lldb/Utility/NonNullSharedPtr.h

@@ -0,0 +1,80 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_NONNULLSHAREDPTR_H
+#define LLDB_UTILITY_NONNULLSHAREDPTR_H
+
+#include <memory>
+#include <utility>
+
+namespace lldb_private {
+
+/// A non-nullable shared pointer that always holds a valid object.
+///
+/// NonNullSharedPtr is a smart pointer wrapper around std::shared_ptr that
+/// guarantees the pointer is never null.
+///
+/// This class is used for enforcing invariants at the type level and
+/// eliminating entire classes of null pointer bugs.
+///
+/// @tparam T The type of object to manage. Must be default-constructible.
+template <typename T> class NonNullSharedPtr : private std::shared_ptr<T> {
+  using Base = std::shared_ptr<T>;
+
+public:
+  NonNullSharedPtr(const std::shared_ptr<T> &t)
+      : Base(t ? t : std::make_shared<T>()) {
+    assert(t && "NonNullSharedPtr initialized from NULL shared_ptr");
+  }
+
+  NonNullSharedPtr(std::shared_ptr<T> &&t)
+      : Base(t ? std::move(t) : std::make_shared<T>()) {
+    // Can't assert on t as it's been moved-from.
+  }
+
+  NonNullSharedPtr(const NonNullSharedPtr &other) : Base(other) {}
+
+  NonNullSharedPtr(NonNullSharedPtr &&other) : Base(std::move(other)) {}
+
+  NonNullSharedPtr &operator=(const NonNullSharedPtr &other) {
+    Base::operator=(other);
+    return *this;
+  }
+
+  NonNullSharedPtr &operator=(NonNullSharedPtr &&other) {
+    Base::operator=(std::move(other));
+    return *this;
+  }
+
+  using Base::operator*;
+  using Base::operator->;
+  using Base::get;
+  using Base::unique;
+  using Base::use_count;
+  using Base::operator bool;
+
+  void swap(NonNullSharedPtr &other) { Base::swap(other); }
+
+  /// Explicitly deleted operations that could introduce nullptr.
+  /// @{
+  void reset() = delete;
+  void reset(T *ptr) = delete;
+  /// @}
+};
+
+} // namespace lldb_private
+
+/// Specialized swap function for NonNullSharedPtr to enable argument-dependent
+/// lookup (ADL) and efficient swapping.
+template <typename T>
+void swap(lldb_private::NonNullSharedPtr<T> &lhs,
+          lldb_private::NonNullSharedPtr<T> &rhs) {
+  lhs.swap(rhs);
+}
+
+#endif

lldb/include/lldb/Utility/SupportFile.h

@@ -11,6 +11,7 @@
 
 #include "lldb/Utility/Checksum.h"
 #include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/NonNullSharedPtr.h"
 
 namespace lldb_private {
 
@@ -76,6 +77,8 @@ class SupportFile {
   const Checksum m_checksum;
 };
 
+typedef NonNullSharedPtr<lldb_private::SupportFile> SupportFileNSP;
+
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_SUPPORTFILE_H

lldb/include/lldb/lldb-forward.h

@@ -493,7 +493,6 @@ typedef std::shared_ptr<lldb_private::TypeSummaryImpl> TypeSummaryImplSP;
 typedef std::shared_ptr<lldb_private::TypeSummaryOptions> TypeSummaryOptionsSP;
 typedef std::shared_ptr<lldb_private::ScriptedSyntheticChildren>
     ScriptedSyntheticChildrenSP;
-typedef std::shared_ptr<lldb_private::SupportFile> SupportFileSP;
 typedef std::shared_ptr<lldb_private::UnixSignals> UnixSignalsSP;
 typedef std::weak_ptr<lldb_private::UnixSignals> UnixSignalsWP;
 typedef std::shared_ptr<lldb_private::UnwindAssembly> UnwindAssemblySP;

lldb/source/Breakpoint/BreakpointResolverFileLine.cpp

@@ -139,7 +139,7 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
     if (!sc.block)
       continue;
 
-    SupportFileSP file_sp;
+    SupportFileNSP file_sp = std::make_shared<SupportFile>();
     uint32_t line;
     const Block *inline_block = sc.block->GetContainingInlinedBlock();
     if (inline_block) {

lldb/source/Commands/CommandObjectBreakpoint.cpp

@@ -795,7 +795,7 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
     // frame's file.
     if (auto maybe_file_and_line =
             target.GetSourceManager().GetDefaultFileAndLine()) {
-      file = maybe_file_and_line->support_file_sp->GetSpecOnly();
+      file = maybe_file_and_line->support_file_nsp->GetSpecOnly();
       return true;
     }