[nullability] Rework the way we ensure raw pointers are initialized.
We’re making this more consistent with smart pointers in that we give our
transfer functions a chance to initialize the pointer first and only create a
“backup” value if the transfer functions didn’t initialize the pointer.
I was prompted to make this change when I observed that, for raw pointer
glvalues, `ensurePointerHasValue()` would create a storage location if necessary
but not associate it with the expression. This seemed like a simple fix, but
merely adding an `Env.setStorageLocation()` caused failures because
`modelGetReferenceableValue()` was now trying to set a storage location for an
expression that already had one.
This can be fixed by moving `ensurePointerHasValue()` until after the transfer
function has run, which makes things more consistent with the treatment of smart
pointers too. To emphasize the distinction between raw and smart pointers, I am
renaming `ensurePointerHasValue()` to `ensureRawPointerHasValue()`.
Some transfer functions previously assumed that the expression they operate on
was already guaranteed to be associated with a pointer. Now, transfer functions
need to take care of this themselves if necessary. To do this, I am providing an
overload of `ensureRawPointerHasValue()` that takes an expression and returns
the pointer associated with that expression (either an already existing pointer
or a newly created one).
PiperOrigin-RevId: 646927739
Change-Id: Id05e9fff268127814606b97dadd7cd9c995eed97
diff --git a/bazel/llvm.bzl b/bazel/llvm.bzl
index a3e90c5..3c1e17c 100644
--- a/bazel/llvm.bzl
+++ b/bazel/llvm.bzl
@@ -53,7 +53,7 @@
executable = False,
)
-LLVM_COMMIT_SHA = "4e0a0eae58f7a6998866719f7eb970096a2a52e9"
+LLVM_COMMIT_SHA = "54b61adc0cbefb7f923ef43c407704ba9f9d8b69"
def llvm_loader_repository_dependencies():
# This *declares* the dependency, but it won't actually be *downloaded* unless it's used.
diff --git a/nullability/pointer_nullability_analysis.cc b/nullability/pointer_nullability_analysis.cc
index fca04ea..59a4dd6 100644
--- a/nullability/pointer_nullability_analysis.cc
+++ b/nullability/pointer_nullability_analysis.cc
@@ -79,6 +79,43 @@
return Result;
}
+// If `E` is already associated with a `PointerValue`, returns it.
+// Otherwise, associates a newly created `PointerValue` with `E` and returns it.
+// Returns null iff `E` is not a raw pointer expression.
+absl::Nullable<PointerValue *> ensureRawPointerHasValue(
+ absl::Nonnull<const Expr *> E, Environment &Env) {
+ if (!isSupportedRawPointerType(E->getType())) return nullptr;
+
+ if (E->isPRValue()) {
+ if (auto *Val = Env.get<PointerValue>(*E)) return Val;
+ auto *Val = cast<PointerValue>(Env.createValue(E->getType()));
+ Env.setValue(*E, *Val);
+ return Val;
+ }
+
+ StorageLocation *Loc = Env.getStorageLocation(*E);
+ if (Loc == nullptr) {
+ Loc = &Env.createStorageLocation(*E);
+ Env.setStorageLocation(*E, *Loc);
+ }
+ if (auto *Val = Env.get<PointerValue>(*Loc)) return Val;
+ auto *Val = cast<PointerValue>(Env.createValue(E->getType()));
+ Env.setValue(*Loc, *Val);
+ return Val;
+}
+
+// If `Elt` is an expression of raw pointer type, ensures that it has a
+// `PointerValue` associated with it.
+void ensureRawPointerHasValue(const CFGElement &Elt, Environment &Env) {
+ auto S = Elt.getAs<CFGStmt>();
+ if (!S) return;
+
+ const Expr *E = dyn_cast<Expr>(S->getStmt());
+ if (!E) return;
+
+ ensureRawPointerHasValue(E, Env);
+}
+
void computeNullability(absl::Nonnull<const Expr *> E,
TransferState<PointerNullabilityLattice> &State,
std::function<TypeNullability()> Compute) {
@@ -326,7 +363,7 @@
void transferValue_NullPointer(
absl::Nonnull<const Expr *> NullPointer, const MatchFinder::MatchResult &,
TransferState<PointerNullabilityLattice> &State) {
- if (auto *PointerVal = getRawPointerValue(NullPointer, State.Env)) {
+ if (auto *PointerVal = ensureRawPointerHasValue(NullPointer, State.Env)) {
initNullPointer(*PointerVal, State.Env.getDataflowAnalysisContext());
}
}
@@ -335,7 +372,7 @@
absl::Nonnull<const Expr *> NotNullPointer,
const MatchFinder::MatchResult &,
TransferState<PointerNullabilityLattice> &State) {
- if (auto *PointerVal = getRawPointerValue(NotNullPointer, State.Env)) {
+ if (auto *PointerVal = ensureRawPointerHasValue(NotNullPointer, State.Env)) {
initPointerNullState(*PointerVal, State.Env.getDataflowAnalysisContext(),
NullabilityKind::NonNull);
}
@@ -709,7 +746,7 @@
void transferValue_Pointer(absl::Nonnull<const Expr *> PointerExpr,
const MatchFinder::MatchResult &Result,
TransferState<PointerNullabilityLattice> &State) {
- auto *PointerVal = getRawPointerValue(PointerExpr, State.Env);
+ auto *PointerVal = ensureRawPointerHasValue(PointerExpr, State.Env);
if (!PointerVal) return;
initPointerFromTypeNullability(*PointerVal, PointerExpr, State);
@@ -1040,9 +1077,7 @@
PointerVal = dyn_cast_or_null<PointerValue>(State.Env.getValue(*Loc));
}
if (!PointerVal) {
- // Use value that may have been set by the builtin transfer function or by
- // `ensurePointerHasValue()`.
- PointerVal = getRawPointerValue(MCE, State.Env);
+ PointerVal = ensureRawPointerHasValue(MCE, State.Env);
}
if (PointerVal) {
State.Env.setValue(*MCE, *PointerVal);
@@ -1585,27 +1620,6 @@
.Build();
}
-// Ensure that all expressions of raw pointer type have a `PointerValue`
-// associated with them so we can track nullability through them.
-void ensurePointerHasValue(const CFGElement &Elt, Environment &Env) {
- auto S = Elt.getAs<CFGStmt>();
- if (!S) return;
-
- auto *E = dyn_cast<Expr>(S->getStmt());
- if (E == nullptr || !isSupportedRawPointerType(E->getType())) return;
-
- if (E->isPRValue()) {
- if (Env.getValue(*E) == nullptr)
- // `createValue()` always produces a value for pointer types.
- Env.setValue(*E, *Env.createValue(E->getType()));
- } else {
- StorageLocation *Loc = Env.getStorageLocation(*E);
- if (Loc == nullptr) Loc = &Env.createStorageLocation(*E);
- if (Env.getValue(*Loc) == nullptr)
- Env.setValue(*Loc, *Env.createValue(E->getType()));
- }
-}
-
// Ensure that all expressions of smart pointer type have an underlying
// raw pointer initialized from the type nullability.
void ensureSmartPointerInitialized(
@@ -1655,9 +1669,9 @@
Environment &Env) {
TransferState<PointerNullabilityLattice> State(Lattice, Env);
- ensurePointerHasValue(Elt, Env);
TypeTransferer(Elt, getASTContext(), State);
ValueTransferer(Elt, getASTContext(), State);
+ ensureRawPointerHasValue(Elt, Env);
ensureSmartPointerInitialized(Elt, State);
}
