-
Notifications
You must be signed in to change notification settings - Fork 15.7k
[lldb] Eliminate SupportFileSP nullptr derefs #168624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
daea9d9
98a3330
ff3e98e
4964eac
8b1c084
76d08e5
b0230d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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>()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this compile for types that cannot be default constructed? Also, I think here you can just make a helper function that asserts and returns. |
||
| // Can't assert on t as it's been moved-from. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can't assert on t, but you can assert that you're non-null, right?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but that could only happen if
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given the LineTableTest path, which would silently create an empty object, should we instead unconditionally move t in Not sure if this snippet works |
||
| } | ||
|
|
||
| NonNullSharedPtr(const NonNullSharedPtr &other) : Base(other) {} | ||
|
|
||
| NonNullSharedPtr(NonNullSharedPtr &&other) : Base(std::move(other)) {} | ||
|
|
||
| NonNullSharedPtr &operator=(const NonNullSharedPtr &other) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this different from |
||
| 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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you want to support calling this class with nullptrs? Based on my "will this compile" question below, I suspect we really don't want
tto ever be null here.