Update pointer nullability when initialized in function call
PiperOrigin-RevId: 563260055
Change-Id: I91858156dfa3f7daa14f4c1ff7e36478e0715e86
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index 58fdb04..25fc9ea 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -388,6 +388,49 @@
State.Env.setValue(*CastExpr, State.Env.makeNot(PointerNull));
}
+void initializeOutputParameter(const Expr *Arg, dataflow::Environment &Env,
+ QualType ParamTy) {
+ // When a function has an "output parameter" - a non-const pointer or
+ // reference to a pointer of unknown nullability - assume that the function
+ // may set the pointer to non-null.
+ //
+ // For example, in the following code sequence we assume that the function may
+ // modify the pointer in a way that makes a subsequent dereference safe:
+ //
+ // void maybeModify(int ** _Nonnull);
+ //
+ // int *p = nullptr;
+ // initializePointer(&p);
+ // *p; // safe
+
+ if (ParamTy.isNull()) return;
+ if (ParamTy->getPointeeType().isNull()) return;
+ if (!isSupportedPointerType(ParamTy->getPointeeType())) return;
+ if (ParamTy->getPointeeType().isConstQualified()) return;
+
+ // TODO(b/298200521): This should extend support to annotations that suggest
+ // different in/out state
+ TypeNullability InnerNullability =
+ getNullabilityAnnotationsFromType(ParamTy->getPointeeType());
+ if (InnerNullability.front().concrete() != NullabilityKind::Unspecified)
+ return;
+
+ StorageLocation *Loc = nullptr;
+ if (ParamTy->isPointerType()) {
+ if (PointerValue *OuterPointer = getPointerValueFromExpr(Arg, Env))
+ Loc = &OuterPointer->getPointeeLoc();
+ } else if (ParamTy->isReferenceType()) {
+ Loc = Env.getStorageLocation(*Arg);
+ }
+ if (Loc == nullptr) return;
+
+ auto *InnerPointer =
+ cast<PointerValue>(Env.createValue(ParamTy->getPointeeType()));
+ initUnknownPointer(*InnerPointer, Env);
+
+ Env.setValue(*Loc, *InnerPointer);
+}
+
void transferFlowSensitiveCallExpr(
const CallExpr *CallExpr, const MatchFinder::MatchResult &Result,
TransferState<PointerNullabilityLattice> &State) {
@@ -426,6 +469,20 @@
// be a prvalue.
State.Env.setValue(*CallExpr, *PointerVal);
}
+
+ // Make output parameters (with unknown nullability) initialized to unknown.
+ const auto *FuncDecl = CallExpr->getDirectCallee();
+ if (!FuncDecl) return;
+ if (FuncDecl->getNumParams() != CallExpr->getNumArgs()) return;
+ if (auto *II = FuncDecl->getDeclName().getAsIdentifierInfo();
+ II && II->isStr("__assert_nullability")) {
+ return;
+ }
+ for (unsigned i = 0; i < CallExpr->getNumArgs(); ++i) {
+ const auto *Arg = CallExpr->getArg(i);
+ initializeOutputParameter(Arg, State.Env,
+ FuncDecl->getParamDecl(i)->getType());
+ }
}
// If nullability for the decl D has been overridden, patch N to reflect it.
diff --git a/nullability/test/function_calls.cc b/nullability/test/function_calls.cc
index 51b0b76..9935cfb 100644
--- a/nullability/test/function_calls.cc
+++ b/nullability/test/function_calls.cc
@@ -104,6 +104,150 @@
)cc"));
}
+TEST(PointerNullabilityTest, OutputParameterBasic) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int** p);
+ void target() {
+ int* p = nullptr;
+ maybeModifyPtr(&p);
+ *p;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterReference) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int*& r);
+ void target() {
+ int* p = nullptr;
+ maybeModifyPtr(p);
+ *p;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterReferenceConst) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void pointerNotModified(int* const& r);
+ void target() {
+ int* p = nullptr;
+ pointerNotModified(p);
+ *p; // [[unsafe]]
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterReferencePointerToPointer) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int**& r);
+ void target() {
+ int** pp = nullptr;
+ maybeModifyPtr(pp);
+ *pp;
+ **pp;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterConst) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void pointerNotModified(int* const* p);
+ void target(int* _Nullable p) {
+ pointerNotModified(&p);
+ *p; // [[unsafe]]
+ }
+ )cc"));
+
+ // The only const qualifier that should be considered is on the inner
+ // pointer, otherwise this pattern should be considered safe.
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(const int** const p);
+ void target() {
+ const int* p = nullptr;
+ maybeModifyPtr(&p);
+ *p;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterNonnull) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void pointerNotModified(int* _Nonnull* p);
+ void target(int* _Nonnull p) {
+ pointerNotModified(&p);
+ *p;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterCheckedNullable) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModify(int* _Nullable* p);
+ void target(int* _Nullable p) {
+ if (!p) return;
+ maybeModify(&p);
+ *p; // false negative: this dereference is actually unsafe!
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterNullable) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int* _Nullable* p);
+ void target() {
+ int* p = nullptr;
+ maybeModifyPtr(&p);
+ *p; // [[unsafe]]
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterConditional) {
+ // This tests that flow sensitivity is preserved, to catch for example if the
+ // underlying pointer was always set to Nonnull once it's passed as an
+ // output parameter.
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int** p);
+ void target(int* _Nullable j, bool b) {
+ if (b) {
+ maybeModifyPtr(&j);
+ }
+ if (b) {
+ *j;
+ }
+ if (!b) {
+ *j; // [[unsafe]]
+ }
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterWithoutAmpersandOperator) {
+ // This tests that the call to maybeModifyPtr works as expected if the param
+ // passed in doesn't directly use the & operator
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ void maybeModifyPtr(int** p);
+ void target(int* _Nullable p) {
+ auto pp = &p;
+ maybeModifyPtr(pp);
+ *p;
+ }
+ )cc"));
+}
+
+TEST(PointerNullabilityTest, OutputParameterTemplate) {
+ EXPECT_TRUE(checkDiagnostics(R"cc(
+ template <typename T>
+ struct S {
+ void maybeModify(T& ref);
+ };
+ void target(S<int*> s, int* _Nullable p) {
+ s.maybeModify(p);
+ *p;
+ }
+ )cc"));
+}
+
TEST(PointerNullabilityTest, CallExprParamAssignment) {
// free function with single param
EXPECT_TRUE(checkDiagnostics(R"cc(
@@ -233,7 +377,7 @@
*ptr_nullable; // [[unsafe]]
takeUnannotatedRef(ptr_nullable);
- *ptr_nullable; // [[unsafe]]
+ *ptr_nullable;
}
)cc"));
