Model moved-from nonnull pointer fields as nullable at destructor entry - #1525
Merged
Conversation
copybara-service
Bot
force-pushed
the
test_953338635
branch
7 times, most recently
from
July 25, 2026 10:14
e509eaa to
8484baa
Compare
The destructor flow analysis assumed `_Nonnull` pointer fields are nonnull at any method entry. That is unsound for destructors: moving from an object can leave its `_Nonnull` smart-pointer members null (and an `&&`-qualified method may null out raw-pointer members too). The destructor then runs on an object whose fields violate their declared nonnull invariant, so the analysis would fail to flag unsafe dereferences of those fields. This CL detects when the class can be moved from and models all of its non-const, `_Nonnull` pointer fields as nullable at destructor entry. Class `T` is considered movable-from if it has any of: * implicit move operation * a non-const `&&`-qualified method, * any non-deleted method with a `T&&` parameter type (T&&-consuming free functions or friend methods are not detected.) Implementation: - The diagnosis driver computes the set of `_Nonnull` non-const pointer fields to treat as nullable and installs it on the dtor analysis - When a field in this set is read via `*this`, its nullability is downgraded from nonnull to nullable. The field then behaves like an ordinary `_Nullable` pointer. - To avoid confusing diagnostics, dereferencing such a field also emits an explanatory note pointing at the field declaration. Tests: added multiple test cases for raw & smart pointers, across multiple types of move operations. PiperOrigin-RevId: 959704539
copybara-service
Bot
force-pushed
the
test_953338635
branch
from
August 5, 2026 16:31
8484baa to
a7bda19
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Model moved-from nonnull pointer fields as nullable at destructor entry
The destructor flow analysis assumed
_Nonnullpointer fields are stillnonnull at destructor entry. That is unsound: running a move operation (move
constructor / move assignment) or an
&&-qualified method on an object canleave its
_Nonnullsmart-pointer members null (and an&&-qualified methodmay null out raw-pointer members too). The destructor then runs on an object
whose fields violate their declared nonnull invariant, so the analysis would
fail to flag unsafe dereferences of those fields.
This CL detects when the destructor's class can be moved from and models all of
its
_Nonnullpointer fields (raw and smart, uniformly) as nullable atdestructor entry. A class is considered movable-from if it needs an implicit
move operation, has a non-const
&&-qualified method (which consumes*this),or has any non-deleted method with a parameter that is an rvalue reference to
the same class. That last case covers move constructors and move assignment
operators as well as other "consuming" methods such as
void Cord::Append(Cord&& src), which may move out another object's members.(This remains a class-local heuristic: consuming free functions or friends
taking such an rvalue reference are not detected.)
Implementation:
_Nonnullraw/smart pointer fields to treat as nullable
(
computeFieldsToTreatAsNullableAtDestructorEntry) and installs it on theanalysis.
*this, its nullability is downgradedfrom nonnull to nullable:
transferMemberExprlowers its type nullabilityand
initPointerFromTypeNullabilityinitializes its runtime null state asnullable. The field then behaves like an ordinary
_Nullablepointer, so anunchecked dereference is flagged as unsafe while a null check narrows it back
to nonnull -- permitting defensive cleanup in the destructor body. Both raw
and smart pointer fields are handled by the same code path.
explanatory note pointing at the field declaration:
note: a Nonnull pointer field may be null at destructor entry (possibly
because it was moved from); null-check it before dereferencing, or make the
type non-movable if it should never be moved from
The note reuses the
shouldTreatFieldAsNullableAtDestructorEntrypredicate,so it fires exactly when the field was downgraded, and is emitted from the
shared
diagnoseNonnullExpectedpath (covering raw dereference, smart-pointerdereference, subscript, and arrow access).
The
&&-qualified predicate is factored so the destructor entry downgrade andthe existing exit-check share one definition.
Tests: added destructor-entry tests for smart-pointer fields
(smart_pointers_diagnosis.cc) and raw-pointer fields (fields.cc), covering the
move-constructor, move-assignment,
&&-method, consuming-RD&&-method,not-movable, deleted-move-operators, const-lvalue-ref-parameter, unannotated
(unknown) field, and null-check cases; and a cymbal clang-tidy runtime test
(runtime_pointer_nullability.cctest) verifying that both the warning and the
explanatory note are emitted for raw and smart pointer fields in a movable
class's destructor.