Check that assignment to parameters in function calls are null-safe.
For example, it is unsafe to pass a nullable value as a nonnull parameter.
```
void foo(int * _Nonnull x);
int * _Nullable x = ...;
foo(x); // Unsafe
```
PiperOrigin-RevId: 468782730
diff --git a/nullability_verification/BUILD b/nullability_verification/BUILD
index d98a9b1..9ff987c 100644
--- a/nullability_verification/BUILD
+++ b/nullability_verification/BUILD
@@ -36,6 +36,7 @@
"@llvm-project//clang:analysis",
"@llvm-project//clang:ast",
"@llvm-project//clang:ast_matchers",
+ "@llvm-project//clang:basic",
"@llvm-project//llvm:Support",
],
)
diff --git a/nullability_verification/pointer_nullability.cc b/nullability_verification/pointer_nullability.cc
index 09a0f1f..c53dfd9 100644
--- a/nullability_verification/pointer_nullability.cc
+++ b/nullability_verification/pointer_nullability.cc
@@ -52,6 +52,13 @@
initPointerBoolProperty(PointerVal, kNotNull, NotNullConstraint, Env);
}
+bool isNullable(const PointerValue& PointerVal, const Environment& Env) {
+ auto [PointerKnown, PointerNotNull] = getPointerNullState(PointerVal, Env);
+ auto& PointerNotKnownNull =
+ Env.makeNot(Env.makeAnd(PointerKnown, Env.makeNot(PointerNotNull)));
+ return !Env.flowConditionImplies(PointerNotKnownNull);
+}
+
} // namespace nullability
} // namespace tidy
} // namespace clang
diff --git a/nullability_verification/pointer_nullability.h b/nullability_verification/pointer_nullability.h
index c8d69e9..01c9c75 100644
--- a/nullability_verification/pointer_nullability.h
+++ b/nullability_verification/pointer_nullability.h
@@ -80,6 +80,10 @@
/*KnownConstraint=*/&Env.getBoolLiteralValue(false));
}
+/// Returns true if there is evidence that `PointerVal` may hold a nullptr.
+bool isNullable(const dataflow::PointerValue& PointerVal,
+ const dataflow::Environment& Env);
+
} // namespace nullability
} // namespace tidy
} // namespace clang
diff --git a/nullability_verification/pointer_nullability_diagnosis.cc b/nullability_verification/pointer_nullability_diagnosis.cc
index 82167f9..7387eef 100644
--- a/nullability_verification/pointer_nullability_diagnosis.cc
+++ b/nullability_verification/pointer_nullability_diagnosis.cc
@@ -8,6 +8,7 @@
#include "nullability_verification/pointer_nullability_matchers.h"
#include "clang/AST/Expr.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/Specifiers.h"
namespace clang {
namespace tidy {
@@ -22,12 +23,7 @@
const Expr* PointerExpr,
const Environment& Env) {
auto* PointerVal = getPointerValueFromExpr(PointerExpr, Env);
- if (!PointerVal) return PointerAccessExpr;
-
- auto [PointerKnown, PointerNotNull] = getPointerNullState(*PointerVal, Env);
- auto& PointerNotKnownNull =
- Env.makeNot(Env.makeAnd(PointerKnown, Env.makeNot(PointerNotNull)));
- if (!Env.flowConditionImplies(PointerNotKnownNull)) {
+ if (!PointerVal || isNullable(*PointerVal, Env)) {
return PointerAccessExpr;
}
return llvm::None;
@@ -45,6 +41,38 @@
return diagnosePointerAccess(MemberExpr, MemberExpr->getBase(), Env);
}
+// TODO(b/233582219): Handle call expressions whose callee is not a decl (e.g.
+// a function returned from another function), or when the callee cannot be
+// interpreted as a function type (e.g. a pointer to a function pointer).
+llvm::Optional<const Stmt*> diagnoseCallExpr(
+ const CallExpr* CE, const MatchFinder::MatchResult& Result,
+ const Environment& Env) {
+ auto* Callee = CE->getCalleeDecl();
+ if (!Callee) return llvm::None;
+
+ auto* CalleeType = Callee->getFunctionType();
+ if (!CalleeType) return llvm::None;
+
+ auto ParamTypes = CalleeType->getAs<FunctionProtoType>()->getParamTypes();
+
+ for (unsigned int I = 0; I < ParamTypes.size(); ++I) {
+ auto ParamType = ParamTypes[I];
+ if (!ParamType->isAnyPointerType() ||
+ ParamType->getNullability(*Result.Context)
+ .value_or(NullabilityKind::Unspecified) !=
+ NullabilityKind::NonNull) {
+ continue;
+ }
+ auto* Arg = CE->getArg(I);
+ auto* PointerVal = getPointerValueFromExpr(Arg, Env);
+ if (!PointerVal || isNullable(*PointerVal, Env)) {
+ return llvm::Optional<const Stmt*>(CE);
+ }
+ }
+
+ return llvm::None;
+}
+
auto buildDiagnoser() {
return dataflow::MatchSwitchBuilder<const Environment,
llvm::Optional<const Stmt*>>()
@@ -52,6 +80,8 @@
.CaseOf<UnaryOperator>(isPointerDereference(), diagnoseDereference)
// (->)
.CaseOf<MemberExpr>(isPointerArrow(), diagnoseArrow)
+ // Check compatibility of parameter assignments
+ .CaseOf<CallExpr>(isCallExpr(), diagnoseCallExpr)
.Build();
}
diff --git a/nullability_verification/pointer_nullability_verification_test.cc b/nullability_verification/pointer_nullability_verification_test.cc
index c4504f0..c28c4b6 100644
--- a/nullability_verification/pointer_nullability_verification_test.cc
+++ b/nullability_verification/pointer_nullability_verification_test.cc
@@ -1389,6 +1389,97 @@
)");
}
+TEST(PointerNullabilityTest, CallExprParamAssignment) {
+ // free function with single param
+ checkDiagnostics(R"(
+ void takeNonnull(int * _Nonnull);
+ void takeNullable(int * _Nullable);
+ void takeUnannotated(int *);
+ void target(int * _Nonnull ptr_nonnull,
+ int * _Nullable ptr_nullable,
+ int *ptr_unannotated) {
+ takeNonnull(nullptr); // [[unsafe]]
+ takeNonnull(ptr_nonnull);
+ takeNonnull(ptr_nullable); // [[unsafe]]
+ takeNonnull(ptr_unannotated);
+
+ takeNullable(nullptr);
+ takeNullable(ptr_nonnull);
+ takeNullable(ptr_nullable);
+ takeNullable(ptr_unannotated);
+
+ takeUnannotated(nullptr);
+ takeUnannotated(ptr_nonnull);
+ takeUnannotated(ptr_nullable);
+ takeUnannotated(ptr_unannotated);
+ }
+ )");
+
+ // free function with multiple params of mixed nullability
+ checkDiagnostics(R"(
+ void takeMixed(int *, int * _Nullable, int * _Nonnull);
+ void target() {
+ takeMixed(nullptr, nullptr, nullptr); // [[unsafe]]
+ }
+ )");
+
+ // member function
+ checkDiagnostics(R"(
+ struct Foo {
+ void takeNonnull(int * _Nonnull);
+ void takeNullable(int * _Nullable);
+ void takeUnannotated(int *);
+ };
+ void target(Foo foo) {
+ foo.takeNonnull(nullptr); // [[unsafe]]
+ foo.takeNullable(nullptr);
+ foo.takeUnannotated(nullptr);
+ }
+ )");
+
+ // function pointer
+ checkDiagnostics(R"(
+ void target(void (*takeNonnull)(int * _Nonnull),
+ void (*takeNullable)(int * _Nullable),
+ void (*takeUnannotated)(int *)) {
+ takeNonnull(nullptr); // [[unsafe]]
+ takeNullable(nullptr);
+ takeUnannotated(nullptr);
+ }
+ )");
+
+ // pointer to function pointer
+ //
+ // TODO(b/233582219): Fix false negative. Implement support for retrieving
+ // parameter types from a pointer to function pointer.
+ checkDiagnostics(R"(
+ void target(void (**takeNonnull)(int * _Nonnull),
+ void (**takeNullable)(int * _Nullable),
+ void (**takeUnannotated)(int *)) {
+ (*takeNonnull)(nullptr); // false-negative
+ (*takeNullable)(nullptr);
+ (*takeUnannotated)(nullptr);
+ }
+ )");
+
+ // function returned from function
+ //
+ // TODO(b/233582219): Fix false negative. Implement support for retrieving
+ // parameter types for functions returned by another function.
+ checkDiagnostics(R"(
+ typedef void (*takeNonnullF)(int * _Nonnull);
+ typedef void (*takeNullableF)(int * _Nullable);
+ typedef void (*takeUnannotatedF)(int *);
+ void target(takeNonnullF (*takeNonnull)(),
+ takeNullableF (*takeNullable)(),
+ takeUnannotatedF (*takeUnannotated)()) {
+ (*takeNonnull)()(nullptr); // false-negative
+ (*takeNullable)()(nullptr);
+ (*takeUnannotated)()(nullptr);
+ }
+ )");
+}
+
} // namespace
} // namespace nullability
} // namespace tidy
