forked from OSchip/llvm-project
[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it
Fix a compilation error due to a missing 'template' keyword. Differential Revision: https://reviews.llvm.org/D108695
This commit is contained in:
parent
d353d1c501
commit
0213d7ec0c
|
@ -633,6 +633,9 @@ class CXXConstructorCall;
|
|||
/// Descendants can define what a "state change is", like a change of value
|
||||
/// to a memory region, liveness, etc. For function calls where the state did
|
||||
/// not change as defined, a custom note may be constructed.
|
||||
///
|
||||
/// For a minimal example, check out
|
||||
/// clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp.
|
||||
class NoStateChangeFuncVisitor : public BugReporterVisitor {
|
||||
private:
|
||||
/// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
|
||||
|
@ -643,6 +646,8 @@ private:
|
|||
/// many times (going up the path for each node and checking whether the
|
||||
/// region was written into) we instead lazily compute the stack frames
|
||||
/// along the path.
|
||||
// TODO: Can't we just use a map instead? This is likely not as cheap as it
|
||||
// makes the code difficult to read.
|
||||
llvm::SmallPtrSet<const StackFrameContext *, 32> FramesModifying;
|
||||
llvm::SmallPtrSet<const StackFrameContext *, 32> FramesModifyingCalculated;
|
||||
|
||||
|
@ -651,6 +656,8 @@ private:
|
|||
/// The calculation is cached in FramesModifying.
|
||||
bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
|
||||
|
||||
void markFrameAsModifying(const StackFrameContext *SCtx);
|
||||
|
||||
/// Write to \c FramesModifying all stack frames along the path in the current
|
||||
/// stack frame which modifies the state.
|
||||
void findModifyingFrames(const ExplodedNode *const CallExitBeginN);
|
||||
|
@ -658,11 +665,37 @@ private:
|
|||
protected:
|
||||
bugreporter::TrackingKind TKind;
|
||||
|
||||
/// \return Whether the state was modified from the current node, \CurrN, to
|
||||
/// the end of the stack fram, at \p CallExitBeginN.
|
||||
virtual bool
|
||||
wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
|
||||
const ExplodedNode *CallExitBeginN) = 0;
|
||||
/// \return Whether the state was modified from the current node, \p CurrN, to
|
||||
/// the end of the stack frame, at \p CallExitBeginN. \p CurrN and
|
||||
/// \p CallExitBeginN are always in the same stack frame.
|
||||
/// Clients should override this callback when a state change is important
|
||||
/// not only on the entire function call, but inside of it as well.
|
||||
/// Example: we may want to leave a note about the lack of locking/unlocking
|
||||
/// on a particular mutex, but not if inside the function its state was
|
||||
/// changed, but also restored. wasModifiedInFunction() wouldn't know of this
|
||||
/// change.
|
||||
virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
|
||||
const ExplodedNode *CallExitBeginN) {
|
||||
return false;
|
||||
}
|
||||
|
||||
/// \return Whether the state was modified in the inlined function call in
|
||||
/// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame
|
||||
/// retrieved from a CallEnterN and CallExitEndN is the *caller's* stack
|
||||
/// frame! The inlined function's stack should be retrieved from either the
|
||||
/// immediate successor to \p CallEnterN or immediate predecessor to
|
||||
/// \p CallExitEndN.
|
||||
/// Clients should override this function if a state changes local to the
|
||||
/// inlined function are not interesting, only the change occuring as a
|
||||
/// result of it.
|
||||
/// Example: we want to leave a not about a leaked resource object not being
|
||||
/// deallocated / its ownership changed inside a function, and we don't care
|
||||
/// if it was assigned to a local variable (its change in ownership is
|
||||
/// inconsequential).
|
||||
virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN,
|
||||
const ExplodedNode *CallExitEndN) {
|
||||
return false;
|
||||
}
|
||||
|
||||
/// Consume the information on the non-modifying stack frame in order to
|
||||
/// either emit a note or not. May suppress the report entirely.
|
||||
|
@ -702,7 +735,6 @@ public:
|
|||
};
|
||||
|
||||
} // namespace ento
|
||||
|
||||
} // namespace clang
|
||||
|
||||
#endif // LLVM_CLANG_STATICANALYZER_CORE_BUGREPORTER_BUGREPORTERVISITORS_H
|
||||
|
|
|
@ -52,6 +52,7 @@
|
|||
#include "clang/AST/Expr.h"
|
||||
#include "clang/AST/ExprCXX.h"
|
||||
#include "clang/AST/ParentMap.h"
|
||||
#include "clang/Analysis/ProgramPoint.h"
|
||||
#include "clang/Basic/LLVM.h"
|
||||
#include "clang/Basic/SourceManager.h"
|
||||
#include "clang/Basic/TargetInfo.h"
|
||||
|
@ -76,8 +77,10 @@
|
|||
#include "llvm/ADT/SetOperations.h"
|
||||
#include "llvm/ADT/SmallString.h"
|
||||
#include "llvm/ADT/StringExtras.h"
|
||||
#include "llvm/Support/Casting.h"
|
||||
#include "llvm/Support/Compiler.h"
|
||||
#include "llvm/Support/ErrorHandling.h"
|
||||
#include "llvm/Support/raw_ostream.h"
|
||||
#include <climits>
|
||||
#include <functional>
|
||||
#include <utility>
|
||||
|
@ -755,6 +758,17 @@ class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
|
|||
Owners.insert(Region);
|
||||
return true;
|
||||
}
|
||||
|
||||
LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
|
||||
LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &out) const {
|
||||
out << "Owners: {\n";
|
||||
for (const MemRegion *Owner : Owners) {
|
||||
out << " ";
|
||||
Owner->dumpToStream(out);
|
||||
out << ",\n";
|
||||
}
|
||||
out << "}\n";
|
||||
}
|
||||
};
|
||||
|
||||
protected:
|
||||
|
@ -768,31 +782,24 @@ protected:
|
|||
return Ret;
|
||||
}
|
||||
|
||||
static const ExplodedNode *getCallExitEnd(const ExplodedNode *N) {
|
||||
while (N && !N->getLocationAs<CallExitEnd>())
|
||||
N = N->getFirstSucc();
|
||||
return N;
|
||||
LLVM_DUMP_METHOD static std::string
|
||||
getFunctionName(const ExplodedNode *CallEnterN) {
|
||||
if (const CallExpr *CE = llvm::dyn_cast_or_null<CallExpr>(
|
||||
CallEnterN->getLocationAs<CallEnter>()->getCallExpr()))
|
||||
if (const FunctionDecl *FD = CE->getDirectCallee())
|
||||
return FD->getQualifiedNameAsString();
|
||||
return "";
|
||||
}
|
||||
|
||||
virtual bool
|
||||
wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
|
||||
const ExplodedNode *CallExitN) override {
|
||||
if (CurrN->getLocationAs<CallEnter>())
|
||||
wasModifiedInFunction(const ExplodedNode *CallEnterN,
|
||||
const ExplodedNode *CallExitEndN) override {
|
||||
if (CallEnterN->getState()->get<RegionState>(Sym) !=
|
||||
CallExitEndN->getState()->get<RegionState>(Sym))
|
||||
return true;
|
||||
|
||||
// Its the state right *after* the call that is interesting. Any pointers
|
||||
// inside the call that pointed to the allocated memory are of little
|
||||
// consequence if their lifetime ends within the function.
|
||||
CallExitN = getCallExitEnd(CallExitN);
|
||||
if (!CallExitN)
|
||||
return true;
|
||||
|
||||
if (CurrN->getState()->get<RegionState>(Sym) !=
|
||||
CallExitN->getState()->get<RegionState>(Sym))
|
||||
return true;
|
||||
|
||||
OwnerSet CurrOwners = getOwnersAtNode(CurrN);
|
||||
OwnerSet ExitOwners = getOwnersAtNode(CallExitN);
|
||||
OwnerSet CurrOwners = getOwnersAtNode(CallEnterN);
|
||||
OwnerSet ExitOwners = getOwnersAtNode(CallExitEndN);
|
||||
|
||||
// Owners in the current set may be purged from the analyzer later on.
|
||||
// If a variable is dead (is not referenced directly or indirectly after
|
||||
|
|
|
@ -355,43 +355,82 @@ bool NoStateChangeFuncVisitor::isModifiedInFrame(const ExplodedNode *N) {
|
|||
return FramesModifying.count(SCtx);
|
||||
}
|
||||
|
||||
void NoStateChangeFuncVisitor::markFrameAsModifying(
|
||||
const StackFrameContext *SCtx) {
|
||||
while (!SCtx->inTopFrame()) {
|
||||
auto p = FramesModifying.insert(SCtx);
|
||||
if (!p.second)
|
||||
break; // Frame and all its parents already inserted.
|
||||
|
||||
SCtx = SCtx->getParent()->getStackFrame();
|
||||
}
|
||||
}
|
||||
|
||||
static const ExplodedNode *getMatchingCallExitEnd(const ExplodedNode *N) {
|
||||
assert(N->getLocationAs<CallEnter>());
|
||||
// The stackframe of the callee is only found in the nodes succeeding
|
||||
// the CallEnter node. CallEnter's stack frame refers to the caller.
|
||||
const StackFrameContext *OrigSCtx = N->getFirstSucc()->getStackFrame();
|
||||
|
||||
// Similarly, the nodes preceding CallExitEnd refer to the callee's stack
|
||||
// frame.
|
||||
auto IsMatchingCallExitEnd = [OrigSCtx](const ExplodedNode *N) {
|
||||
return N->getLocationAs<CallExitEnd>() &&
|
||||
OrigSCtx == N->getFirstPred()->getStackFrame();
|
||||
};
|
||||
while (N && !IsMatchingCallExitEnd(N)) {
|
||||
assert(N->succ_size() <= 1 &&
|
||||
"This function is to be used on the trimmed ExplodedGraph!");
|
||||
N = N->getFirstSucc();
|
||||
}
|
||||
return N;
|
||||
}
|
||||
|
||||
void NoStateChangeFuncVisitor::findModifyingFrames(
|
||||
const ExplodedNode *const CallExitBeginN) {
|
||||
|
||||
assert(CallExitBeginN->getLocationAs<CallExitBegin>());
|
||||
const ExplodedNode *LastReturnN = CallExitBeginN;
|
||||
|
||||
const StackFrameContext *const OriginalSCtx =
|
||||
CallExitBeginN->getLocationContext()->getStackFrame();
|
||||
|
||||
const ExplodedNode *CurrN = CallExitBeginN;
|
||||
const ExplodedNode *CurrCallExitBeginN = CallExitBeginN;
|
||||
const StackFrameContext *CurrentSCtx = OriginalSCtx;
|
||||
|
||||
do {
|
||||
ProgramStateRef State = CurrN->getState();
|
||||
auto CallExitLoc = CurrN->getLocationAs<CallExitBegin>();
|
||||
if (CallExitLoc) {
|
||||
LastReturnN = CurrN;
|
||||
for (const ExplodedNode *CurrN = CallExitBeginN; CurrN;
|
||||
CurrN = CurrN->getFirstPred()) {
|
||||
// Found a new inlined call.
|
||||
if (CurrN->getLocationAs<CallExitBegin>()) {
|
||||
CurrCallExitBeginN = CurrN;
|
||||
CurrentSCtx = CurrN->getStackFrame();
|
||||
FramesModifyingCalculated.insert(CurrentSCtx);
|
||||
// We won't see a change in between two identical exploded nodes: skip.
|
||||
continue;
|
||||
}
|
||||
|
||||
FramesModifyingCalculated.insert(
|
||||
CurrN->getLocationContext()->getStackFrame());
|
||||
if (auto CE = CurrN->getLocationAs<CallEnter>()) {
|
||||
if (const ExplodedNode *CallExitEndN = getMatchingCallExitEnd(CurrN))
|
||||
if (wasModifiedInFunction(CurrN, CallExitEndN))
|
||||
markFrameAsModifying(CurrentSCtx);
|
||||
|
||||
if (wasModifiedBeforeCallExit(CurrN, LastReturnN)) {
|
||||
const StackFrameContext *SCtx = CurrN->getStackFrame();
|
||||
while (!SCtx->inTopFrame()) {
|
||||
auto p = FramesModifying.insert(SCtx);
|
||||
if (!p.second)
|
||||
break; // Frame and all its parents already inserted.
|
||||
SCtx = SCtx->getParent()->getStackFrame();
|
||||
// We exited this inlined call, lets actualize the stack frame.
|
||||
CurrentSCtx = CurrN->getStackFrame();
|
||||
|
||||
// Stop calculating at the current function, but always regard it as
|
||||
// modifying, so we can avoid notes like this:
|
||||
// void f(Foo &F) {
|
||||
// F.field = 0; // note: 0 assigned to 'F.field'
|
||||
// // note: returning without writing to 'F.field'
|
||||
// }
|
||||
if (CE->getCalleeContext() == OriginalSCtx) {
|
||||
markFrameAsModifying(CurrentSCtx);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Stop calculation at the call to the current function.
|
||||
if (auto CE = CurrN->getLocationAs<CallEnter>())
|
||||
if (CE->getCalleeContext() == OriginalSCtx)
|
||||
break;
|
||||
|
||||
CurrN = CurrN->getFirstPred();
|
||||
} while (CurrN);
|
||||
if (wasModifiedBeforeCallExit(CurrN, CurrCallExitBeginN))
|
||||
markFrameAsModifying(CurrentSCtx);
|
||||
}
|
||||
}
|
||||
|
||||
PathDiagnosticPieceRef NoStateChangeFuncVisitor::VisitNode(
|
||||
|
|
|
@ -9,6 +9,7 @@ add_clang_unittest(StaticAnalysisTests
|
|||
CallDescriptionTest.cpp
|
||||
CallEventTest.cpp
|
||||
FalsePositiveRefutationBRVisitorTest.cpp
|
||||
NoStateChangeFuncVisitorTest.cpp
|
||||
ParamRegionTest.cpp
|
||||
RangeSetTest.cpp
|
||||
RegisterCustomCheckersTest.cpp
|
||||
|
|
|
@ -81,7 +81,7 @@ TEST(CXXDeallocatorCall, SimpleDestructor) {
|
|||
}
|
||||
)",
|
||||
Diags));
|
||||
EXPECT_EQ(Diags, "test.CXXDeallocator:NumArgs: 1\n");
|
||||
EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
|
|
@ -6,6 +6,7 @@
|
|||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "clang/Analysis/PathDiagnostic.h"
|
||||
#include "clang/Frontend/CompilerInstance.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
|
||||
|
@ -26,8 +27,23 @@ public:
|
|||
DiagConsumer(llvm::raw_ostream &Output) : Output(Output) {}
|
||||
void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
|
||||
FilesMade *filesMade) override {
|
||||
for (const auto *PD : Diags)
|
||||
Output << PD->getCheckerName() << ":" << PD->getShortDescription() << '\n';
|
||||
for (const auto *PD : Diags) {
|
||||
Output << PD->getCheckerName() << ": ";
|
||||
|
||||
for (PathDiagnosticPieceRef Piece :
|
||||
PD->path.flatten(/*ShouldFlattenMacros*/ true)) {
|
||||
if (Piece->getKind() != PathDiagnosticPiece::Event)
|
||||
continue;
|
||||
if (Piece->getString().empty())
|
||||
continue;
|
||||
// The last event is usually the same as the warning message, skip.
|
||||
if (Piece->getString() == PD->getShortDescription())
|
||||
continue;
|
||||
|
||||
Output << Piece->getString() << " | ";
|
||||
}
|
||||
Output << PD->getShortDescription() << '\n';
|
||||
}
|
||||
}
|
||||
|
||||
StringRef getName() const override { return "Test"; }
|
||||
|
|
|
@ -128,7 +128,7 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase, UnSatInTheMiddleNoReport) {
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeAndCrossCheckArgs, Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n");
|
||||
// Single warning. The second report was invalidated by the visitor.
|
||||
|
||||
// Without enabling the crosscheck-with-z3 both reports are displayed.
|
||||
|
@ -136,8 +136,8 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase, UnSatInTheMiddleNoReport) {
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeArgs, Diags2));
|
||||
EXPECT_EQ(Diags2,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_CONTRADICTION\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_CONTRADICTION\n");
|
||||
}
|
||||
|
||||
TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
||||
|
@ -159,7 +159,7 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeAndCrossCheckArgs, Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n");
|
||||
// Single warning. The second report was invalidated by the visitor.
|
||||
|
||||
// Without enabling the crosscheck-with-z3 both reports are displayed.
|
||||
|
@ -167,8 +167,8 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeArgs, Diags2));
|
||||
EXPECT_EQ(Diags2,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator:CAN_BE_TRUE\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator: CAN_BE_TRUE\n");
|
||||
}
|
||||
|
||||
TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
||||
|
@ -206,7 +206,7 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeAndCrossCheckArgs, Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n");
|
||||
// Single warning. The second report was invalidated by the visitor.
|
||||
|
||||
// Without enabling the crosscheck-with-z3 both reports are displayed.
|
||||
|
@ -214,8 +214,8 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase,
|
|||
EXPECT_TRUE(runCheckerOnCodeWithArgs<addFalsePositiveGenerator>(
|
||||
Code, LazyAssumeArgs, Diags2));
|
||||
EXPECT_EQ(Diags2,
|
||||
"test.FalsePositiveGenerator:REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator:CAN_BE_TRUE\n");
|
||||
"test.FalsePositiveGenerator: REACHED_WITH_NO_CONTRADICTION\n"
|
||||
"test.FalsePositiveGenerator: CAN_BE_TRUE\n");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
|
|
@ -0,0 +1,302 @@
|
|||
//===- unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp ----------===//
|
||||
//
|
||||
// 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 "CheckerRegistration.h"
|
||||
#include "clang/Frontend/CompilerInstance.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
|
||||
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
|
||||
#include "clang/StaticAnalyzer/Core/Checker.h"
|
||||
#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
|
||||
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
|
||||
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
|
||||
#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
|
||||
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
|
||||
#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
|
||||
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
|
||||
#include "llvm/Support/ErrorHandling.h"
|
||||
#include "llvm/Support/raw_ostream.h"
|
||||
#include "gtest/gtest.h"
|
||||
#include <memory>
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Base classes for testing NoStateChangeFuncVisitor.
|
||||
//
|
||||
// Testing is done by observing a very simple trait change from one node to
|
||||
// another -- the checker sets the ErrorPrevented trait to true if
|
||||
// 'preventError()' is called in the source code, and sets it to false if
|
||||
// 'allowError()' is called. If this trait is false when 'error()' is called,
|
||||
// a warning is emitted.
|
||||
//
|
||||
// The checker then registers a simple NoStateChangeFuncVisitor to add notes to
|
||||
// inlined functions that could have, but neglected to prevent the error.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
REGISTER_TRAIT_WITH_PROGRAMSTATE(ErrorPrevented, bool)
|
||||
|
||||
namespace clang {
|
||||
namespace ento {
|
||||
namespace {
|
||||
|
||||
class ErrorNotPreventedFuncVisitor : public NoStateChangeFuncVisitor {
|
||||
public:
|
||||
ErrorNotPreventedFuncVisitor()
|
||||
: NoStateChangeFuncVisitor(bugreporter::TrackingKind::Thorough) {}
|
||||
|
||||
virtual PathDiagnosticPieceRef
|
||||
maybeEmitNoteForObjCSelf(PathSensitiveBugReport &R,
|
||||
const ObjCMethodCall &Call,
|
||||
const ExplodedNode *N) override {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
virtual PathDiagnosticPieceRef
|
||||
maybeEmitNoteForCXXThis(PathSensitiveBugReport &R,
|
||||
const CXXConstructorCall &Call,
|
||||
const ExplodedNode *N) override {
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
virtual PathDiagnosticPieceRef
|
||||
maybeEmitNoteForParameters(PathSensitiveBugReport &R, const CallEvent &Call,
|
||||
const ExplodedNode *N) override {
|
||||
PathDiagnosticLocation L = PathDiagnosticLocation::create(
|
||||
N->getLocation(),
|
||||
N->getState()->getStateManager().getContext().getSourceManager());
|
||||
return std::make_shared<PathDiagnosticEventPiece>(
|
||||
L, "Returning without prevening the error");
|
||||
}
|
||||
|
||||
void Profile(llvm::FoldingSetNodeID &ID) const override {
|
||||
static int Tag = 0;
|
||||
ID.AddPointer(&Tag);
|
||||
}
|
||||
};
|
||||
|
||||
template <class Visitor>
|
||||
class StatefulChecker : public Checker<check::PreCall> {
|
||||
mutable std::unique_ptr<BugType> BT;
|
||||
|
||||
public:
|
||||
void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
|
||||
if (Call.isCalled(CallDescription{"preventError", 0})) {
|
||||
C.addTransition(C.getState()->set<ErrorPrevented>(true));
|
||||
return;
|
||||
}
|
||||
|
||||
if (Call.isCalled(CallDescription{"allowError", 0})) {
|
||||
C.addTransition(C.getState()->set<ErrorPrevented>(false));
|
||||
return;
|
||||
}
|
||||
|
||||
if (Call.isCalled(CallDescription{"error", 0})) {
|
||||
if (C.getState()->get<ErrorPrevented>())
|
||||
return;
|
||||
const ExplodedNode *N = C.generateErrorNode();
|
||||
if (!N)
|
||||
return;
|
||||
if (!BT)
|
||||
BT.reset(new BugType(this->getCheckerName(), "error()",
|
||||
categories::SecurityError));
|
||||
auto R =
|
||||
std::make_unique<PathSensitiveBugReport>(*BT, "error() called", N);
|
||||
R->template addVisitor<Visitor>();
|
||||
C.emitReport(std::move(R));
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
} // namespace
|
||||
} // namespace ento
|
||||
} // namespace clang
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Non-thorough analysis: only the state right before and right after the
|
||||
// function call is checked for the difference in trait value.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
namespace clang {
|
||||
namespace ento {
|
||||
namespace {
|
||||
|
||||
class NonThoroughErrorNotPreventedFuncVisitor
|
||||
: public ErrorNotPreventedFuncVisitor {
|
||||
public:
|
||||
virtual bool
|
||||
wasModifiedInFunction(const ExplodedNode *CallEnterN,
|
||||
const ExplodedNode *CallExitEndN) override {
|
||||
return CallEnterN->getState()->get<ErrorPrevented>() !=
|
||||
CallExitEndN->getState()->get<ErrorPrevented>();
|
||||
}
|
||||
};
|
||||
|
||||
void addNonThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
|
||||
AnalyzerOptions &AnOpts) {
|
||||
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
|
||||
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
|
||||
Registry
|
||||
.addChecker<StatefulChecker<NonThoroughErrorNotPreventedFuncVisitor>>(
|
||||
"test.StatefulChecker", "Description", "");
|
||||
});
|
||||
}
|
||||
|
||||
TEST(NoStateChangeFuncVisitor, NonThoroughFunctionAnalysis) {
|
||||
std::string Diags;
|
||||
EXPECT_TRUE(runCheckerOnCode<addNonThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
//preventError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.StatefulChecker: Calling 'g' | Returning without prevening "
|
||||
"the error | Returning from 'g' | error() called\n");
|
||||
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(runCheckerOnCode<addNonThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
preventError();
|
||||
allowError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.StatefulChecker: Calling 'g' | Returning without prevening "
|
||||
"the error | Returning from 'g' | error() called\n");
|
||||
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(runCheckerOnCode<addNonThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
preventError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags, "");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace ento
|
||||
} // namespace clang
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
// Thorough analysis: only the state right before and right after the
|
||||
// function call is checked for the difference in trait value.
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
namespace clang {
|
||||
namespace ento {
|
||||
namespace {
|
||||
|
||||
class ThoroughErrorNotPreventedFuncVisitor
|
||||
: public ErrorNotPreventedFuncVisitor {
|
||||
public:
|
||||
virtual bool
|
||||
wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
|
||||
const ExplodedNode *CallExitBeginN) override {
|
||||
return CurrN->getState()->get<ErrorPrevented>() !=
|
||||
CallExitBeginN->getState()->get<ErrorPrevented>();
|
||||
}
|
||||
};
|
||||
|
||||
void addThoroughStatefulChecker(AnalysisASTConsumer &AnalysisConsumer,
|
||||
AnalyzerOptions &AnOpts) {
|
||||
AnOpts.CheckersAndPackages = {{"test.StatefulChecker", true}};
|
||||
AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
|
||||
Registry.addChecker<StatefulChecker<ThoroughErrorNotPreventedFuncVisitor>>(
|
||||
"test.StatefulChecker", "Description", "");
|
||||
});
|
||||
}
|
||||
|
||||
TEST(NoStateChangeFuncVisitor, ThoroughFunctionAnalysis) {
|
||||
std::string Diags;
|
||||
EXPECT_TRUE(runCheckerOnCode<addThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
//preventError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags,
|
||||
"test.StatefulChecker: Calling 'g' | Returning without prevening "
|
||||
"the error | Returning from 'g' | error() called\n");
|
||||
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(runCheckerOnCode<addThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
preventError();
|
||||
allowError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags, "test.StatefulChecker: error() called\n");
|
||||
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(runCheckerOnCode<addThoroughStatefulChecker>(R"(
|
||||
void error();
|
||||
void preventError();
|
||||
void allowError();
|
||||
|
||||
void g() {
|
||||
preventError();
|
||||
}
|
||||
|
||||
void f() {
|
||||
g();
|
||||
error();
|
||||
}
|
||||
)", Diags));
|
||||
EXPECT_EQ(Diags, "");
|
||||
}
|
||||
|
||||
} // namespace
|
||||
} // namespace ento
|
||||
} // namespace clang
|
|
@ -51,7 +51,7 @@ void addCustomChecker(AnalysisASTConsumer &AnalysisConsumer,
|
|||
TEST(RegisterCustomCheckers, RegisterChecker) {
|
||||
std::string Diags;
|
||||
EXPECT_TRUE(runCheckerOnCode<addCustomChecker>("void f() {;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.CustomChecker:Custom diagnostic description\n");
|
||||
EXPECT_EQ(Diags, "test.CustomChecker: Custom diagnostic description\n");
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
@ -169,7 +169,7 @@ void addDep(AnalysisASTConsumer &AnalysisConsumer,
|
|||
TEST(RegisterDeps, UnsatisfiedDependency) {
|
||||
std::string Diags;
|
||||
EXPECT_TRUE(runCheckerOnCode<addDep>("void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.RegistrationOrder\n");
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.RegistrationOrder\n");
|
||||
}
|
||||
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
@ -272,7 +272,7 @@ TEST(RegisterDeps, SimpleWeakDependency) {
|
|||
std::string Diags;
|
||||
EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerBothEnabled>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep\ntest."
|
||||
"Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
|
@ -280,31 +280,33 @@ TEST(RegisterDeps, SimpleWeakDependency) {
|
|||
// but the dependencies are switched.
|
||||
EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerBothEnabledSwitched>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.Dep\ntest."
|
||||
"RegistrationOrder\ntest.WeakDep\n");
|
||||
Diags.clear();
|
||||
|
||||
// Weak dependencies dont prevent dependent checkers from being enabled.
|
||||
EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerDepDisabled>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
|
||||
EXPECT_EQ(Diags,
|
||||
"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
// Nor will they be enabled just because a dependent checker is.
|
||||
EXPECT_TRUE(runCheckerOnCode<addWeakDepCheckerDepUnspecified>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
|
||||
EXPECT_EQ(Diags,
|
||||
"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(
|
||||
runCheckerOnCode<addWeakDepTransitivity>("void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep2\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep2\ntest."
|
||||
"Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(
|
||||
runCheckerOnCode<addWeakDepHasWeakDep>("void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep2\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep2\ntest."
|
||||
"WeakDep\ntest.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
}
|
||||
|
@ -414,7 +416,7 @@ TEST(RegisterDeps, DependencyInteraction) {
|
|||
std::string Diags;
|
||||
EXPECT_TRUE(
|
||||
runCheckerOnCode<addWeakDepHasStrongDep>("void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.StrongDep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.StrongDep\ntest."
|
||||
"WeakDep\ntest.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
|
@ -424,14 +426,14 @@ TEST(RegisterDeps, DependencyInteraction) {
|
|||
// established in between the modeling portion and the weak dependency.
|
||||
EXPECT_TRUE(
|
||||
runCheckerOnCode<addWeakDepAndStrongDep>("void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.WeakDep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.WeakDep\ntest."
|
||||
"StrongDep\ntest.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
// If a weak dependency is disabled, the checker itself can still be enabled.
|
||||
EXPECT_TRUE(runCheckerOnCode<addDisabledWeakDepHasStrongDep>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.Dep\ntest."
|
||||
"RegistrationOrder\ntest.StrongDep\n");
|
||||
Diags.clear();
|
||||
|
||||
|
@ -439,20 +441,22 @@ TEST(RegisterDeps, DependencyInteraction) {
|
|||
// but it shouldn't enable a strong unspecified dependency.
|
||||
EXPECT_TRUE(runCheckerOnCode<addDisabledWeakDepHasUnspecifiedStrongDep>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
|
||||
EXPECT_EQ(Diags,
|
||||
"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
// A strong dependency of a weak dependency is disabled, so neither of them
|
||||
// should be enabled.
|
||||
EXPECT_TRUE(runCheckerOnCode<addWeakDepHasDisabledStrongDep>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.Dep\ntest.RegistrationOrder\n");
|
||||
EXPECT_EQ(Diags,
|
||||
"test.RegistrationOrder: test.Dep\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
|
||||
EXPECT_TRUE(
|
||||
runCheckerOnCode<addWeakDepHasUnspecifiedButLaterEnabledStrongDep>(
|
||||
"void f() {int i;}", Diags));
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder:test.StrongDep\ntest.WeakDep\ntest."
|
||||
EXPECT_EQ(Diags, "test.RegistrationOrder: test.StrongDep\ntest.WeakDep\ntest."
|
||||
"Dep\ntest.Dep2\ntest.RegistrationOrder\n");
|
||||
Diags.clear();
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue