[nullability] Model pointer arithmetic in shared analysis.
This introduces two false positives in the pointer_arithmetic_diagnosis test
that will be resolved by a followup patch (that adds diagnosis requiring the
operands of increment and decrement operators to be nonnull).
PiperOrigin-RevId: 670848600
Change-Id: I04e8076c8098b439cda4495b3b962e58d47c2d43
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index 4c042cf..efd6aae 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -382,6 +382,37 @@
}
}
+void transferValue_PointerIncOrDec(
+ absl::Nonnull<const UnaryOperator *> UnaryOp,
+ const MatchFinder::MatchResult &,
+ TransferState<PointerNullabilityLattice> &State) {
+ // The framework propagates the subexpression's value (in the case of post-
+ // increment) or storage location (in the case of pre-increment). We just
+ // need to create a new nonnull value.
+ if (StorageLocation *Loc =
+ State.Env.getStorageLocation(*UnaryOp->getSubExpr())) {
+ auto *Val = cast<PointerValue>(State.Env.createValue(Loc->getType()));
+ initPointerNullState(*Val, State.Env.getDataflowAnalysisContext(),
+ NullabilityKind::NonNull);
+ State.Env.setValue(*Loc, *Val);
+ }
+}
+
+void transferValue_PointerAddOrSubAssign(
+ absl::Nonnull<const BinaryOperator *> BinaryOp,
+ const MatchFinder::MatchResult &,
+ TransferState<PointerNullabilityLattice> &State) {
+ // The framework propagates the storage location of the LHS, so we just need
+ // to create a new nonnull value.
+ if (StorageLocation *Loc =
+ State.Env.getStorageLocation(*BinaryOp->getLHS())) {
+ auto *Val = cast<PointerValue>(State.Env.createValue(Loc->getType()));
+ initPointerNullState(*Val, State.Env.getDataflowAnalysisContext(),
+ NullabilityKind::NonNull);
+ State.Env.setValue(*Loc, *Val);
+ }
+}
+
void transferValue_NotNullPointer(
absl::Nonnull<const Expr *> NotNullPointer,
const MatchFinder::MatchResult &,
@@ -1479,10 +1510,19 @@
.drop_front()
.vec();
+ case UO_PreInc:
+ case UO_PreDec: {
+ TypeNullability SubNullability =
+ getNullabilityForChild(UO->getSubExpr(), State);
+ if (!isSupportedRawPointerType(UO->getSubExpr()->getType()))
+ return SubNullability;
+ assert(!SubNullability.empty());
+ SubNullability[0] = NullabilityKind::NonNull;
+ return SubNullability;
+ }
+
case UO_PostInc:
case UO_PostDec:
- case UO_PreInc:
- case UO_PreDec:
case UO_Plus:
case UO_Minus:
case UO_Not:
@@ -1512,6 +1552,23 @@
case BO_Assign:
case BO_Comma:
return getNullabilityForChild(BO->getRHS(), State);
+ case BO_Add:
+ case BO_Sub:
+ // The `+=` and `-=` operators will always take the "LHS" branch below but
+ // can otherwise be handled using the same code as `+` and `-`, so we do.
+ case BO_AddAssign:
+ case BO_SubAssign: {
+ TypeNullability PtrNullability;
+ if (isSupportedRawPointerType(BO->getLHS()->getType()))
+ PtrNullability = getNullabilityForChild(BO->getLHS(), State);
+ else if (isSupportedRawPointerType(BO->getRHS()->getType()))
+ PtrNullability = getNullabilityForChild(BO->getRHS(), State);
+ else
+ return unspecifiedNullability(BO);
+ assert(!PtrNullability.empty());
+ PtrNullability[0] = NullabilityKind::NonNull;
+ return PtrNullability;
+ }
default:
// No other built-in binary operators can be pointer-valued
return unspecifiedNullability(BO);
@@ -1605,6 +1662,10 @@
.CaseOfCFGStmt<Expr>(isNullPointerLiteral(), transferValue_NullPointer)
.CaseOfCFGStmt<CXXScalarValueInitExpr>(isRawPointerValueInit(),
transferValue_NullPointer)
+ .CaseOfCFGStmt<UnaryOperator>(isPointerIncOrDec(),
+ transferValue_PointerIncOrDec)
+ .CaseOfCFGStmt<BinaryOperator>(isPointerAddOrSubAssign(),
+ transferValue_PointerAddOrSubAssign)
.CaseOfCFGStmt<CXXConstructExpr>(isSmartPointerConstructor(),
transferValue_SmartPointerConstructor)
.CaseOfCFGStmt<CXXOperatorCallExpr>(isSmartPointerOperatorCall("="),
diff --git a/nullability/pointer_nullability_matchers.cc b/nullability/pointer_nullability_matchers.cc
index 493799a..01ea2ce 100644
--- a/nullability/pointer_nullability_matchers.cc
+++ b/nullability/pointer_nullability_matchers.cc
@@ -55,6 +55,7 @@
using ast_matchers::isArrow;
using ast_matchers::isConst;
using ast_matchers::isInStdNamespace;
+using ast_matchers::isInteger;
using ast_matchers::isMemberInitializer;
using ast_matchers::memberExpr;
using ast_matchers::ofClass;
@@ -85,6 +86,14 @@
return binaryOperator(hasAnyOperatorName("!=", "=="),
hasOperands(isPointerExpr(), isPointerExpr()));
}
+Matcher<Stmt> isPointerIncOrDec() {
+ return unaryOperator(hasAnyOperatorName("++", "--"),
+ hasUnaryOperand(isPointerExpr()));
+}
+Matcher<Stmt> isPointerAddOrSubAssign() {
+ return binaryOperator(hasAnyOperatorName("+=", "-="),
+ hasOperands(isPointerExpr(), hasType(isInteger())));
+}
Matcher<Stmt> isImplicitCastPointerToBool() {
return implicitCastExpr(hasCastKind(CK_PointerToBoolean));
}
diff --git a/nullability/pointer_nullability_matchers.h b/nullability/pointer_nullability_matchers.h
index 2969faf..396c93f 100644
--- a/nullability/pointer_nullability_matchers.h
+++ b/nullability/pointer_nullability_matchers.h
@@ -44,6 +44,8 @@
ast_matchers::internal::Matcher<Stmt> isPointerDereference();
ast_matchers::internal::Matcher<Stmt> isPointerSubscript();
ast_matchers::internal::Matcher<Stmt> isPointerCheckBinOp();
+ast_matchers::internal::Matcher<Stmt> isPointerIncOrDec();
+ast_matchers::internal::Matcher<Stmt> isPointerAddOrSubAssign();
ast_matchers::internal::Matcher<Stmt> isImplicitCastPointerToBool();
ast_matchers::internal::Matcher<Stmt> isPointerReturn();
ast_matchers::internal::Matcher<CXXCtorInitializer> isCtorMemberInitializer();
diff --git a/nullability/test/BUILD b/nullability/test/BUILD
index 9585290..4dbc297 100644
--- a/nullability/test/BUILD
+++ b/nullability/test/BUILD
@@ -268,6 +268,11 @@
],
)
+nullability_test(
+ name = "pointer_arithmetic",
+ srcs = ["pointer_arithmetic.cc"],
+)
+
cc_test(
name = "pointer_arithmetic_diagnosis",
srcs = ["pointer_arithmetic_diagnosis.cc"],
diff --git a/nullability/test/pointer_arithmetic.cc b/nullability/test/pointer_arithmetic.cc
new file mode 100644
index 0000000..3780c10
--- /dev/null
+++ b/nullability/test/pointer_arithmetic.cc
@@ -0,0 +1,131 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+// Tests for pointer arithmetic.
+
+// Note that, for example, the increment and decrement operators do not cause
+// their operand to become nonnull, even though we know that they may only be
+// applied to nonnull pointers. This is consistent with our treatment of other
+// operators, such as `*` and `->`; these also do not cause their operand to
+// become nonnull.
+
+#include "nullability_test.h"
+
+// This test intentionally contains violations of `-Wunsafe-buffer-usage`, so
+// turn it off.
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunsafe-buffer-usage"
+
+TEST void preIncrementType(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam) {
+ type<Nonnull<int *>>(++NonnullParam);
+ type<Nonnull<int *>>(++NullableParam);
+ type<Nonnull<int *>>(++UnknownParam);
+}
+
+TEST void preIncrementValue(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam) {
+ int *NewVal = ++NonnullParam;
+ provable(NonnullParam == NewVal);
+ nonnull(NonnullParam);
+
+ NewVal = ++NullableParam;
+ provable(NullableParam == NewVal);
+ nonnull(NullableParam);
+
+ NewVal = ++UnknownParam;
+ provable(UnknownParam == NewVal);
+ nonnull(UnknownParam);
+}
+
+TEST void postIncrementType(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam) {
+ type<Nonnull<int *>>(NonnullParam++);
+ type<Nullable<int *>>(NullableParam++);
+ type<int *>(UnknownParam++);
+}
+
+TEST void postIncrementValue(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam) {
+ int *OldVal = NonnullParam;
+ provable(NonnullParam++ == OldVal);
+ nonnull(NonnullParam);
+
+ OldVal = NullableParam;
+ provable(NullableParam++ == OldVal);
+ nonnull(NullableParam);
+
+ OldVal = UnknownParam;
+ provable(UnknownParam++ == OldVal);
+ nonnull(UnknownParam);
+}
+
+TEST void add(Nonnull<int *> NonnullParam, Nullable<int *> NullableParam,
+ int *UnknownParam, int I) {
+ type<Nonnull<int *>>(NonnullParam + I);
+ type<Nonnull<int *>>(I + NonnullParam);
+
+ type<Nonnull<int *>>(NullableParam + I);
+ type<Nonnull<int *>>(I + NullableParam);
+
+ type<Nonnull<int *>>(UnknownParam + I);
+ type<Nonnull<int *>>(I + UnknownParam);
+}
+
+TEST void subtract(Nonnull<int *> NonnullParam, Nullable<int *> NullableParam,
+ int *UnknownParam, int I) {
+ type<Nonnull<int *>>(NonnullParam - I);
+ type<Nonnull<int *>>(NullableParam - I);
+ type<Nonnull<int *>>(UnknownParam - I);
+}
+
+TEST void addAssignType(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam,
+ int I) {
+ type<Nonnull<int *>>(NonnullParam += I);
+ type<Nonnull<int *>>(NullableParam += I);
+ type<Nonnull<int *>>(UnknownParam += I);
+}
+
+TEST void addAssignValue(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam,
+ int I) {
+ int *NewVal = (NonnullParam += I);
+ provable(NonnullParam == NewVal);
+ nonnull(NonnullParam);
+
+ NewVal = (NullableParam += I);
+ provable(NullableParam == NewVal);
+ nonnull(NullableParam);
+
+ NewVal = (UnknownParam += I);
+ provable(UnknownParam == NewVal);
+ nonnull(UnknownParam);
+}
+
+TEST void subtractAssignType(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam,
+ int I) {
+ type<Nonnull<int *>>(NonnullParam -= I);
+ type<Nonnull<int *>>(NullableParam -= I);
+ type<Nonnull<int *>>(UnknownParam -= I);
+}
+
+TEST void subtractAssignValue(Nonnull<int *> NonnullParam,
+ Nullable<int *> NullableParam, int *UnknownParam,
+ int I) {
+ int *NewVal = (NonnullParam -= I);
+ provable(NonnullParam == NewVal);
+ nonnull(NonnullParam);
+
+ NewVal = (NullableParam -= I);
+ provable(NullableParam == NewVal);
+ nonnull(NullableParam);
+
+ NewVal = (UnknownParam -= I);
+ provable(UnknownParam == NewVal);
+ nonnull(UnknownParam);
+}
+
+#pragma clang diagnostic pop
diff --git a/nullability/test/pointer_arithmetic_diagnosis.cc b/nullability/test/pointer_arithmetic_diagnosis.cc
index 1fa81a4..86131d0 100644
--- a/nullability/test/pointer_arithmetic_diagnosis.cc
+++ b/nullability/test/pointer_arithmetic_diagnosis.cc
@@ -25,13 +25,19 @@
*nullable++; // [[unsafe]]
nullable = orig;
- *++nullable; // [[unsafe]]
+ // TODO(mboehme): False negative. The increment operation is creating a
+ // new nonnull value, so the dereference isn't considered unsafe, but the
+ // increment of a nullable pointer should itself be considered unsafe.
+ *++nullable;
nullable = orig;
*nullable--; // [[unsafe]]
nullable = orig;
- *--nullable; // [[unsafe]]
+ // TODO(mboehme): False negative. The decrement operation is creating a
+ // new nonnull value, so the dereference isn't considered unsafe, but the
+ // decrement of a nullable pointer should itself be considered unsafe.
+ *--nullable;
nullable = orig;
// On a nullable pointer, the pointer arithmetic itself should already be
