diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp index 5c38255d2b839..bf28a76eb42ac 100644 --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -29,6 +29,7 @@ #include "clang/Lex/Preprocessor.h" #include "llvm/ADT/APInt.h" #include "llvm/ADT/APSInt.h" +#include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallSet.h" #include "llvm/ADT/SmallVector.h" @@ -426,8 +427,9 @@ const Expr *findCountArg(const Expr *Count, const CallExpr *Call) { return Call->getArg(Index); } -// Mapping: dependent decl -> value. -using DependentValuesTy = llvm::DenseMap; +// Mapping: (DependentDecl, Deref) -> Value. +using DeclDerefPair = llvm::PointerIntPair; +using DependentValuesTy = llvm::DenseMap; // Given the call expr, find the mapping from the dependent parameter to the // argument that is passed to that parameter. @@ -445,7 +447,8 @@ getDependentValuesFromCall(const CountAttributedType *CAT, return std::nullopt; const Expr *Arg = Call->getArg(Index); - [[maybe_unused]] bool Inserted = Values.insert({PVD, Arg}).second; + [[maybe_unused]] bool Inserted = + Values.insert({{PVD, /*Deref=*/false}, Arg}).second; assert(Inserted); } return {std::move(Values)}; @@ -499,13 +502,16 @@ struct CompatibleCountExprVisitor const Expr * trySubstituteAndSimplify(const Expr *E, bool &hasBeenSubstituted, const DependentValuesTy *DependentValues) const { + auto trySubstitute = [&](const ValueDecl *VD, bool Deref) -> const Expr * { + if (hasBeenSubstituted || !DependentValues) + return nullptr; + auto It = DependentValues->find({VD, Deref}); + return It != DependentValues->end() ? It->second : nullptr; + }; + // Attempts to simplify `E`: if `E` has the form `*&e`, return `e`; // return `E` without change otherwise: - auto trySimplifyDerefAddressof = - [](const Expr *E, - const DependentValuesTy - *DependentValues, // Deref may need subsitution - bool &hasBeenSubstituted) -> const Expr * { + auto trySimplifyDeref = [&](const Expr *E) -> const Expr * { const auto *Deref = dyn_cast(E->IgnoreParenImpCasts()); if (!Deref || Deref->getOpcode() != UO_Deref) @@ -513,36 +519,40 @@ struct CompatibleCountExprVisitor const Expr *DerefOperand = Deref->getSubExpr()->IgnoreParenImpCasts(); + // Just simplify `*&...`. if (const auto *UO = dyn_cast(DerefOperand)) if (UO->getOpcode() == UO_AddrOf) return UO->getSubExpr(); + if (const auto *DRE = dyn_cast(DerefOperand)) { - if (!DependentValues || hasBeenSubstituted) - return E; - - if (auto I = DependentValues->find(DRE->getDecl()); - I != DependentValues->end()) - if (const auto *UO = dyn_cast( - I->getSecond()->IgnoreParenImpCasts())) - if (UO->getOpcode() == UO_AddrOf) { - hasBeenSubstituted = true; - return UO->getSubExpr(); - } + // Substitute `*x`. + if (const auto *Sub = trySubstitute(DRE->getDecl(), /*Deref=*/true)) { + hasBeenSubstituted = true; + return Sub; + } + + // Substitute `x` in `*x` if we have `x -> &...` in our mapping. + if (const auto *Sub = trySubstitute(DRE->getDecl(), /*Deref=*/false)) { + if (const auto *UO = + dyn_cast(Sub->IgnoreParenImpCasts()); + UO && UO->getOpcode() == UO_AddrOf) { + hasBeenSubstituted = true; + return UO->getSubExpr(); + } + } } + return E; }; - if (!hasBeenSubstituted && DependentValues) { - if (const auto *DRE = dyn_cast(E->IgnoreParenImpCasts())) { - if (auto It = DependentValues->find(DRE->getDecl()); - It != DependentValues->end()) { - hasBeenSubstituted = true; - return trySimplifyDerefAddressof(It->second, nullptr, - hasBeenSubstituted); - } + if (const auto *DRE = dyn_cast(E->IgnoreParenImpCasts())) { + if (const auto *Sub = trySubstitute(DRE->getDecl(), /*Deref=*/false)) { + hasBeenSubstituted = true; + return trySimplifyDeref(Sub); } } - return trySimplifyDerefAddressof(E, DependentValues, hasBeenSubstituted); + + return trySimplifyDeref(E); } explicit CompatibleCountExprVisitor( @@ -673,6 +683,13 @@ struct CompatibleCountExprVisitor bool hasOtherBeenSubstituted) { if (SelfUO->getOpcode() != UO_Deref) return false; // We don't support any other unary operator + + const auto *SimplifiedSelf = trySubstituteAndSimplify( + SelfUO, hasSelfBeenSubstituted, DependentValuesSelf); + if (SimplifiedSelf != SelfUO) + return Visit(SimplifiedSelf, Other, hasSelfBeenSubstituted, + hasOtherBeenSubstituted); + Other = trySubstituteAndSimplify(Other, hasOtherBeenSubstituted, DependentValuesOther); if (const auto *OtherUO = @@ -681,13 +698,7 @@ struct CompatibleCountExprVisitor return Visit(SelfUO->getSubExpr(), OtherUO->getSubExpr(), hasSelfBeenSubstituted, hasOtherBeenSubstituted); } - // If `Other` is not a dereference expression, try to simplify `SelfUO`: - const auto *SimplifiedSelf = trySubstituteAndSimplify( - SelfUO, hasSelfBeenSubstituted, DependentValuesSelf); - if (SimplifiedSelf != SelfUO) - return Visit(SimplifiedSelf, Other, hasSelfBeenSubstituted, - hasOtherBeenSubstituted); return false; }