diff options
Diffstat (limited to 'contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp')
| -rw-r--r-- | contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp | 535 | 
1 files changed, 340 insertions, 195 deletions
diff --git a/contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 308dc25dad1f..b0a8667f3fe5 100644 --- a/contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/contrib/llvm-project/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -18,9 +18,11 @@  #include "clang/AST/ExprCXX.h"  #include "clang/AST/Stmt.h"  #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchersMacros.h"  #include "clang/Analysis/CFG.h"  #include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"  #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" +#include "clang/Analysis/FlowSensitive/Formula.h"  #include "clang/Analysis/FlowSensitive/NoopLattice.h"  #include "clang/Analysis/FlowSensitive/StorageLocation.h"  #include "clang/Analysis/FlowSensitive/Value.h" @@ -36,16 +38,45 @@  namespace clang {  namespace dataflow { + +static bool isTopLevelNamespaceWithName(const NamespaceDecl &NS, +                                        llvm::StringRef Name) { +  return NS.getDeclName().isIdentifier() && NS.getName() == Name && +         NS.getParent() != nullptr && NS.getParent()->isTranslationUnit(); +} + +static bool hasOptionalClassName(const CXXRecordDecl &RD) { +  if (!RD.getDeclName().isIdentifier()) +    return false; + +  if (RD.getName() == "optional") { +    if (const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext())) +      return N->isStdNamespace() || isTopLevelNamespaceWithName(*N, "absl"); +    return false; +  } + +  if (RD.getName() == "Optional") { +    // Check whether namespace is "::base" or "::folly". +    const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext()); +    return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") || +                            isTopLevelNamespaceWithName(*N, "folly")); +  } + +  return false; +} +  namespace {  using namespace ::clang::ast_matchers;  using LatticeTransferState = TransferState<NoopLattice>; +AST_MATCHER(CXXRecordDecl, hasOptionalClassNameMatcher) { +  return hasOptionalClassName(Node); +} +  DeclarationMatcher optionalClass() {    return classTemplateSpecializationDecl( -      anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"), -            hasName("__optional_destruct_base"), hasName("absl::optional"), -            hasName("base::Optional")), +      hasOptionalClassNameMatcher(),        hasTemplateArgument(0, refersToType(type().bind("T"))));  } @@ -57,14 +88,16 @@ auto optionalOrAliasType() {  /// Matches any of the spellings of the optional types and sugar, aliases, etc.  auto hasOptionalType() { return hasType(optionalOrAliasType()); } -auto isOptionalMemberCallWithName( -    llvm::StringRef MemberName, +auto isOptionalMemberCallWithNameMatcher( +    ast_matchers::internal::Matcher<NamedDecl> matcher,      const std::optional<StatementMatcher> &Ignorable = std::nullopt) {    auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))                                      : cxxThisExpr());    return cxxMemberCallExpr( -      on(expr(Exception)), -      callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass())))); +      on(expr(Exception, +              anyOf(hasOptionalType(), +                    hasType(pointerType(pointee(optionalOrAliasType())))))), +      callee(cxxMethodDecl(matcher)));  }  auto isOptionalOperatorCallWithName( @@ -77,15 +110,15 @@ auto isOptionalOperatorCallWithName(  }  auto isMakeOptionalCall() { -  return callExpr( -      callee(functionDecl(hasAnyName( -          "std::make_optional", "base::make_optional", "absl::make_optional"))), -      hasOptionalType()); +  return callExpr(callee(functionDecl(hasAnyName( +                      "std::make_optional", "base::make_optional", +                      "absl::make_optional", "folly::make_optional"))), +                  hasOptionalType());  }  auto nulloptTypeDecl() { -  return namedDecl( -      hasAnyName("std::nullopt_t", "absl::nullopt_t", "base::nullopt_t")); +  return namedDecl(hasAnyName("std::nullopt_t", "absl::nullopt_t", +                              "base::nullopt_t", "folly::None"));  }  auto hasNulloptType() { return hasType(nulloptTypeDecl()); } @@ -96,10 +129,9 @@ auto hasAnyOptionalType() {        recordType(hasDeclaration(anyOf(nulloptTypeDecl(), optionalClass())))));  } -  auto inPlaceClass() { -  return recordDecl( -      hasAnyName("std::in_place_t", "absl::in_place_t", "base::in_place_t")); +  return recordDecl(hasAnyName("std::in_place_t", "absl::in_place_t", +                               "base::in_place_t", "folly::in_place_t"));  }  auto isOptionalNulloptConstructor() { @@ -149,6 +181,11 @@ auto isStdSwapCall() {                    hasArgument(1, hasOptionalType()));  } +auto isStdForwardCall() { +  return callExpr(callee(functionDecl(hasName("std::forward"))), +                  argumentCountIs(1), hasArgument(0, hasOptionalType())); +} +  constexpr llvm::StringLiteral ValueOrCallID = "ValueOrCall";  auto isValueOrStringEmptyCall() { @@ -199,17 +236,17 @@ auto isComparisonOperatorCall(L lhs_arg_matcher, R rhs_arg_matcher) {        hasArgument(1, rhs_arg_matcher));  } -// Ensures that `Expr` is mapped to a `BoolValue` and returns it. -BoolValue &forceBoolValue(Environment &Env, const Expr &Expr) { +/// Ensures that `Expr` is mapped to a `BoolValue` and returns its formula. +const Formula &forceBoolValue(Environment &Env, const Expr &Expr) {    auto *Value = cast_or_null<BoolValue>(Env.getValue(Expr, SkipPast::None));    if (Value != nullptr) -    return *Value; +    return Value->formula();    auto &Loc = Env.createStorageLocation(Expr);    Value = &Env.makeAtomicBoolValue();    Env.setValue(Loc, *Value);    Env.setStorageLocation(Expr, Loc); -  return *Value; +  return Value->formula();  }  /// Sets `HasValueVal` as the symbolic value that represents the "has_value" @@ -218,12 +255,15 @@ void setHasValue(Value &OptionalVal, BoolValue &HasValueVal) {    OptionalVal.setProperty("has_value", HasValueVal);  } -/// Creates a symbolic value for an `optional` value using `HasValueVal` as the -/// symbolic value of its "has_value" property. -StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) { -  auto OptionalVal = std::make_unique<StructValue>(); -  setHasValue(*OptionalVal, HasValueVal); -  return Env.takeOwnership(std::move(OptionalVal)); +/// Creates a symbolic value for an `optional` value at an existing storage +/// location. Uses `HasValueVal` as the symbolic value of the "has_value" +/// property. +StructValue &createOptionalValue(AggregateStorageLocation &Loc, +                                 BoolValue &HasValueVal, Environment &Env) { +  auto &OptionalVal = Env.create<StructValue>(Loc); +  Env.setValue(Loc, OptionalVal); +  setHasValue(OptionalVal, HasValueVal); +  return OptionalVal;  }  /// Returns the symbolic value that represents the "has_value" property of the @@ -241,20 +281,12 @@ BoolValue *getHasValue(Environment &Env, Value *OptionalVal) {    return nullptr;  } -/// If `Type` is a reference type, returns the type of its pointee. Otherwise, -/// returns `Type` itself. -QualType stripReference(QualType Type) { -  return Type->isReferenceType() ? Type->getPointeeType() : Type; -} -  /// Returns true if and only if `Type` is an optional type.  bool isOptionalType(QualType Type) {    if (!Type->isRecordType())      return false; -  // FIXME: Optimize this by avoiding the `getQualifiedNameAsString` call. -  auto TypeName = Type->getAsCXXRecordDecl()->getQualifiedNameAsString(); -  return TypeName == "std::optional" || TypeName == "absl::optional" || -         TypeName == "base::Optional"; +  const CXXRecordDecl *D = Type->getAsCXXRecordDecl(); +  return D != nullptr && hasOptionalClassName(*D);  }  /// Returns the number of optional wrappers in `Type`. @@ -280,40 +312,91 @@ StorageLocation *maybeInitializeOptionalValueMember(QualType Q,                                                      Environment &Env) {    // The "value" property represents a synthetic field. As such, it needs    // `StorageLocation`, like normal fields (and other variables). So, we model -  // it with a `ReferenceValue`, since that includes a storage location.  Once +  // it with a `PointerValue`, since that includes a storage location.  Once    // the property is set, it will be shared by all environments that access the    // `Value` representing the optional (here, `OptionalVal`).    if (auto *ValueProp = OptionalVal.getProperty("value")) { -    auto *ValueRef = clang::cast<ReferenceValue>(ValueProp); -    auto &ValueLoc = ValueRef->getReferentLoc(); -    if (Env.getValue(ValueLoc) == nullptr) { -      // The property was previously set, but the value has been lost. This can -      // happen, for example, because of an environment merge (where the two -      // environments mapped the property to different values, which resulted in -      // them both being discarded), or when two blocks in the CFG, with neither -      // a dominator of the other, visit the same optional value, or even when a -      // block is revisited during testing to collect per-statement state. -      // FIXME: This situation means that the optional contents are not shared -      // between branches and the like. Practically, this lack of sharing -      // reduces the precision of the model when the contents are relevant to -      // the check, like another optional or a boolean that influences control -      // flow. +    auto *ValuePtr = clang::cast<PointerValue>(ValueProp); +    auto &ValueLoc = ValuePtr->getPointeeLoc(); +    if (Env.getValue(ValueLoc) != nullptr) +      return &ValueLoc; + +    // The property was previously set, but the value has been lost. This can +    // happen in various situations, for example: +    // - Because of an environment merge (where the two environments mapped the +    //   property to different values, which resulted in them both being +    //   discarded). +    // - When two blocks in the CFG, with neither a dominator of the other, +    //   visit the same optional value. (FIXME: This is something we can and +    //   should fix -- see also the lengthy FIXME below.) +    // - Or even when a block is revisited during testing to collect +    //   per-statement state. +    // FIXME: This situation means that the optional contents are not shared +    // between branches and the like. Practically, this lack of sharing +    // reduces the precision of the model when the contents are relevant to +    // the check, like another optional or a boolean that influences control +    // flow. +    if (ValueLoc.getType()->isRecordType()) { +      refreshStructValue(cast<AggregateStorageLocation>(ValueLoc), Env); +      return &ValueLoc; +    } else {        auto *ValueVal = Env.createValue(ValueLoc.getType());        if (ValueVal == nullptr)          return nullptr;        Env.setValue(ValueLoc, *ValueVal); +      return &ValueLoc;      } -    return &ValueLoc;    } -  auto Ty = stripReference(Q); -  auto *ValueVal = Env.createValue(Ty); -  if (ValueVal == nullptr) -    return nullptr; -  auto &ValueLoc = Env.createStorageLocation(Ty); -  Env.setValue(ValueLoc, *ValueVal); -  auto ValueRef = std::make_unique<ReferenceValue>(ValueLoc); -  OptionalVal.setProperty("value", Env.takeOwnership(std::move(ValueRef))); +  auto Ty = Q.getNonReferenceType(); +  auto &ValueLoc = Env.createObject(Ty); +  auto &ValuePtr = Env.create<PointerValue>(ValueLoc); +  // FIXME: +  // The change we make to the `value` property below may become visible to +  // other blocks that aren't successors of the current block and therefore +  // don't see the change we made above mapping `ValueLoc` to `ValueVal`. For +  // example: +  // +  //   void target(optional<int> oo, bool b) { +  //     // `oo` is associated with a `StructValue` here, which we will call +  //     // `OptionalVal`. +  // +  //     // The `has_value` property is set on `OptionalVal` (but not the +  //     // `value` property yet). +  //     if (!oo.has_value()) return; +  // +  //     if (b) { +  //       // Let's assume we transfer the `if` branch first. +  //       // +  //       // This causes us to call `maybeInitializeOptionalValueMember()`, +  //       // which causes us to set the `value` property on `OptionalVal` +  //       // (which had not been set until this point). This `value` property +  //       // refers to a `PointerValue`, which in turn refers to a +  //       // StorageLocation` that is associated to an `IntegerValue`. +  //       oo.value(); +  //     } else { +  //       // Let's assume we transfer the `else` branch after the `if` branch. +  //       // +  //       // We see the `value` property that the `if` branch set on +  //       // `OptionalVal`, but in the environment for this block, the +  //       // `StorageLocation` in the `PointerValue` is not associated with any +  //       // `Value`. +  //       oo.value(); +  //     } +  //   } +  // +  // This situation is currently "saved" by the code above that checks whether +  // the `value` property is already set, and if, the `ValueLoc` is not +  // associated with a `ValueVal`, creates a new `ValueVal`. +  // +  // However, what we should really do is to make sure that the change to the +  // `value` property does not "leak" to other blocks that are not successors +  // of this block. To do this, instead of simply setting the `value` property +  // on the existing `OptionalVal`, we should create a new `Value` for the +  // optional, set the property on that, and associate the storage location that +  // is currently associated with the existing `OptionalVal` with the newly +  // created `Value` instead. +  OptionalVal.setProperty("value", ValuePtr);    return &ValueLoc;  } @@ -329,26 +412,34 @@ void initializeOptionalReference(const Expr *OptionalExpr,  }  /// Returns true if and only if `OptionalVal` is initialized and known to be -/// empty in `Env. +/// empty in `Env`.  bool isEmptyOptional(const Value &OptionalVal, const Environment &Env) {    auto *HasValueVal =        cast_or_null<BoolValue>(OptionalVal.getProperty("has_value"));    return HasValueVal != nullptr && -         Env.flowConditionImplies(Env.makeNot(*HasValueVal)); +         Env.flowConditionImplies(Env.arena().makeNot(HasValueVal->formula()));  }  /// Returns true if and only if `OptionalVal` is initialized and known to be -/// non-empty in `Env. +/// non-empty in `Env`.  bool isNonEmptyOptional(const Value &OptionalVal, const Environment &Env) {    auto *HasValueVal =        cast_or_null<BoolValue>(OptionalVal.getProperty("has_value")); -  return HasValueVal != nullptr && Env.flowConditionImplies(*HasValueVal); +  return HasValueVal != nullptr && +         Env.flowConditionImplies(HasValueVal->formula()); +} + +Value *getValueBehindPossiblePointer(const Expr &E, const Environment &Env) { +  Value *Val = Env.getValue(E, SkipPast::Reference); +  if (auto *PointerVal = dyn_cast_or_null<PointerValue>(Val)) +    return Env.getValue(PointerVal->getPointeeLoc()); +  return Val;  }  void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,                          LatticeTransferState &State) {    if (auto *OptionalVal = -          State.Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) { +          getValueBehindPossiblePointer(*ObjectExpr, State.Env)) {      if (State.Env.getStorageLocation(*UnwrapExpr, SkipPast::None) == nullptr)        if (auto *Loc = maybeInitializeOptionalValueMember(                UnwrapExpr->getType(), *OptionalVal, State.Env)) @@ -356,21 +447,31 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,    }  } +void transferArrowOpCall(const Expr *UnwrapExpr, const Expr *ObjectExpr, +                         LatticeTransferState &State) { +  if (auto *OptionalVal = +          getValueBehindPossiblePointer(*ObjectExpr, State.Env)) { +    if (auto *Loc = maybeInitializeOptionalValueMember( +            UnwrapExpr->getType()->getPointeeType(), *OptionalVal, State.Env)) { +      State.Env.setValueStrict(*UnwrapExpr, +                               State.Env.create<PointerValue>(*Loc)); +    } +  } +} +  void transferMakeOptionalCall(const CallExpr *E,                                const MatchFinder::MatchResult &,                                LatticeTransferState &State) { -  auto &Loc = State.Env.createStorageLocation(*E); -  State.Env.setStorageLocation(*E, Loc); -  State.Env.setValue( -      Loc, createOptionalValue(State.Env, State.Env.getBoolLiteralValue(true))); +  createOptionalValue(State.Env.getResultObjectLocation(*E), +                      State.Env.getBoolLiteralValue(true), State.Env);  }  void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,                                    const MatchFinder::MatchResult &,                                    LatticeTransferState &State) {    if (auto *HasValueVal = getHasValue( -          State.Env, State.Env.getValue(*CallExpr->getImplicitObjectArgument(), -                                        SkipPast::ReferenceThenPointer))) { +          State.Env, getValueBehindPossiblePointer( +                         *CallExpr->getImplicitObjectArgument(), State.Env))) {      auto &CallExprLoc = State.Env.createStorageLocation(*CallExpr);      State.Env.setValue(CallExprLoc, *HasValueVal);      State.Env.setStorageLocation(*CallExpr, CallExprLoc); @@ -379,12 +480,11 @@ void transferOptionalHasValueCall(const CXXMemberCallExpr *CallExpr,  /// `ModelPred` builds a logical formula relating the predicate in  /// `ValueOrPredExpr` to the optional's `has_value` property. -void transferValueOrImpl(const clang::Expr *ValueOrPredExpr, -                         const MatchFinder::MatchResult &Result, -                         LatticeTransferState &State, -                         BoolValue &(*ModelPred)(Environment &Env, -                                                 BoolValue &ExprVal, -                                                 BoolValue &HasValueVal)) { +void transferValueOrImpl( +    const clang::Expr *ValueOrPredExpr, const MatchFinder::MatchResult &Result, +    LatticeTransferState &State, +    const Formula &(*ModelPred)(Environment &Env, const Formula &ExprVal, +                                const Formula &HasValueVal)) {    auto &Env = State.Env;    const auto *ObjectArgumentExpr = @@ -392,29 +492,29 @@ void transferValueOrImpl(const clang::Expr *ValueOrPredExpr,            ->getImplicitObjectArgument();    auto *HasValueVal = getHasValue( -      State.Env, -      State.Env.getValue(*ObjectArgumentExpr, SkipPast::ReferenceThenPointer)); +      State.Env, getValueBehindPossiblePointer(*ObjectArgumentExpr, State.Env));    if (HasValueVal == nullptr)      return; -  Env.addToFlowCondition( -      ModelPred(Env, forceBoolValue(Env, *ValueOrPredExpr), *HasValueVal)); +  Env.addToFlowCondition(ModelPred(Env, forceBoolValue(Env, *ValueOrPredExpr), +                                   HasValueVal->formula()));  }  void transferValueOrStringEmptyCall(const clang::Expr *ComparisonExpr,                                      const MatchFinder::MatchResult &Result,                                      LatticeTransferState &State) {    return transferValueOrImpl(ComparisonExpr, Result, State, -                             [](Environment &Env, BoolValue &ExprVal, -                                BoolValue &HasValueVal) -> BoolValue & { +                             [](Environment &Env, const Formula &ExprVal, +                                const Formula &HasValueVal) -> const Formula & { +                               auto &A = Env.arena();                                 // If the result is *not* empty, then we know the                                 // optional must have been holding a value. If                                 // `ExprVal` is true, though, we don't learn                                 // anything definite about `has_value`, so we                                 // don't add any corresponding implications to                                 // the flow condition. -                               return Env.makeImplication(Env.makeNot(ExprVal), -                                                          HasValueVal); +                               return A.makeImplies(A.makeNot(ExprVal), +                                                    HasValueVal);                               });  } @@ -422,12 +522,13 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,                             const MatchFinder::MatchResult &Result,                             LatticeTransferState &State) {    transferValueOrImpl(ComparisonExpr, Result, State, -                      [](Environment &Env, BoolValue &ExprVal, -                         BoolValue &HasValueVal) -> BoolValue & { +                      [](Environment &Env, const Formula &ExprVal, +                         const Formula &HasValueVal) -> const Formula & { +                        auto &A = Env.arena();                          // We know that if `(opt.value_or(X) != X)` then                          // `opt.hasValue()`, even without knowing further                          // details about the contents of `opt`. -                        return Env.makeImplication(ExprVal, HasValueVal); +                        return A.makeImplies(ExprVal, HasValueVal);                        });  } @@ -437,18 +538,21 @@ void transferCallReturningOptional(const CallExpr *E,    if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)      return; -  auto &Loc = State.Env.createStorageLocation(*E); -  State.Env.setStorageLocation(*E, Loc); -  State.Env.setValue( -      Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue())); +  AggregateStorageLocation *Loc = nullptr; +  if (E->isPRValue()) { +    Loc = &State.Env.getResultObjectLocation(*E); +  } else { +    Loc = &cast<AggregateStorageLocation>(State.Env.createStorageLocation(*E)); +    State.Env.setStorageLocationStrict(*E, *Loc); +  } + +  createOptionalValue(*Loc, State.Env.makeAtomicBoolValue(), State.Env);  } -void assignOptionalValue(const Expr &E, Environment &Env, -                         BoolValue &HasValueVal) { -  if (auto *OptionalLoc = -          Env.getStorageLocation(E, SkipPast::ReferenceThenPointer)) { -    Env.setValue(*OptionalLoc, createOptionalValue(Env, HasValueVal)); -  } +void constructOptionalValue(const Expr &E, Environment &Env, +                            BoolValue &HasValueVal) { +  AggregateStorageLocation &Loc = Env.getResultObjectLocation(E); +  Env.setValueStrict(E, createOptionalValue(Loc, HasValueVal, Env));  }  /// Returns a symbolic value for the "has_value" property of an `optional<T>` @@ -460,11 +564,13 @@ BoolValue &valueOrConversionHasValue(const FunctionDecl &F, const Expr &E,    assert(F.getTemplateSpecializationArgs() != nullptr);    assert(F.getTemplateSpecializationArgs()->size() > 0); -  const int TemplateParamOptionalWrappersCount = countOptionalWrappers( -      *MatchRes.Context, -      stripReference(F.getTemplateSpecializationArgs()->get(0).getAsType())); -  const int ArgTypeOptionalWrappersCount = -      countOptionalWrappers(*MatchRes.Context, stripReference(E.getType())); +  const int TemplateParamOptionalWrappersCount = +      countOptionalWrappers(*MatchRes.Context, F.getTemplateSpecializationArgs() +                                                   ->get(0) +                                                   .getAsType() +                                                   .getNonReferenceType()); +  const int ArgTypeOptionalWrappersCount = countOptionalWrappers( +      *MatchRes.Context, E.getType().getNonReferenceType());    // Check if this is a constructor/assignment call for `optional<T>` with    // argument of type `U` such that `T` is constructible from `U`. @@ -484,25 +590,23 @@ void transferValueOrConversionConstructor(      LatticeTransferState &State) {    assert(E->getNumArgs() > 0); -  assignOptionalValue(*E, State.Env, -                      valueOrConversionHasValue(*E->getConstructor(), -                                                *E->getArg(0), MatchRes, -                                                State)); +  constructOptionalValue(*E, State.Env, +                         valueOrConversionHasValue(*E->getConstructor(), +                                                   *E->getArg(0), MatchRes, +                                                   State));  }  void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,                          LatticeTransferState &State) {    assert(E->getNumArgs() > 0); -  auto *OptionalLoc = -      State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference); -  if (OptionalLoc == nullptr) -    return; +  if (auto *Loc = cast<AggregateStorageLocation>( +          State.Env.getStorageLocationStrict(*E->getArg(0)))) { +    createOptionalValue(*Loc, HasValueVal, State.Env); -  State.Env.setValue(*OptionalLoc, createOptionalValue(State.Env, HasValueVal)); - -  // Assign a storage location for the whole expression. -  State.Env.setStorageLocation(*E, *OptionalLoc); +    // Assign a storage location for the whole expression. +    State.Env.setStorageLocationStrict(*E, *Loc); +  }  }  void transferValueOrConversionAssignment( @@ -521,52 +625,69 @@ 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(AggregateStorageLocation *Loc1, +                  AggregateStorageLocation *Loc2, 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 *OptionalVal2 = State.Env.getValue(OptionalLoc2); -  assert(OptionalVal2 != nullptr); +  if (Loc1 == nullptr) { +    if (Loc2 != nullptr) +      createOptionalValue(*Loc2, Env.makeAtomicBoolValue(), Env); +    return; +  } +  if (Loc2 == nullptr) { +    createOptionalValue(*Loc1, Env.makeAtomicBoolValue(), Env); +    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. +  BoolValue *BoolVal1 = getHasValue(Env, Env.getValue(*Loc1)); +  if (BoolVal1 == nullptr) +    BoolVal1 = &Env.makeAtomicBoolValue(); + +  BoolValue *BoolVal2 = getHasValue(Env, Env.getValue(*Loc2)); +  if (BoolVal2 == nullptr) +    BoolVal2 = &Env.makeAtomicBoolValue(); + +  createOptionalValue(*Loc1, *BoolVal2, Env); +  createOptionalValue(*Loc2, *BoolVal1, Env);  }  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); +  auto *OtherLoc = cast_or_null<AggregateStorageLocation>( +      State.Env.getStorageLocationStrict(*E->getArg(0))); +  transferSwap(getImplicitObjectLocation(*E, State.Env), OtherLoc, State.Env);  }  void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,                           LatticeTransferState &State) {    assert(E->getNumArgs() == 2); +  auto *Arg0Loc = cast_or_null<AggregateStorageLocation>( +      State.Env.getStorageLocationStrict(*E->getArg(0))); +  auto *Arg1Loc = cast_or_null<AggregateStorageLocation>( +      State.Env.getStorageLocationStrict(*E->getArg(1))); +  transferSwap(Arg0Loc, Arg1Loc, State.Env); +} -  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); +void transferStdForwardCall(const CallExpr *E, const MatchFinder::MatchResult &, +                            LatticeTransferState &State) { +  assert(E->getNumArgs() == 1); -  transferSwap(*OptionalLoc1, *OptionalLoc2, State); +  if (auto *Loc = State.Env.getStorageLocationStrict(*E->getArg(0))) +    State.Env.setStorageLocationStrict(*E, *Loc);  } -BoolValue &evaluateEquality(Environment &Env, BoolValue &EqVal, BoolValue &LHS, -                            BoolValue &RHS) { +const Formula &evaluateEquality(Arena &A, const Formula &EqVal, +                                const Formula &LHS, const Formula &RHS) {    // Logically, an optional<T> object is composed of two values - a `has_value`    // bit and a value of type T. Equality of optional objects compares both    // values. Therefore, merely comparing the `has_value` bits isn't sufficient: @@ -581,37 +702,38 @@ BoolValue &evaluateEquality(Environment &Env, BoolValue &EqVal, BoolValue &LHS,    // b) (!LHS & !RHS) => EqVal    //    If neither is set, then they are equal.    // We rewrite b) as !EqVal => (LHS v RHS), for a more compact formula. -  return Env.makeAnd( -      Env.makeImplication( -          EqVal, Env.makeOr(Env.makeAnd(LHS, RHS), -                            Env.makeAnd(Env.makeNot(LHS), Env.makeNot(RHS)))), -      Env.makeImplication(Env.makeNot(EqVal), Env.makeOr(LHS, RHS))); +  return A.makeAnd( +      A.makeImplies(EqVal, A.makeOr(A.makeAnd(LHS, RHS), +                                    A.makeAnd(A.makeNot(LHS), A.makeNot(RHS)))), +      A.makeImplies(A.makeNot(EqVal), A.makeOr(LHS, RHS)));  }  void transferOptionalAndOptionalCmp(const clang::CXXOperatorCallExpr *CmpExpr,                                      const MatchFinder::MatchResult &,                                      LatticeTransferState &State) {    Environment &Env = State.Env; +  auto &A = Env.arena();    auto *CmpValue = &forceBoolValue(Env, *CmpExpr);    if (auto *LHasVal = getHasValue(            Env, Env.getValue(*CmpExpr->getArg(0), SkipPast::Reference)))      if (auto *RHasVal = getHasValue(              Env, Env.getValue(*CmpExpr->getArg(1), SkipPast::Reference))) {        if (CmpExpr->getOperator() == clang::OO_ExclaimEqual) -        CmpValue = &State.Env.makeNot(*CmpValue); -      Env.addToFlowCondition( -          evaluateEquality(Env, *CmpValue, *LHasVal, *RHasVal)); +        CmpValue = &A.makeNot(*CmpValue); +      Env.addToFlowCondition(evaluateEquality(A, *CmpValue, LHasVal->formula(), +                                              RHasVal->formula()));      }  }  void transferOptionalAndValueCmp(const clang::CXXOperatorCallExpr *CmpExpr,                                   const clang::Expr *E, Environment &Env) { +  auto &A = Env.arena();    auto *CmpValue = &forceBoolValue(Env, *CmpExpr);    if (auto *HasVal = getHasValue(Env, Env.getValue(*E, SkipPast::Reference))) {      if (CmpExpr->getOperator() == clang::OO_ExclaimEqual) -      CmpValue = &Env.makeNot(*CmpValue); -    Env.addToFlowCondition(evaluateEquality(Env, *CmpValue, *HasVal, -                                            Env.getBoolLiteralValue(true))); +      CmpValue = &A.makeNot(*CmpValue); +    Env.addToFlowCondition( +        evaluateEquality(A, *CmpValue, HasVal->formula(), A.makeLiteral(true)));    }  } @@ -629,7 +751,8 @@ ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) {  StatementMatcher  valueCall(const std::optional<StatementMatcher> &IgnorableOptional) { -  return isOptionalMemberCallWithName("value", IgnorableOptional); +  return isOptionalMemberCallWithNameMatcher(hasName("value"), +                                             IgnorableOptional);  }  StatementMatcher @@ -657,30 +780,29 @@ auto buildTransferMatchSwitch() {            isOptionalInPlaceConstructor(),            [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,               LatticeTransferState &State) { -            assignOptionalValue(*E, State.Env, -                                State.Env.getBoolLiteralValue(true)); +            constructOptionalValue(*E, State.Env, +                                   State.Env.getBoolLiteralValue(true));            })        // nullopt_t::nullopt_t        .CaseOfCFGStmt<CXXConstructExpr>(            isNulloptConstructor(),            [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,               LatticeTransferState &State) { -            assignOptionalValue(*E, State.Env, -                                State.Env.getBoolLiteralValue(false)); +            constructOptionalValue(*E, State.Env, +                                   State.Env.getBoolLiteralValue(false));            })        // optional::optional(nullopt_t)        .CaseOfCFGStmt<CXXConstructExpr>(            isOptionalNulloptConstructor(),            [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,               LatticeTransferState &State) { -            assignOptionalValue(*E, State.Env, -                                State.Env.getBoolLiteralValue(false)); +            constructOptionalValue(*E, State.Env, +                                   State.Env.getBoolLiteralValue(false));            })        // optional::optional (value/conversion)        .CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),                                         transferValueOrConversionConstructor) -        // optional::operator=        .CaseOfCFGStmt<CXXOperatorCallExpr>(            isOptionalValueOrConversionAssignment(), @@ -696,49 +818,70 @@ auto buildTransferMatchSwitch() {              transferUnwrapCall(E, E->getImplicitObjectArgument(), State);            }) -      // optional::operator*, optional::operator-> -      .CaseOfCFGStmt<CallExpr>(valueOperatorCall(std::nullopt), +      // optional::operator* +      .CaseOfCFGStmt<CallExpr>(isOptionalOperatorCallWithName("*"),                                 [](const CallExpr *E,                                    const MatchFinder::MatchResult &,                                    LatticeTransferState &State) {                                   transferUnwrapCall(E, E->getArg(0), State);                                 }) -      // optional::has_value +      // optional::operator-> +      .CaseOfCFGStmt<CallExpr>(isOptionalOperatorCallWithName("->"), +                               [](const CallExpr *E, +                                  const MatchFinder::MatchResult &, +                                  LatticeTransferState &State) { +                                 transferArrowOpCall(E, E->getArg(0), State); +                               }) + +      // optional::has_value, optional::hasValue +      // Of the supported optionals only folly::Optional uses hasValue, but this +      // will also pass for other types        .CaseOfCFGStmt<CXXMemberCallExpr>( -          isOptionalMemberCallWithName("has_value"), +          isOptionalMemberCallWithNameMatcher( +              hasAnyName("has_value", "hasValue")),            transferOptionalHasValueCall)        // optional::operator bool        .CaseOfCFGStmt<CXXMemberCallExpr>( -          isOptionalMemberCallWithName("operator bool"), +          isOptionalMemberCallWithNameMatcher(hasName("operator bool")),            transferOptionalHasValueCall)        // optional::emplace        .CaseOfCFGStmt<CXXMemberCallExpr>( -          isOptionalMemberCallWithName("emplace"), +          isOptionalMemberCallWithNameMatcher(hasName("emplace")),            [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,               LatticeTransferState &State) { -            assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, -                                State.Env.getBoolLiteralValue(true)); +            if (AggregateStorageLocation *Loc = +                    getImplicitObjectLocation(*E, State.Env)) { +              createOptionalValue(*Loc, State.Env.getBoolLiteralValue(true), +                                  State.Env); +            }            })        // optional::reset        .CaseOfCFGStmt<CXXMemberCallExpr>( -          isOptionalMemberCallWithName("reset"), +          isOptionalMemberCallWithNameMatcher(hasName("reset")),            [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,               LatticeTransferState &State) { -            assignOptionalValue(*E->getImplicitObjectArgument(), State.Env, -                                State.Env.getBoolLiteralValue(false)); +            if (AggregateStorageLocation *Loc = +                    getImplicitObjectLocation(*E, State.Env)) { +              createOptionalValue(*Loc, State.Env.getBoolLiteralValue(false), +                                  State.Env); +            }            })        // optional::swap -      .CaseOfCFGStmt<CXXMemberCallExpr>(isOptionalMemberCallWithName("swap"), -                                        transferSwapCall) +      .CaseOfCFGStmt<CXXMemberCallExpr>( +          isOptionalMemberCallWithNameMatcher(hasName("swap")), +          transferSwapCall)        // std::swap        .CaseOfCFGStmt<CallExpr>(isStdSwapCall(), transferStdSwapCall) +      // std::forward +      .CaseOfCFGStmt<CallExpr>(isStdForwardCall(), transferStdForwardCall) +        // opt.value_or("").empty()        .CaseOfCFGStmt<Expr>(isValueOrStringEmptyCall(),                             transferValueOrStringEmptyCall) @@ -772,14 +915,12 @@ auto buildTransferMatchSwitch() {        .Build();  } -std::vector<SourceLocation> diagnoseUnwrapCall(const Expr *UnwrapExpr, -                                               const Expr *ObjectExpr, +std::vector<SourceLocation> diagnoseUnwrapCall(const Expr *ObjectExpr,                                                 const Environment &Env) { -  if (auto *OptionalVal = -          Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) { +  if (auto *OptionalVal = getValueBehindPossiblePointer(*ObjectExpr, Env)) {      auto *Prop = OptionalVal->getProperty("has_value");      if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) { -      if (Env.flowConditionImplies(*HasValueVal)) +      if (Env.flowConditionImplies(HasValueVal->formula()))          return {};      }    } @@ -802,16 +943,16 @@ auto buildDiagnoseMatchSwitch(            valueCall(IgnorableOptional),            [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,               const Environment &Env) { -            return diagnoseUnwrapCall(E, E->getImplicitObjectArgument(), Env); +            return diagnoseUnwrapCall(E->getImplicitObjectArgument(), Env);            })        // optional::operator*, optional::operator-> -      .CaseOfCFGStmt<CallExpr>( -          valueOperatorCall(IgnorableOptional), -          [](const CallExpr *E, const MatchFinder::MatchResult &, -             const Environment &Env) { -            return diagnoseUnwrapCall(E, E->getArg(0), Env); -          }) +      .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional), +                               [](const CallExpr *E, +                                  const MatchFinder::MatchResult &, +                                  const Environment &Env) { +                                 return diagnoseUnwrapCall(E->getArg(0), Env); +                               })        .Build();  } @@ -826,10 +967,10 @@ UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(ASTContext &Ctx)      : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(Ctx),        TransferMatchSwitch(buildTransferMatchSwitch()) {} -void UncheckedOptionalAccessModel::transfer(const CFGElement *Elt, +void UncheckedOptionalAccessModel::transfer(const CFGElement &Elt,                                              NoopLattice &L, Environment &Env) {    LatticeTransferState State(L, Env); -  TransferMatchSwitch(*Elt, getASTContext(), State); +  TransferMatchSwitch(Elt, getASTContext(), State);  }  ComparisonResult UncheckedOptionalAccessModel::compare( @@ -839,10 +980,12 @@ ComparisonResult UncheckedOptionalAccessModel::compare(      return ComparisonResult::Unknown;    bool MustNonEmpty1 = isNonEmptyOptional(Val1, Env1);    bool MustNonEmpty2 = isNonEmptyOptional(Val2, Env2); -  if (MustNonEmpty1 && MustNonEmpty2) return ComparisonResult::Same; +  if (MustNonEmpty1 && MustNonEmpty2) +    return ComparisonResult::Same;    // If exactly one is true, then they're different, no reason to check whether    // they're definitely empty. -  if (MustNonEmpty1 || MustNonEmpty2) return ComparisonResult::Different; +  if (MustNonEmpty1 || MustNonEmpty2) +    return ComparisonResult::Different;    // Check if they're both definitely empty.    return (isEmptyOptional(Val1, Env1) && isEmptyOptional(Val2, Env2))               ? ComparisonResult::Same @@ -863,13 +1006,14 @@ bool UncheckedOptionalAccessModel::merge(QualType Type, const Value &Val1,    bool MustNonEmpty1 = isNonEmptyOptional(Val1, Env1);    bool MustNonEmpty2 = isNonEmptyOptional(Val2, Env2);    if (MustNonEmpty1 && MustNonEmpty2) -    MergedEnv.addToFlowCondition(HasValueVal); +    MergedEnv.addToFlowCondition(HasValueVal.formula());    else if (        // Only make the costly calls to `isEmptyOptional` if we got "unknown"        // (false) for both calls to `isNonEmptyOptional`.        !MustNonEmpty1 && !MustNonEmpty2 && isEmptyOptional(Val1, Env1) &&        isEmptyOptional(Val2, Env2)) -    MergedEnv.addToFlowCondition(MergedEnv.makeNot(HasValueVal)); +    MergedEnv.addToFlowCondition( +        MergedEnv.arena().makeNot(HasValueVal.formula()));    setHasValue(MergedVal, HasValueVal);    return true;  } @@ -892,7 +1036,8 @@ Value *UncheckedOptionalAccessModel::widen(QualType Type, Value &Prev,        if (isa<TopBoolValue>(CurrentHasVal))          return &Current;      } -    return &createOptionalValue(CurrentEnv, CurrentEnv.makeTopBoolValue()); +    return &createOptionalValue(cast<StructValue>(Current).getAggregateLoc(), +                                CurrentEnv.makeTopBoolValue(), CurrentEnv);    case ComparisonResult::Unknown:      return nullptr;    }  | 
