mirror of
https://github.com/llvm/llvm-project.git
synced 2025-04-29 11:46:06 +00:00
[clang][dataflow] Relax validity assumptions in UncheckedOptionalAccessModel.
Currently, the interpretation of `swap` calls in the optional model assumes the optional arguments are modeled (and therefore have valid storage locations and values). This assumption is incorrect, for example, in the case of unmodeled optional fields (which can be missing either value or location). This patch relaxes these assumptions, to return rather than assert when either argument is not modeled. Differential Revision: https://reviews.llvm.org/D142710
This commit is contained in:
Yitzhak Mandelbaum
parent
b0c1a45ba5
commit
d4fb829b71
@ -521,48 +521,54 @@ void transferNulloptAssignment(const CXXOperatorCallExpr *E,
|
||||
transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
|
||||
}
|
||||
|
||||
void transferSwap(const StorageLocation &OptionalLoc1,
|
||||
const StorageLocation &OptionalLoc2,
|
||||
LatticeTransferState &State) {
|
||||
auto *OptionalVal1 = State.Env.getValue(OptionalLoc1);
|
||||
assert(OptionalVal1 != nullptr);
|
||||
void transferSwap(const Expr &E1, SkipPast E1Skip, const Expr &E2,
|
||||
Environment &Env) {
|
||||
// We account for cases where one or both of the optionals are not modeled,
|
||||
// either lacking associated storage locations, or lacking values associated
|
||||
// to such storage locations.
|
||||
auto *Loc1 = Env.getStorageLocation(E1, E1Skip);
|
||||
auto *Loc2 = Env.getStorageLocation(E2, SkipPast::Reference);
|
||||
|
||||
auto *OptionalVal2 = State.Env.getValue(OptionalLoc2);
|
||||
assert(OptionalVal2 != nullptr);
|
||||
if (Loc1 == nullptr) {
|
||||
if (Loc2 != nullptr)
|
||||
Env.setValue(*Loc2, createOptionalValue(Env, Env.makeAtomicBoolValue()));
|
||||
return;
|
||||
}
|
||||
if (Loc2 == nullptr) {
|
||||
Env.setValue(*Loc1, createOptionalValue(Env, Env.makeAtomicBoolValue()));
|
||||
return;
|
||||
}
|
||||
|
||||
State.Env.setValue(OptionalLoc1, *OptionalVal2);
|
||||
State.Env.setValue(OptionalLoc2, *OptionalVal1);
|
||||
// Both expressions have locations, though they may not have corresponding
|
||||
// values. In that case, we create a fresh value at this point. Note that if
|
||||
// two branches both do this, they will not share the value, but it at least
|
||||
// allows for local reasoning about the value. To avoid the above, we would
|
||||
// need *lazy* value allocation.
|
||||
// FIXME: allocate values lazily, instead of just creating a fresh value.
|
||||
auto *Val1 = Env.getValue(*Loc1);
|
||||
if (Val1 == nullptr)
|
||||
Val1 = &createOptionalValue(Env, Env.makeAtomicBoolValue());
|
||||
|
||||
auto *Val2 = Env.getValue(*Loc2);
|
||||
if (Val2 == nullptr)
|
||||
Val2 = &createOptionalValue(Env, Env.makeAtomicBoolValue());
|
||||
|
||||
Env.setValue(*Loc1, *Val2);
|
||||
Env.setValue(*Loc2, *Val1);
|
||||
}
|
||||
|
||||
void transferSwapCall(const CXXMemberCallExpr *E,
|
||||
const MatchFinder::MatchResult &,
|
||||
LatticeTransferState &State) {
|
||||
assert(E->getNumArgs() == 1);
|
||||
|
||||
auto *OptionalLoc1 = State.Env.getStorageLocation(
|
||||
*E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer);
|
||||
assert(OptionalLoc1 != nullptr);
|
||||
|
||||
auto *OptionalLoc2 =
|
||||
State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
|
||||
assert(OptionalLoc2 != nullptr);
|
||||
|
||||
transferSwap(*OptionalLoc1, *OptionalLoc2, State);
|
||||
transferSwap(*E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer,
|
||||
*E->getArg(0), State.Env);
|
||||
}
|
||||
|
||||
void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
|
||||
LatticeTransferState &State) {
|
||||
assert(E->getNumArgs() == 2);
|
||||
|
||||
auto *OptionalLoc1 =
|
||||
State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
|
||||
assert(OptionalLoc1 != nullptr);
|
||||
|
||||
auto *OptionalLoc2 =
|
||||
State.Env.getStorageLocation(*E->getArg(1), SkipPast::Reference);
|
||||
assert(OptionalLoc2 != nullptr);
|
||||
|
||||
transferSwap(*OptionalLoc1, *OptionalLoc2, State);
|
||||
transferSwap(*E->getArg(0), SkipPast::Reference, *E->getArg(1), State.Env);
|
||||
}
|
||||
|
||||
BoolValue &evaluateEquality(Environment &Env, BoolValue &EqVal, BoolValue &LHS,
|
||||
|
||||
@ -11,7 +11,6 @@
|
||||
#include "TestingSupport.h"
|
||||
#include "clang/AST/ASTContext.h"
|
||||
#include "clang/ASTMatchers/ASTMatchers.h"
|
||||
#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
|
||||
#include "clang/Basic/SourceLocation.h"
|
||||
#include "clang/Tooling/Tooling.h"
|
||||
#include "llvm/ADT/DenseSet.h"
|
||||
@ -2124,6 +2123,139 @@ TEST_P(UncheckedOptionalAccessTest, StdSwap) {
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledLocLeft) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct L { $ns::$optional<int> hd; L* tl; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<int> foo = 3;
|
||||
L bar;
|
||||
|
||||
// Any `tl` beyond the first is not modeled.
|
||||
bar.tl->tl->hd.swap(foo);
|
||||
|
||||
bar.tl->tl->hd.value(); // [[unsafe]]
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledLocRight) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct L { $ns::$optional<int> hd; L* tl; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<int> foo = 3;
|
||||
L bar;
|
||||
|
||||
// Any `tl` beyond the first is not modeled.
|
||||
foo.swap(bar.tl->tl->hd);
|
||||
|
||||
bar.tl->tl->hd.value(); // [[unsafe]]
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueLeftSet) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct S { int x; };
|
||||
struct A { $ns::$optional<S> late; };
|
||||
struct B { A f3; };
|
||||
struct C { B f2; };
|
||||
struct D { C f1; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<S> foo = S{3};
|
||||
D bar;
|
||||
|
||||
bar.f1.f2.f3.late.swap(foo);
|
||||
|
||||
bar.f1.f2.f3.late.value();
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueLeftUnset) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct S { int x; };
|
||||
struct A { $ns::$optional<S> late; };
|
||||
struct B { A f3; };
|
||||
struct C { B f2; };
|
||||
struct D { C f1; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<S> foo;
|
||||
D bar;
|
||||
|
||||
bar.f1.f2.f3.late.swap(foo);
|
||||
|
||||
bar.f1.f2.f3.late.value(); // [[unsafe]]
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
// fixme: use recursion instead of depth.
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueRightSet) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct S { int x; };
|
||||
struct A { $ns::$optional<S> late; };
|
||||
struct B { A f3; };
|
||||
struct C { B f2; };
|
||||
struct D { C f1; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<S> foo = S{3};
|
||||
D bar;
|
||||
|
||||
foo.swap(bar.f1.f2.f3.late);
|
||||
|
||||
bar.f1.f2.f3.late.value();
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, SwapUnmodeledValueRightUnset) {
|
||||
ExpectDiagnosticsFor(
|
||||
R"(
|
||||
#include "unchecked_optional_access_test.h"
|
||||
|
||||
struct S { int x; };
|
||||
struct A { $ns::$optional<S> late; };
|
||||
struct B { A f3; };
|
||||
struct C { B f2; };
|
||||
struct D { C f1; };
|
||||
|
||||
void target() {
|
||||
$ns::$optional<S> foo;
|
||||
D bar;
|
||||
|
||||
foo.swap(bar.f1.f2.f3.late);
|
||||
|
||||
bar.f1.f2.f3.late.value(); // [[unsafe]]
|
||||
foo.value(); // [[unsafe]]
|
||||
}
|
||||
)");
|
||||
}
|
||||
|
||||
TEST_P(UncheckedOptionalAccessTest, UniquePtrToOptional) {
|
||||
// We suppress diagnostics for optionals in smart pointers (other than
|
||||
// `optional` itself).
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user