f252333b97
We have WIP Fixables for local variables and this central part of the machinery was dropping Fixables attached to local variables instead of keeping those and dropping everything else. We are in the process of rewriting our patches for emitting fixits after we discovered a conceptual problem in our design. That is why there's currently no tests that would've detected the issue but that will change very shortly.
696 lines
24 KiB
C++
696 lines
24 KiB
C++
//===- UnsafeBufferUsage.cpp - Replace pointers with modern C++ -----------===//
|
|
//
|
|
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
// See https://llvm.org/LICENSE.txt for license information.
|
|
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
#include "clang/Analysis/Analyses/UnsafeBufferUsage.h"
|
|
#include "clang/AST/RecursiveASTVisitor.h"
|
|
#include "clang/ASTMatchers/ASTMatchFinder.h"
|
|
#include "llvm/ADT/SmallVector.h"
|
|
#include <memory>
|
|
#include <optional>
|
|
|
|
using namespace llvm;
|
|
using namespace clang;
|
|
using namespace ast_matchers;
|
|
|
|
namespace clang::ast_matchers {
|
|
// A `RecursiveASTVisitor` that traverses all descendants of a given node "n"
|
|
// except for those belonging to a different callable of "n".
|
|
class MatchDescendantVisitor
|
|
: public RecursiveASTVisitor<MatchDescendantVisitor> {
|
|
public:
|
|
typedef RecursiveASTVisitor<MatchDescendantVisitor> VisitorBase;
|
|
|
|
// Creates an AST visitor that matches `Matcher` on all
|
|
// descendants of a given node "n" except for the ones
|
|
// belonging to a different callable of "n".
|
|
MatchDescendantVisitor(const internal::DynTypedMatcher *Matcher,
|
|
internal::ASTMatchFinder *Finder,
|
|
internal::BoundNodesTreeBuilder *Builder,
|
|
internal::ASTMatchFinder::BindKind Bind)
|
|
: Matcher(Matcher), Finder(Finder), Builder(Builder), Bind(Bind),
|
|
Matches(false) {}
|
|
|
|
// Returns true if a match is found in a subtree of `DynNode`, which belongs
|
|
// to the same callable of `DynNode`.
|
|
bool findMatch(const DynTypedNode &DynNode) {
|
|
Matches = false;
|
|
if (const Stmt *StmtNode = DynNode.get<Stmt>()) {
|
|
TraverseStmt(const_cast<Stmt *>(StmtNode));
|
|
*Builder = ResultBindings;
|
|
return Matches;
|
|
}
|
|
return false;
|
|
}
|
|
|
|
// The following are overriding methods from the base visitor class.
|
|
// They are public only to allow CRTP to work. They are *not *part
|
|
// of the public API of this class.
|
|
|
|
// For the matchers so far used in safe buffers, we only need to match
|
|
// `Stmt`s. To override more as needed.
|
|
|
|
bool TraverseDecl(Decl *Node) {
|
|
if (!Node)
|
|
return true;
|
|
if (!match(*Node))
|
|
return false;
|
|
// To skip callables:
|
|
if (isa<FunctionDecl, BlockDecl, ObjCMethodDecl>(Node))
|
|
return true;
|
|
// Traverse descendants
|
|
return VisitorBase::TraverseDecl(Node);
|
|
}
|
|
|
|
bool TraverseStmt(Stmt *Node, DataRecursionQueue *Queue = nullptr) {
|
|
if (!Node)
|
|
return true;
|
|
if (!match(*Node))
|
|
return false;
|
|
// To skip callables:
|
|
if (isa<LambdaExpr>(Node))
|
|
return true;
|
|
return VisitorBase::TraverseStmt(Node);
|
|
}
|
|
|
|
bool shouldVisitTemplateInstantiations() const { return true; }
|
|
bool shouldVisitImplicitCode() const {
|
|
// TODO: let's ignore implicit code for now
|
|
return false;
|
|
}
|
|
|
|
private:
|
|
// Sets 'Matched' to true if 'Matcher' matches 'Node'
|
|
//
|
|
// Returns 'true' if traversal should continue after this function
|
|
// returns, i.e. if no match is found or 'Bind' is 'BK_All'.
|
|
template <typename T> bool match(const T &Node) {
|
|
internal::BoundNodesTreeBuilder RecursiveBuilder(*Builder);
|
|
|
|
if (Matcher->matches(DynTypedNode::create(Node), Finder,
|
|
&RecursiveBuilder)) {
|
|
ResultBindings.addMatch(RecursiveBuilder);
|
|
Matches = true;
|
|
if (Bind != internal::ASTMatchFinder::BK_All)
|
|
return false; // Abort as soon as a match is found.
|
|
}
|
|
return true;
|
|
}
|
|
|
|
const internal::DynTypedMatcher *const Matcher;
|
|
internal::ASTMatchFinder *const Finder;
|
|
internal::BoundNodesTreeBuilder *const Builder;
|
|
internal::BoundNodesTreeBuilder ResultBindings;
|
|
const internal::ASTMatchFinder::BindKind Bind;
|
|
bool Matches;
|
|
};
|
|
|
|
AST_MATCHER_P(Stmt, forEveryDescendant, internal::Matcher<Stmt>, innerMatcher) {
|
|
const DynTypedMatcher &DTM = static_cast<DynTypedMatcher>(innerMatcher);
|
|
|
|
MatchDescendantVisitor Visitor(&DTM, Finder, Builder, ASTMatchFinder::BK_All);
|
|
return Visitor.findMatch(DynTypedNode::create(Node));
|
|
}
|
|
} // namespace clang::ast_matchers
|
|
|
|
namespace {
|
|
// Because the analysis revolves around variables and their types, we'll need to
|
|
// track uses of variables (aka DeclRefExprs).
|
|
using DeclUseList = SmallVector<const DeclRefExpr *, 1>;
|
|
|
|
// Convenience typedef.
|
|
using FixItList = SmallVector<FixItHint, 4>;
|
|
|
|
// Defined below.
|
|
class Strategy;
|
|
} // namespace
|
|
|
|
// Because we're dealing with raw pointers, let's define what we mean by that.
|
|
static auto hasPointerType() {
|
|
return hasType(hasCanonicalType(pointerType()));
|
|
}
|
|
|
|
static auto hasArrayType() {
|
|
return hasType(hasCanonicalType(arrayType()));
|
|
}
|
|
|
|
namespace {
|
|
/// Gadget is an individual operation in the code that may be of interest to
|
|
/// this analysis. Each (non-abstract) subclass corresponds to a specific
|
|
/// rigid AST structure that constitutes an operation on a pointer-type object.
|
|
/// Discovery of a gadget in the code corresponds to claiming that we understand
|
|
/// what this part of code is doing well enough to potentially improve it.
|
|
/// Gadgets can be warning (immediately deserving a warning) or fixable (not
|
|
/// always deserving a warning per se, but requires our attention to identify
|
|
/// it warrants a fixit).
|
|
class Gadget {
|
|
public:
|
|
enum class Kind {
|
|
#define GADGET(x) x,
|
|
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
|
|
};
|
|
|
|
/// Common type of ASTMatchers used for discovering gadgets.
|
|
/// Useful for implementing the static matcher() methods
|
|
/// that are expected from all non-abstract subclasses.
|
|
using Matcher = decltype(stmt());
|
|
|
|
Gadget(Kind K) : K(K) {}
|
|
|
|
Kind getKind() const { return K; }
|
|
|
|
virtual bool isWarningGadget() const = 0;
|
|
virtual const Stmt *getBaseStmt() const = 0;
|
|
|
|
/// Returns the list of pointer-type variables on which this gadget performs
|
|
/// its operation. Typically, there's only one variable. This isn't a list
|
|
/// of all DeclRefExprs in the gadget's AST!
|
|
virtual DeclUseList getClaimedVarUseSites() const = 0;
|
|
|
|
virtual ~Gadget() = default;
|
|
|
|
private:
|
|
Kind K;
|
|
};
|
|
|
|
|
|
/// Warning gadgets correspond to unsafe code patterns that warrants
|
|
/// an immediate warning.
|
|
class WarningGadget : public Gadget {
|
|
public:
|
|
WarningGadget(Kind K) : Gadget(K) {}
|
|
|
|
static bool classof(const Gadget *G) { return G->isWarningGadget(); }
|
|
bool isWarningGadget() const final { return true; }
|
|
};
|
|
|
|
/// Fixable gadgets correspond to code patterns that aren't always unsafe but need to be
|
|
/// properly recognized in order to emit fixes. For example, if a raw pointer-type
|
|
/// variable is replaced by a safe C++ container, every use of such variable must be
|
|
/// carefully considered and possibly updated.
|
|
class FixableGadget : public Gadget {
|
|
public:
|
|
FixableGadget(Kind K) : Gadget(K) {}
|
|
|
|
static bool classof(const Gadget *G) { return !G->isWarningGadget(); }
|
|
bool isWarningGadget() const final { return false; }
|
|
|
|
/// Returns a fixit that would fix the current gadget according to
|
|
/// the current strategy. Returns None if the fix cannot be produced;
|
|
/// returns an empty list if no fixes are necessary.
|
|
virtual std::optional<FixItList> getFixits(const Strategy &) const {
|
|
return std::nullopt;
|
|
}
|
|
};
|
|
|
|
using FixableGadgetList = std::vector<std::unique_ptr<FixableGadget>>;
|
|
using WarningGadgetList = std::vector<std::unique_ptr<WarningGadget>>;
|
|
|
|
/// An increment of a pointer-type value is unsafe as it may run the pointer
|
|
/// out of bounds.
|
|
class IncrementGadget : public WarningGadget {
|
|
static constexpr const char *const OpTag = "op";
|
|
const UnaryOperator *Op;
|
|
|
|
public:
|
|
IncrementGadget(const MatchFinder::MatchResult &Result)
|
|
: WarningGadget(Kind::Increment),
|
|
Op(Result.Nodes.getNodeAs<UnaryOperator>(OpTag)) {}
|
|
|
|
static bool classof(const Gadget *G) {
|
|
return G->getKind() == Kind::Increment;
|
|
}
|
|
|
|
static Matcher matcher() {
|
|
return stmt(unaryOperator(
|
|
hasOperatorName("++"),
|
|
hasUnaryOperand(ignoringParenImpCasts(hasPointerType()))
|
|
).bind(OpTag));
|
|
}
|
|
|
|
const UnaryOperator *getBaseStmt() const override { return Op; }
|
|
|
|
DeclUseList getClaimedVarUseSites() const override {
|
|
SmallVector<const DeclRefExpr *, 2> Uses;
|
|
if (const auto *DRE =
|
|
dyn_cast<DeclRefExpr>(Op->getSubExpr()->IgnoreParenImpCasts())) {
|
|
Uses.push_back(DRE);
|
|
}
|
|
|
|
return std::move(Uses);
|
|
}
|
|
};
|
|
|
|
/// A decrement of a pointer-type value is unsafe as it may run the pointer
|
|
/// out of bounds.
|
|
class DecrementGadget : public WarningGadget {
|
|
static constexpr const char *const OpTag = "op";
|
|
const UnaryOperator *Op;
|
|
|
|
public:
|
|
DecrementGadget(const MatchFinder::MatchResult &Result)
|
|
: WarningGadget(Kind::Decrement),
|
|
Op(Result.Nodes.getNodeAs<UnaryOperator>(OpTag)) {}
|
|
|
|
static bool classof(const Gadget *G) {
|
|
return G->getKind() == Kind::Decrement;
|
|
}
|
|
|
|
static Matcher matcher() {
|
|
return stmt(unaryOperator(
|
|
hasOperatorName("--"),
|
|
hasUnaryOperand(ignoringParenImpCasts(hasPointerType()))
|
|
).bind(OpTag));
|
|
}
|
|
|
|
const UnaryOperator *getBaseStmt() const override { return Op; }
|
|
|
|
DeclUseList getClaimedVarUseSites() const override {
|
|
if (const auto *DRE =
|
|
dyn_cast<DeclRefExpr>(Op->getSubExpr()->IgnoreParenImpCasts())) {
|
|
return {DRE};
|
|
}
|
|
|
|
return {};
|
|
}
|
|
};
|
|
|
|
/// Array subscript expressions on raw pointers as if they're arrays. Unsafe as
|
|
/// it doesn't have any bounds checks for the array.
|
|
class ArraySubscriptGadget : public WarningGadget {
|
|
static constexpr const char *const ArraySubscrTag = "arraySubscr";
|
|
const ArraySubscriptExpr *ASE;
|
|
|
|
public:
|
|
ArraySubscriptGadget(const MatchFinder::MatchResult &Result)
|
|
: WarningGadget(Kind::ArraySubscript),
|
|
ASE(Result.Nodes.getNodeAs<ArraySubscriptExpr>(ArraySubscrTag)) {}
|
|
|
|
static bool classof(const Gadget *G) {
|
|
return G->getKind() == Kind::ArraySubscript;
|
|
}
|
|
|
|
static Matcher matcher() {
|
|
// FIXME: What if the index is integer literal 0? Should this be
|
|
// a safe gadget in this case?
|
|
// clang-format off
|
|
return stmt(arraySubscriptExpr(
|
|
hasBase(ignoringParenImpCasts(
|
|
anyOf(hasPointerType(), hasArrayType()))),
|
|
unless(hasIndex(integerLiteral(equals(0)))))
|
|
.bind(ArraySubscrTag));
|
|
// clang-format on
|
|
}
|
|
|
|
const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
|
|
|
|
DeclUseList getClaimedVarUseSites() const override {
|
|
if (const auto *DRE =
|
|
dyn_cast<DeclRefExpr>(ASE->getBase()->IgnoreParenImpCasts())) {
|
|
return {DRE};
|
|
}
|
|
|
|
return {};
|
|
}
|
|
};
|
|
|
|
/// A pointer arithmetic expression of one of the forms:
|
|
/// \code
|
|
/// ptr + n | n + ptr | ptr - n | ptr += n | ptr -= n
|
|
/// \endcode
|
|
class PointerArithmeticGadget : public WarningGadget {
|
|
static constexpr const char *const PointerArithmeticTag = "ptrAdd";
|
|
static constexpr const char *const PointerArithmeticPointerTag = "ptrAddPtr";
|
|
const BinaryOperator *PA; // pointer arithmetic expression
|
|
const Expr * Ptr; // the pointer expression in `PA`
|
|
|
|
public:
|
|
PointerArithmeticGadget(const MatchFinder::MatchResult &Result)
|
|
: WarningGadget(Kind::PointerArithmetic),
|
|
PA(Result.Nodes.getNodeAs<BinaryOperator>(PointerArithmeticTag)),
|
|
Ptr(Result.Nodes.getNodeAs<Expr>(PointerArithmeticPointerTag)) {}
|
|
|
|
static bool classof(const Gadget *G) {
|
|
return G->getKind() == Kind::PointerArithmetic;
|
|
}
|
|
|
|
static Matcher matcher() {
|
|
auto HasIntegerType = anyOf(
|
|
hasType(isInteger()), hasType(enumType()));
|
|
auto PtrAtRight = allOf(hasOperatorName("+"),
|
|
hasRHS(expr(hasPointerType()).bind(PointerArithmeticPointerTag)),
|
|
hasLHS(HasIntegerType));
|
|
auto PtrAtLeft = allOf(
|
|
anyOf(hasOperatorName("+"), hasOperatorName("-"),
|
|
hasOperatorName("+="), hasOperatorName("-=")),
|
|
hasLHS(expr(hasPointerType()).bind(PointerArithmeticPointerTag)),
|
|
hasRHS(HasIntegerType));
|
|
|
|
return stmt(binaryOperator(anyOf(PtrAtLeft, PtrAtRight)).bind(PointerArithmeticTag));
|
|
}
|
|
|
|
const Stmt *getBaseStmt() const override { return PA; }
|
|
|
|
DeclUseList getClaimedVarUseSites() const override {
|
|
if (const auto *DRE =
|
|
dyn_cast<DeclRefExpr>(Ptr->IgnoreParenImpCasts())) {
|
|
return {DRE};
|
|
}
|
|
|
|
return {};
|
|
}
|
|
// FIXME: pointer adding zero should be fine
|
|
//FIXME: this gadge will need a fix-it
|
|
};
|
|
} // namespace
|
|
|
|
namespace {
|
|
// An auxiliary tracking facility for the fixit analysis. It helps connect
|
|
// declarations to its and make sure we've covered all uses with our analysis
|
|
// before we try to fix the declaration.
|
|
class DeclUseTracker {
|
|
using UseSetTy = SmallSet<const DeclRefExpr *, 16>;
|
|
using DefMapTy = DenseMap<const VarDecl *, const DeclStmt *>;
|
|
|
|
// Allocate on the heap for easier move.
|
|
std::unique_ptr<UseSetTy> Uses{std::make_unique<UseSetTy>()};
|
|
DefMapTy Defs{};
|
|
|
|
public:
|
|
DeclUseTracker() = default;
|
|
DeclUseTracker(const DeclUseTracker &) = delete; // Let's avoid copies.
|
|
DeclUseTracker(DeclUseTracker &&) = default;
|
|
DeclUseTracker &operator=(DeclUseTracker &&) = default;
|
|
|
|
// Start tracking a freshly discovered DRE.
|
|
void discoverUse(const DeclRefExpr *DRE) { Uses->insert(DRE); }
|
|
|
|
// Stop tracking the DRE as it's been fully figured out.
|
|
void claimUse(const DeclRefExpr *DRE) {
|
|
assert(Uses->count(DRE) &&
|
|
"DRE not found or claimed by multiple matchers!");
|
|
Uses->erase(DRE);
|
|
}
|
|
|
|
// A variable is unclaimed if at least one use is unclaimed.
|
|
bool hasUnclaimedUses(const VarDecl *VD) const {
|
|
// FIXME: Can this be less linear? Maybe maintain a map from VDs to DREs?
|
|
return any_of(*Uses, [VD](const DeclRefExpr *DRE) {
|
|
return DRE->getDecl()->getCanonicalDecl() == VD->getCanonicalDecl();
|
|
});
|
|
}
|
|
|
|
void discoverDecl(const DeclStmt *DS) {
|
|
for (const Decl *D : DS->decls()) {
|
|
if (const auto *VD = dyn_cast<VarDecl>(D)) {
|
|
// FIXME: Assertion temporarily disabled due to a bug in
|
|
// ASTMatcher internal behavior in presence of GNU
|
|
// statement-expressions. We need to properly investigate this
|
|
// because it can screw up our algorithm in other ways.
|
|
// assert(Defs.count(VD) == 0 && "Definition already discovered!");
|
|
Defs[VD] = DS;
|
|
}
|
|
}
|
|
}
|
|
|
|
const DeclStmt *lookupDecl(const VarDecl *VD) const {
|
|
auto It = Defs.find(VD);
|
|
assert(It != Defs.end() && "Definition never discovered!");
|
|
return It->second;
|
|
}
|
|
};
|
|
} // namespace
|
|
|
|
namespace {
|
|
// Strategy is a map from variables to the way we plan to emit fixes for
|
|
// these variables. It is figured out gradually by trying different fixes
|
|
// for different variables depending on gadgets in which these variables
|
|
// participate.
|
|
class Strategy {
|
|
public:
|
|
enum class Kind {
|
|
Wontfix, // We don't plan to emit a fixit for this variable.
|
|
Span, // We recommend replacing the variable with std::span.
|
|
Iterator, // We recommend replacing the variable with std::span::iterator.
|
|
Array, // We recommend replacing the variable with std::array.
|
|
Vector // We recommend replacing the variable with std::vector.
|
|
};
|
|
|
|
private:
|
|
using MapTy = llvm::DenseMap<const VarDecl *, Kind>;
|
|
|
|
MapTy Map;
|
|
|
|
public:
|
|
Strategy() = default;
|
|
Strategy(const Strategy &) = delete; // Let's avoid copies.
|
|
Strategy(Strategy &&) = default;
|
|
|
|
void set(const VarDecl *VD, Kind K) {
|
|
Map[VD] = K;
|
|
}
|
|
|
|
Kind lookup(const VarDecl *VD) const {
|
|
auto I = Map.find(VD);
|
|
if (I == Map.end())
|
|
return Kind::Wontfix;
|
|
|
|
return I->second;
|
|
}
|
|
};
|
|
} // namespace
|
|
|
|
/// Scan the function and return a list of gadgets found with provided kits.
|
|
static std::tuple<FixableGadgetList, WarningGadgetList, DeclUseTracker> findGadgets(const Decl *D) {
|
|
|
|
struct GadgetFinderCallback : MatchFinder::MatchCallback {
|
|
FixableGadgetList FixableGadgets;
|
|
WarningGadgetList WarningGadgets;
|
|
DeclUseTracker Tracker;
|
|
|
|
void run(const MatchFinder::MatchResult &Result) override {
|
|
// In debug mode, assert that we've found exactly one gadget.
|
|
// This helps us avoid conflicts in .bind() tags.
|
|
#if NDEBUG
|
|
#define NEXT return
|
|
#else
|
|
[[maybe_unused]] int numFound = 0;
|
|
#define NEXT ++numFound
|
|
#endif
|
|
|
|
if (const auto *DRE = Result.Nodes.getNodeAs<DeclRefExpr>("any_dre")) {
|
|
Tracker.discoverUse(DRE);
|
|
NEXT;
|
|
}
|
|
|
|
if (const auto *DS = Result.Nodes.getNodeAs<DeclStmt>("any_ds")) {
|
|
Tracker.discoverDecl(DS);
|
|
NEXT;
|
|
}
|
|
|
|
// Figure out which matcher we've found, and call the appropriate
|
|
// subclass constructor.
|
|
// FIXME: Can we do this more logarithmically?
|
|
#define FIXABLE_GADGET(name) \
|
|
if (Result.Nodes.getNodeAs<Stmt>(#name)) { \
|
|
FixableGadgets.push_back(std::make_unique<name ## Gadget>(Result)); \
|
|
NEXT; \
|
|
}
|
|
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
|
|
#define WARNING_GADGET(name) \
|
|
if (Result.Nodes.getNodeAs<Stmt>(#name)) { \
|
|
WarningGadgets.push_back(std::make_unique<name ## Gadget>(Result)); \
|
|
NEXT; \
|
|
}
|
|
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
|
|
|
|
assert(numFound >= 1 && "Gadgets not found in match result!");
|
|
assert(numFound <= 1 && "Conflicting bind tags in gadgets!");
|
|
}
|
|
};
|
|
|
|
MatchFinder M;
|
|
GadgetFinderCallback CB;
|
|
|
|
// clang-format off
|
|
M.addMatcher(
|
|
stmt(forEveryDescendant(
|
|
stmt(anyOf(
|
|
// Add Gadget::matcher() for every gadget in the registry.
|
|
#define GADGET(x) \
|
|
x ## Gadget::matcher().bind(#x),
|
|
#include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
|
|
// In parallel, match all DeclRefExprs so that to find out
|
|
// whether there are any uncovered by gadgets.
|
|
declRefExpr(anyOf(hasPointerType(), hasArrayType()),
|
|
to(varDecl())).bind("any_dre"),
|
|
// Also match DeclStmts because we'll need them when fixing
|
|
// their underlying VarDecls that otherwise don't have
|
|
// any backreferences to DeclStmts.
|
|
declStmt().bind("any_ds")
|
|
))
|
|
// FIXME: Idiomatically there should be a forCallable(equalsNode(D))
|
|
// here, to make sure that the statement actually belongs to the
|
|
// function and not to a nested function. However, forCallable uses
|
|
// ParentMap which can't be used before the AST is fully constructed.
|
|
// The original problem doesn't sound like it needs ParentMap though,
|
|
// maybe there's a more direct solution?
|
|
)),
|
|
&CB
|
|
);
|
|
// clang-format on
|
|
|
|
M.match(*D->getBody(), D->getASTContext());
|
|
|
|
// Gadgets "claim" variables they're responsible for. Once this loop finishes,
|
|
// the tracker will only track DREs that weren't claimed by any gadgets,
|
|
// i.e. not understood by the analysis.
|
|
for (const auto &G : CB.FixableGadgets) {
|
|
for (const auto *DRE : G->getClaimedVarUseSites()) {
|
|
CB.Tracker.claimUse(DRE);
|
|
}
|
|
}
|
|
|
|
return {std::move(CB.FixableGadgets), std::move(CB.WarningGadgets), std::move(CB.Tracker)};
|
|
}
|
|
|
|
struct WarningGadgetSets {
|
|
std::map<const VarDecl *, std::set<std::unique_ptr<WarningGadget>>> byVar;
|
|
// These Gadgets are not related to pointer variables (e. g. temporaries).
|
|
llvm::SmallVector<std::unique_ptr<WarningGadget>, 16> noVar;
|
|
};
|
|
|
|
static WarningGadgetSets
|
|
groupWarningGadgetsByVar(WarningGadgetList &&AllUnsafeOperations) {
|
|
WarningGadgetSets result;
|
|
// If some gadgets cover more than one
|
|
// variable, they'll appear more than once in the map.
|
|
for (auto &G : AllUnsafeOperations) {
|
|
DeclUseList ClaimedVarUseSites = G->getClaimedVarUseSites();
|
|
|
|
bool AssociatedWithVarDecl = false;
|
|
for (const DeclRefExpr *DRE : ClaimedVarUseSites) {
|
|
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
|
|
result.byVar[VD].emplace(std::move(G));
|
|
AssociatedWithVarDecl = true;
|
|
}
|
|
}
|
|
|
|
if (!AssociatedWithVarDecl) {
|
|
result.noVar.emplace_back(std::move(G));
|
|
continue;
|
|
}
|
|
}
|
|
return result;
|
|
}
|
|
|
|
struct FixableGadgetSets {
|
|
std::map<const VarDecl *, std::set<std::unique_ptr<FixableGadget>>> byVar;
|
|
};
|
|
|
|
static FixableGadgetSets
|
|
groupFixablesByVar(FixableGadgetList &&AllFixableOperations) {
|
|
FixableGadgetSets FixablesForUnsafeVars;
|
|
for (auto &F : AllFixableOperations) {
|
|
DeclUseList DREs = F->getClaimedVarUseSites();
|
|
|
|
for (const DeclRefExpr *DRE : DREs) {
|
|
if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
|
|
FixablesForUnsafeVars.byVar[VD].emplace(std::move(F));
|
|
}
|
|
}
|
|
}
|
|
return FixablesForUnsafeVars;
|
|
}
|
|
|
|
static std::map<const VarDecl *, FixItList>
|
|
getFixIts(FixableGadgetSets &FixablesForUnsafeVars, const Strategy &S) {
|
|
std::map<const VarDecl *, FixItList> FixItsForVariable;
|
|
for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
|
|
// TODO fixVariable - fixit for the variable itself
|
|
bool ImpossibleToFix = false;
|
|
llvm::SmallVector<FixItHint, 16> FixItsForVD;
|
|
for (const auto &F : Fixables) {
|
|
llvm::Optional<FixItList> Fixits = F->getFixits(S);
|
|
if (!Fixits) {
|
|
ImpossibleToFix = true;
|
|
break;
|
|
} else {
|
|
const FixItList CorrectFixes = Fixits.value();
|
|
FixItsForVD.insert(FixItsForVD.end(), CorrectFixes.begin(),
|
|
CorrectFixes.end());
|
|
}
|
|
}
|
|
if (ImpossibleToFix)
|
|
FixItsForVariable.erase(VD);
|
|
else
|
|
FixItsForVariable[VD].insert(FixItsForVariable[VD].end(),
|
|
FixItsForVD.begin(), FixItsForVD.end());
|
|
}
|
|
return FixItsForVariable;
|
|
}
|
|
|
|
static Strategy
|
|
getNaiveStrategy(const llvm::SmallVectorImpl<const VarDecl *> &UnsafeVars) {
|
|
Strategy S;
|
|
for (const VarDecl *VD : UnsafeVars) {
|
|
S.set(VD, Strategy::Kind::Span);
|
|
}
|
|
return S;
|
|
}
|
|
|
|
void clang::checkUnsafeBufferUsage(const Decl *D,
|
|
UnsafeBufferUsageHandler &Handler) {
|
|
assert(D && D->getBody());
|
|
|
|
WarningGadgetSets UnsafeOps;
|
|
FixableGadgetSets FixablesForUnsafeVars;
|
|
DeclUseTracker Tracker;
|
|
|
|
{
|
|
auto [FixableGadgets, WarningGadgets, TrackerRes] = findGadgets(D);
|
|
UnsafeOps = groupWarningGadgetsByVar(std::move(WarningGadgets));
|
|
FixablesForUnsafeVars = groupFixablesByVar(std::move(FixableGadgets));
|
|
Tracker = std::move(TrackerRes);
|
|
}
|
|
|
|
// Filter out non-local vars and vars with unclaimed DeclRefExpr-s.
|
|
for (auto it = FixablesForUnsafeVars.byVar.cbegin();
|
|
it != FixablesForUnsafeVars.byVar.cend();) {
|
|
// FIXME: Support ParmVarDecl as well.
|
|
if (!it->first->isLocalVarDecl() || Tracker.hasUnclaimedUses(it->first)) {
|
|
it = FixablesForUnsafeVars.byVar.erase(it);
|
|
} else {
|
|
++it;
|
|
}
|
|
}
|
|
|
|
llvm::SmallVector<const VarDecl *, 16> UnsafeVars;
|
|
for (const auto &[VD, ignore] : FixablesForUnsafeVars.byVar)
|
|
UnsafeVars.push_back(VD);
|
|
|
|
Strategy NaiveStrategy = getNaiveStrategy(UnsafeVars);
|
|
std::map<const VarDecl *, FixItList> FixItsForVariable =
|
|
getFixIts(FixablesForUnsafeVars, NaiveStrategy);
|
|
|
|
// FIXME Detect overlapping FixIts.
|
|
|
|
for (const auto &G : UnsafeOps.noVar) {
|
|
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false);
|
|
}
|
|
|
|
for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {
|
|
auto FixItsIt = FixItsForVariable.find(VD);
|
|
Handler.handleFixableVariable(VD, FixItsIt != FixItsForVariable.end()
|
|
? std::move(FixItsIt->second)
|
|
: FixItList{});
|
|
for (const auto &G : WarningGadgets) {
|
|
Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true);
|
|
}
|
|
}
|
|
}
|