Skip to content

Model moved-from nonnull pointer fields as nullable at destructor entry - #1525

Merged
copybara-service[bot] merged 1 commit into
mainfrom
test_953338635
Aug 5, 2026
Merged

Model moved-from nonnull pointer fields as nullable at destructor entry#1525
copybara-service[bot] merged 1 commit into
mainfrom
test_953338635

Conversation

@copybara-service

@copybara-service copybara-service Bot commented Jul 24, 2026

Copy link
Copy Markdown

Model moved-from nonnull pointer fields as nullable at destructor entry

The destructor flow analysis assumed _Nonnull pointer fields are still
nonnull at destructor entry. That is unsound: running a move operation (move
constructor / move assignment) or an &&-qualified method on 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 destructor's class can be moved from and models all of
its _Nonnull pointer fields (raw and smart, uniformly) as nullable at
destructor 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:

  • The diagnosis driver computes, for such a destructor, the set of _Nonnull
    raw/smart pointer fields to treat as nullable
    (computeFieldsToTreatAsNullableAtDestructorEntry) and installs it on the
    analysis.
  • The set is threaded through the lattice's non-flow-sensitive state.
  • When a field in this set is read via *this, its nullability is downgraded
    from nonnull to nullable: transferMemberExpr lowers its type nullability
    and initPointerFromTypeNullability initializes its runtime null state as
    nullable. The field then behaves like an ordinary _Nullable pointer, so an
    unchecked 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.
  • To avoid confusing diagnostics, dereferencing such a field also emits an
    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 shouldTreatFieldAsNullableAtDestructorEntry predicate,
    so it fires exactly when the field was downgraded, and is emitted from the
    shared diagnoseNonnullExpected path (covering raw dereference, smart-pointer
    dereference, subscript, and arrow access).

The &&-qualified predicate is factored so the destructor entry downgrade and
the 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.

@copybara-service
copybara-service Bot force-pushed the test_953338635 branch 7 times, most recently from e509eaa to 8484baa Compare July 25, 2026 10:14
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
copybara-service Bot merged commit a7bda19 into main Aug 5, 2026
@copybara-service
copybara-service Bot deleted the test_953338635 branch August 5, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant