Skip to content

Commit c62b866

Browse files
cs01cs01
committed
warn on null assigned to _Nonnull members, locals, and deref after
1 parent 4ca70da commit c62b866

5 files changed

Lines changed: 136 additions & 19 deletions

File tree

‎clang/include/clang/Analysis/Analyses/FlowNullability.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class FlowNullabilityHandler {
3838
QualType ReturnType) {}
3939
virtual void handleNullableAssignment(const Expr *AssignExpr,
4040
const VarDecl *LHSVar) {}
41+
virtual void handleNullableMemberAssignment(const Expr *AssignExpr,
42+
const FieldDecl *Member) {}
4143
virtual void handleNullableArgument(const Expr *ArgExpr,
4244
const ParmVarDecl *Param) {}
4345

‎clang/include/clang/Basic/DiagnosticSemaKinds.td‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13026,6 +13026,11 @@ def warn_flow_nullable_field_init : Warning<
1302613026
InGroup<FlowNullableAssignment>;
1302713027
def note_nullable_field_init_fix : Note<
1302813028
"remove '_Nonnull' if this member can be null, or remove the null initializer">;
13029+
def warn_flow_nullable_member_assignment : Warning<
13030+
"assigning nullable pointer to nonnull member %0">,
13031+
InGroup<FlowNullableAssignment>;
13032+
def note_nullable_member_assignment_fix : Note<
13033+
"add a null check before assigning, or remove '_Nonnull' from the member">;
1302913034
def note_nullable_assignment_fix : Note<
1303013035
"add a null check before assigning, or change the variable type to "
1303113036
"'_Nullable'">;

‎clang/lib/Analysis/FlowNullability.cpp‎

Lines changed: 45 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,21 +1208,32 @@ class TransferFunctions {
12081208
}
12091209
}
12101210
}
1211-
if (!Narrowed && isNonnullInit(RHS))
1212-
Narrowed = true;
1213-
if (!Narrowed && isNonnullType(BO->getRHS()->getType()))
1214-
Narrowed = true;
1215-
1216-
if (Narrowed) {
1217-
if (IsThisMember)
1218-
State.NarrowedThisMembers.insert(FD);
1219-
else if (BaseVD)
1220-
State.NarrowedMembers.insert({BaseVD, FD});
1221-
} else if (isNullableType(BO->getRHS()->getType(), StrictMode,
1222-
DefaultNullability) ||
1223-
isNullableInit(RHS)) {
1211+
// Null constant assigned to _Nonnull member — warn immediately.
1212+
// Check before isNonnullInit/isNonnullType because implicit
1213+
// casts can propagate _Nonnull from the LHS onto the RHS type.
1214+
if (!Narrowed && isNonnullType(FD->getType()) &&
1215+
isNullableInit(RHS) && !isNonnullInit(RHS)) {
12241216
if (IsThisMember)
12251217
State.NullableThisMembers.insert(FD);
1218+
++NumAssignmentWarnings;
1219+
Handler.handleNullableMemberAssignment(BO, FD);
1220+
} else {
1221+
if (!Narrowed && isNonnullInit(RHS))
1222+
Narrowed = true;
1223+
if (!Narrowed && isNonnullType(BO->getRHS()->getType()))
1224+
Narrowed = true;
1225+
1226+
if (Narrowed) {
1227+
if (IsThisMember)
1228+
State.NarrowedThisMembers.insert(FD);
1229+
else if (BaseVD)
1230+
State.NarrowedMembers.insert({BaseVD, FD});
1231+
} else if (isNullableType(BO->getRHS()->getType(), StrictMode,
1232+
DefaultNullability) ||
1233+
isNullableInit(RHS)) {
1234+
if (IsThisMember)
1235+
State.NullableThisMembers.insert(FD);
1236+
}
12261237
}
12271238

12281239
// Emit evidence for cross-TU inference.
@@ -1284,18 +1295,23 @@ class TransferFunctions {
12841295
}
12851296
}
12861297
}
1287-
if (isNonnullInit(RHS)) {
1298+
// Null constant assigned to _Nonnull — warn immediately.
1299+
// Check before isNonnullInit/isNonnullType because implicit
1300+
// casts can propagate _Nonnull from the LHS onto the RHS type.
1301+
if (isNonnullType(VD->getType()) && isNullableInit(RHS) &&
1302+
!isNonnullInit(RHS)) {
1303+
State.NullableVars.insert(VD);
1304+
++NumAssignmentWarnings;
1305+
Handler.handleNullableAssignment(BO, VD);
1306+
} else if (isNonnullInit(RHS)) {
12881307
State.NarrowedVars.insert(VD);
12891308
return;
1290-
}
1291-
if (isNonnullType(BO->getRHS()->getType())) {
1309+
} else if (isNonnullType(BO->getRHS()->getType())) {
12921310
State.NarrowedVars.insert(VD);
12931311
} else if (isNullableType(BO->getRHS()->getType(), StrictMode,
12941312
DefaultNullability) ||
12951313
isNullableInit(RHS)) {
12961314
State.NullableVars.insert(VD);
1297-
// Flow-sensitive assignment check: warn when assigning a
1298-
// nullable value to a _Nonnull variable.
12991315
if (isNonnullType(VD->getType())) {
13001316
++NumAssignmentWarnings;
13011317
Handler.handleNullableAssignment(BO, VD);
@@ -1861,8 +1877,14 @@ class TransferFunctions {
18611877

18621878
if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
18631879
if (isa<CXXThisExpr>(Base)) {
1864-
if (!isThisMemberNarrowed(FD))
1880+
// If flow analysis marked this member nullable (e.g. assigned nullptr),
1881+
// that overrides the declared _Nonnull type.
1882+
if (State.NullableThisMembers.contains(FD)) {
1883+
++NumDereferenceWarnings;
1884+
Handler.handleNullableDereference(DerefExpr, ME->getType());
1885+
} else if (!isThisMemberNarrowed(FD)) {
18651886
checkDeref(DerefExpr, ME->getType());
1887+
}
18661888
} else if (const auto *DRE = dyn_cast<DeclRefExpr>(Base)) {
18671889
if (const auto *BaseVD = dyn_cast<VarDecl>(DRE->getDecl())) {
18681890
if (!isMemberNarrowed(BaseVD, FD))
@@ -1913,6 +1935,10 @@ class ReturnNonnullTracker : public FlowNullabilityHandler {
19131935
void handleNullableAssignment(const Expr *E, const VarDecl *V) override {
19141936
Inner.handleNullableAssignment(E, V);
19151937
}
1938+
void handleNullableMemberAssignment(const Expr *E,
1939+
const FieldDecl *M) override {
1940+
Inner.handleNullableMemberAssignment(E, M);
1941+
}
19161942
void handleNullableArgument(const Expr *E, const ParmVarDecl *P) override {
19171943
Inner.handleNullableArgument(E, P);
19181944
}

‎clang/lib/Sema/AnalysisBasedWarnings.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2984,6 +2984,13 @@ class FlowNullabilityReporter : public FlowNullabilityHandler {
29842984
S.Diag(AssignExpr->getExprLoc(), diag::note_nullable_assignment_fix);
29852985
}
29862986

2987+
void handleNullableMemberAssignment(const Expr *AssignExpr,
2988+
const FieldDecl *Member) override {
2989+
S.Diag(AssignExpr->getExprLoc(), diag::warn_flow_nullable_member_assignment)
2990+
<< Member;
2991+
S.Diag(AssignExpr->getExprLoc(), diag::note_nullable_member_assignment_fix);
2992+
}
2993+
29872994
void handleNullableArgument(const Expr *ArgExpr,
29882995
const ParmVarDecl *Param) override {
29892996
S.Diag(ArgExpr->getExprLoc(), diag::warn_flow_nullable_argument) << Param;

‎clang/test/SemaCXX/flow-nullability-nonnull-assign.cpp‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// flow-nullability-nonnull-assign.cpp - Tests for assigning null to _Nonnull.
2+
//
3+
// Covers: field default init, member assignment, local variable assignment,
4+
// and dereference after null assignment to _Nonnull.
5+
//
6+
// RUN: %clang_cc1 -fsyntax-only -fflow-sensitive-nullability -fnullability-default=nullable -std=c++17 -Wno-unused-value %s -verify
7+
8+
struct Config { int timeout; };
9+
10+
// --- Gap 1: _Nonnull field initialized with nullptr in class body ---
11+
12+
struct FieldInitNull {
13+
Config* _Nonnull config_ = nullptr; // expected-warning{{initializing nonnull member 'config_'}} expected-note{{remove '_Nonnull'}}
14+
};
15+
16+
struct FieldInitZero {
17+
Config* _Nonnull config_ = 0; // expected-warning{{initializing nonnull member 'config_'}} expected-note{{remove '_Nonnull'}}
18+
};
19+
20+
struct FieldInitOk {
21+
Config* config_ = nullptr; // no warning — not _Nonnull
22+
};
23+
24+
struct FieldNoInit {
25+
Config* _Nonnull config_; // no warning — no initializer
26+
};
27+
28+
// --- Gap 2: assigning nullptr to _Nonnull member inside method ---
29+
30+
struct MemberAssignNull {
31+
Config* _Nonnull config_;
32+
MemberAssignNull(Config* _Nonnull c) : config_(c) {}
33+
void reset() {
34+
config_ = nullptr; // expected-warning{{assigning nullable pointer to nonnull member 'config_'}} expected-note{{add a null check}}
35+
}
36+
};
37+
38+
struct MemberAssignOk {
39+
Config* _Nonnull config_;
40+
MemberAssignOk(Config* _Nonnull c) : config_(c) {}
41+
void swap(Config* _Nonnull other) {
42+
config_ = other; // no warning — nonnull to nonnull
43+
}
44+
};
45+
46+
// --- Gap 3: assigning nullptr to _Nonnull local variable ---
47+
48+
void local_assign_null() {
49+
Config c;
50+
Config* _Nonnull p = &c;
51+
p = nullptr; // expected-warning{{assigning nullable pointer to nonnull variable 'p'}} expected-note{{add a null check}}
52+
}
53+
54+
void local_assign_ok() {
55+
Config c1, c2;
56+
Config* _Nonnull p = &c1;
57+
p = &c2; // no warning — address-of is nonnull
58+
}
59+
60+
// --- Gap 4: dereference after null assignment to _Nonnull member ---
61+
62+
struct DerefAfterNull {
63+
Config* _Nonnull config_;
64+
DerefAfterNull(Config* _Nonnull c) : config_(c) {}
65+
int bad() {
66+
config_ = nullptr; // expected-warning{{assigning nullable pointer to nonnull member 'config_'}} expected-note{{add a null check}}
67+
return config_->timeout; // expected-warning{{dereference of nullable pointer}} expected-note{{add a null check}}
68+
}
69+
};
70+
71+
// Deref after null assign to local
72+
int local_deref_after_null() {
73+
Config c;
74+
Config* _Nonnull p = &c;
75+
p = nullptr; // expected-warning{{assigning nullable pointer to nonnull variable 'p'}} expected-note{{add a null check}}
76+
return p->timeout; // expected-warning{{dereference of nullable pointer}} expected-note{{add a null check}}
77+
}

0 commit comments

Comments
 (0)