Skip to content

Commit 03bec58

Browse files
authored
Merge pull request llvm#11879 from swiftlang/jdevlieghere/radar/164989579
[lldb] Eliminate SupportFileSP nullptr derefs (llvm#168624)
2 parents 3a3d7e2 + 62bb648 commit 03bec58

24 files changed

+195
-111
lines changed

lldb/include/lldb/Core/SourceManager.h

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "lldb/Utility/Checksum.h"
1313
#include "lldb/Utility/FileSpec.h"
14+
#include "lldb/Utility/SupportFile.h"
1415
#include "lldb/lldb-defines.h"
1516
#include "lldb/lldb-forward.h"
1617

@@ -38,8 +39,8 @@ class SourceManager {
3839
const SourceManager::File &rhs);
3940

4041
public:
41-
File(lldb::SupportFileSP support_file_sp, lldb::TargetSP target_sp);
42-
File(lldb::SupportFileSP support_file_sp, lldb::DebuggerSP debugger_sp);
42+
File(SupportFileNSP support_file_nsp, lldb::TargetSP target_sp);
43+
File(SupportFileNSP support_file_nsp, lldb::DebuggerSP debugger_sp);
4344

4445
bool ModificationTimeIsStale() const;
4546
bool PathRemappingIsStale() const;
@@ -57,9 +58,9 @@ class SourceManager {
5758

5859
bool LineIsValid(uint32_t line);
5960

60-
lldb::SupportFileSP GetSupportFile() const {
61-
assert(m_support_file_sp && "SupportFileSP must always be valid");
62-
return m_support_file_sp;
61+
SupportFileNSP GetSupportFile() const {
62+
assert(m_support_file_nsp && "SupportFileNSP must always be valid");
63+
return m_support_file_nsp;
6364
}
6465

6566
uint32_t GetSourceMapModificationID() const { return m_source_map_mod_id; }
@@ -80,13 +81,13 @@ class SourceManager {
8081

8182
protected:
8283
/// Set file and update modification time.
83-
void SetSupportFile(lldb::SupportFileSP support_file_sp);
84+
void SetSupportFile(SupportFileNSP support_file_nsp);
8485

8586
bool CalculateLineOffsets(uint32_t line = UINT32_MAX);
8687

8788
/// The support file. If the target has source mappings, this might be
8889
/// different from the original support file passed to the constructor.
89-
lldb::SupportFileSP m_support_file_sp;
90+
SupportFileNSP m_support_file_nsp;
9091

9192
/// Keep track of the on-disk checksum.
9293
Checksum m_checksum;
@@ -107,9 +108,9 @@ class SourceManager {
107108
lldb::TargetWP m_target_wp;
108109

109110
private:
110-
void CommonInitializer(lldb::SupportFileSP support_file_sp,
111+
void CommonInitializer(SupportFileNSP support_file_nsp,
111112
lldb::TargetSP target_sp);
112-
void CommonInitializerImpl(lldb::SupportFileSP support_file_sp,
113+
void CommonInitializerImpl(SupportFileNSP support_file_nsp,
113114
lldb::TargetSP target_sp);
114115
};
115116

@@ -156,13 +157,13 @@ class SourceManager {
156157

157158
~SourceManager();
158159

159-
FileSP GetLastFile() { return GetFile(m_last_support_file_sp); }
160+
FileSP GetLastFile() { return GetFile(m_last_support_file_nsp); }
160161
bool AtLastLine(bool reverse) {
161162
return m_last_line == UINT32_MAX || (reverse && m_last_line == 1);
162163
}
163164

164165
size_t DisplaySourceLinesWithLineNumbers(
165-
lldb::SupportFileSP support_file_sp, uint32_t line, uint32_t column,
166+
SupportFileNSP support_file_nsp, uint32_t line, uint32_t column,
166167
uint32_t context_before, uint32_t context_after,
167168
const char *current_line_cstr, Stream *s,
168169
const SymbolContextList *bp_locs = nullptr);
@@ -176,31 +177,30 @@ class SourceManager {
176177
size_t DisplayMoreWithLineNumbers(Stream *s, uint32_t count, bool reverse,
177178
const SymbolContextList *bp_locs = nullptr);
178179

179-
bool SetDefaultFileAndLine(lldb::SupportFileSP support_file_sp,
180-
uint32_t line);
180+
bool SetDefaultFileAndLine(SupportFileNSP support_file_nsp, uint32_t line);
181181

182182
struct SupportFileAndLine {
183-
lldb::SupportFileSP support_file_sp;
183+
SupportFileNSP support_file_nsp;
184184
uint32_t line;
185-
SupportFileAndLine(lldb::SupportFileSP support_file_sp, uint32_t line)
186-
: support_file_sp(support_file_sp), line(line) {}
185+
SupportFileAndLine(SupportFileNSP support_file_nsp, uint32_t line)
186+
: support_file_nsp(support_file_nsp), line(line) {}
187187
};
188188

189189
std::optional<SupportFileAndLine> GetDefaultFileAndLine();
190190

191191
bool DefaultFileAndLineSet() {
192-
return (GetFile(m_last_support_file_sp).get() != nullptr);
192+
return (GetFile(m_last_support_file_nsp).get() != nullptr);
193193
}
194194

195-
void FindLinesMatchingRegex(lldb::SupportFileSP support_file_sp,
195+
void FindLinesMatchingRegex(SupportFileNSP support_file_nsp,
196196
RegularExpression &regex, uint32_t start_line,
197197
uint32_t end_line,
198198
std::vector<uint32_t> &match_lines);
199199

200-
FileSP GetFile(lldb::SupportFileSP support_file_sp);
200+
FileSP GetFile(SupportFileNSP support_file_nsp);
201201

202202
protected:
203-
lldb::SupportFileSP m_last_support_file_sp;
203+
SupportFileNSP m_last_support_file_nsp;
204204
uint32_t m_last_line;
205205
uint32_t m_last_count;
206206
bool m_default_set;

lldb/include/lldb/Symbol/CompileUnit.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
9393
/// \param[in] user_data
9494
/// User data where the SymbolFile parser can store data.
9595
///
96-
/// \param[in] support_file_sp
96+
/// \param[in] support_file_nsp
9797
/// The file specification for the source file of this compile
9898
/// unit.
9999
///
@@ -118,7 +118,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
118118
/// An rvalue list of already parsed support files.
119119
/// \see lldb::LanguageType
120120
CompileUnit(const lldb::ModuleSP &module_sp, void *user_data,
121-
lldb::SupportFileSP support_file_sp, lldb::user_id_t uid,
121+
SupportFileNSP support_file_nsp, lldb::user_id_t uid,
122122
lldb::LanguageType language, lldb_private::LazyBool is_optimized,
123123
SupportFileList &&support_files = {});
124124

@@ -230,12 +230,12 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
230230

231231
/// Return the primary source spec associated with this compile unit.
232232
const FileSpec &GetPrimaryFile() const {
233-
return m_primary_support_file_sp->GetSpecOnly();
233+
return m_primary_support_file_nsp->GetSpecOnly();
234234
}
235235

236236
/// Return the primary source file associated with this compile unit.
237-
lldb::SupportFileSP GetPrimarySupportFile() const {
238-
return m_primary_support_file_sp;
237+
SupportFileNSP GetPrimarySupportFile() const {
238+
return m_primary_support_file_nsp;
239239
}
240240

241241
/// Get the line table for the compile unit.
@@ -430,7 +430,7 @@ class CompileUnit : public std::enable_shared_from_this<CompileUnit>,
430430
/// compile unit.
431431
std::vector<SourceModule> m_imported_modules;
432432
/// The primary file associated with this compile unit.
433-
lldb::SupportFileSP m_primary_support_file_sp;
433+
SupportFileNSP m_primary_support_file_nsp;
434434
/// Files associated with this compile unit's line table and declarations.
435435
SupportFileList m_support_files;
436436
/// Line table that will get parsed on demand.

lldb/include/lldb/Symbol/Function.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -473,12 +473,12 @@ class Function : public UserID, public SymbolContextScope {
473473
///
474474
/// \param[out] line_no
475475
/// The line number.
476-
void GetStartLineSourceInfo(lldb::SupportFileSP &source_file_sp,
476+
void GetStartLineSourceInfo(SupportFileNSP &source_file_sp,
477477
uint32_t &line_no);
478478

479479
using SourceRange = Range<uint32_t, uint32_t>;
480480
/// Find the file and line number range of the function.
481-
llvm::Expected<std::pair<lldb::SupportFileSP, SourceRange>> GetSourceInfo();
481+
llvm::Expected<std::pair<SupportFileNSP, SourceRange>> GetSourceInfo();
482482

483483
/// Get the outgoing call edges from this function, sorted by their return
484484
/// PC addresses (in increasing order).

lldb/include/lldb/Symbol/LineEntry.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ struct LineEntry {
137137
AddressRange range;
138138

139139
/// The source file, possibly mapped by the target.source-map setting.
140-
lldb::SupportFileSP file_sp;
140+
SupportFileNSP file_sp;
141141

142142
/// The original source file, from debug info.
143-
lldb::SupportFileSP original_file_sp;
143+
SupportFileNSP original_file_sp;
144144

145145
/// The source line number, or LLDB_INVALID_LINE_NUMBER if there is no line
146146
/// number information.

lldb/include/lldb/Utility/FileSpecList.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class SupportFileList {
4141
bool AppendIfUnique(const FileSpec &file);
4242
size_t GetSize() const { return m_files.size(); }
4343
const FileSpec &GetFileSpecAtIndex(size_t idx) const;
44-
lldb::SupportFileSP GetSupportFileAtIndex(size_t idx) const;
44+
SupportFileNSP GetSupportFileAtIndex(size_t idx) const;
4545
size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const;
4646
/// Find a compatible file index.
4747
///
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLDB_UTILITY_NONNULLSHAREDPTR_H
10+
#define LLDB_UTILITY_NONNULLSHAREDPTR_H
11+
12+
#include <memory>
13+
#include <utility>
14+
15+
namespace lldb_private {
16+
17+
/// A non-nullable shared pointer that always holds a valid object.
18+
///
19+
/// NonNullSharedPtr is a smart pointer wrapper around std::shared_ptr that
20+
/// guarantees the pointer is never null.
21+
///
22+
/// This class is used for enforcing invariants at the type level and
23+
/// eliminating entire classes of null pointer bugs.
24+
///
25+
/// @tparam T The type of object to manage. Must be default-constructible.
26+
template <typename T> class NonNullSharedPtr : private std::shared_ptr<T> {
27+
using Base = std::shared_ptr<T>;
28+
29+
public:
30+
NonNullSharedPtr(const std::shared_ptr<T> &t)
31+
: Base(t ? t : std::make_shared<T>()) {
32+
assert(t && "NonNullSharedPtr constructed from nullptr");
33+
}
34+
35+
NonNullSharedPtr(std::shared_ptr<T> &&t) : Base(std::move(t)) {
36+
const auto b = static_cast<bool>(*this);
37+
assert(b && "NonNullSharedPtr constructed from nullptr");
38+
if (!b)
39+
Base::operator=(std::make_shared<T>());
40+
}
41+
42+
NonNullSharedPtr(const NonNullSharedPtr &other) : Base(other) {}
43+
44+
NonNullSharedPtr(NonNullSharedPtr &&other) : Base(std::move(other)) {}
45+
46+
NonNullSharedPtr &operator=(const NonNullSharedPtr &other) {
47+
Base::operator=(other);
48+
return *this;
49+
}
50+
51+
NonNullSharedPtr &operator=(NonNullSharedPtr &&other) {
52+
Base::operator=(std::move(other));
53+
return *this;
54+
}
55+
56+
using Base::operator*;
57+
using Base::operator->;
58+
using Base::get;
59+
using Base::unique;
60+
using Base::use_count;
61+
using Base::operator bool;
62+
63+
void swap(NonNullSharedPtr &other) { Base::swap(other); }
64+
65+
/// Explicitly deleted operations that could introduce nullptr.
66+
/// @{
67+
void reset() = delete;
68+
void reset(T *ptr) = delete;
69+
/// @}
70+
};
71+
72+
} // namespace lldb_private
73+
74+
/// Specialized swap function for NonNullSharedPtr to enable argument-dependent
75+
/// lookup (ADL) and efficient swapping.
76+
template <typename T>
77+
void swap(lldb_private::NonNullSharedPtr<T> &lhs,
78+
lldb_private::NonNullSharedPtr<T> &rhs) {
79+
lhs.swap(rhs);
80+
}
81+
82+
#endif

lldb/include/lldb/Utility/SupportFile.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
#include "lldb/Utility/Checksum.h"
1313
#include "lldb/Utility/FileSpec.h"
14+
#include "lldb/Utility/NonNullSharedPtr.h"
1415

1516
namespace lldb_private {
1617

@@ -76,6 +77,8 @@ class SupportFile {
7677
const Checksum m_checksum;
7778
};
7879

80+
typedef NonNullSharedPtr<lldb_private::SupportFile> SupportFileNSP;
81+
7982
} // namespace lldb_private
8083

8184
#endif // LLDB_UTILITY_SUPPORTFILE_H

lldb/include/lldb/lldb-forward.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,6 @@ typedef std::shared_ptr<lldb_private::TypeSummaryImpl> TypeSummaryImplSP;
487487
typedef std::shared_ptr<lldb_private::TypeSummaryOptions> TypeSummaryOptionsSP;
488488
typedef std::shared_ptr<lldb_private::ScriptedSyntheticChildren>
489489
ScriptedSyntheticChildrenSP;
490-
typedef std::shared_ptr<lldb_private::SupportFile> SupportFileSP;
491490
typedef std::shared_ptr<lldb_private::UnixSignals> UnixSignalsSP;
492491
typedef std::weak_ptr<lldb_private::UnixSignals> UnixSignalsWP;
493492
typedef std::shared_ptr<lldb_private::UnwindAssembly> UnwindAssemblySP;

lldb/source/Breakpoint/BreakpointResolverFileLine.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void BreakpointResolverFileLine::FilterContexts(SymbolContextList &sc_list) {
139139
if (!sc.block)
140140
continue;
141141

142-
SupportFileSP file_sp;
142+
SupportFileNSP file_sp = std::make_shared<SupportFile>();
143143
uint32_t line;
144144
const Block *inline_block = sc.block->GetContainingInlinedBlock();
145145
if (inline_block) {

lldb/source/Commands/CommandObjectBreakpoint.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
800800
// frame's file.
801801
if (auto maybe_file_and_line =
802802
target.GetSourceManager().GetDefaultFileAndLine()) {
803-
file = maybe_file_and_line->support_file_sp->GetSpecOnly();
803+
file = maybe_file_and_line->support_file_nsp->GetSpecOnly();
804804
return true;
805805
}
806806

0 commit comments

Comments
 (0)