Skip to content

Commit

Permalink
Support LLVM 10 in GC checker (JuliaLang#37153)
Browse files Browse the repository at this point in the history 
Ref llvm/llvm-project@2f169e7
Ref llvm/llvm-project@6b85f8e

Also handle noreturn function call in GC frame counting,
and remove some old code for LLVM <= 7
  • Loading branch information
@yuyichao @simeonschaub
yuyichao authored and simeonschaub committed Aug 29, 2020
1 parent b268f30 commit 7203b76
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 55 deletions.
88 changes: 33 additions & 55 deletions src/clangsa/GCChecker.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@
#include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "clang/Tooling/Tooling.h"
#if LLVM_VERSION_MAJOR >= 8
#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
#else
#include "clang/StaticAnalyzer/Core/CheckerRegistry.h"
#endif

#include <iostream>
#include <memory>
Expand All @@ -34,6 +30,7 @@
using std::make_unique;
#else
using llvm::make_unique;
#define PathSensitiveBugReport BugReport
#endif

namespace {
Expand All @@ -43,6 +40,16 @@ using namespace ento;
#define PDP std::shared_ptr<PathDiagnosticPiece>
#define MakePDP make_unique<PathDiagnosticEventPiece>

static const Stmt *getStmtForDiagnostics(const ExplodedNode *N)
{
#if LLVM_VERSION_MAJOR >= 10
return N->getStmtForDiagnostics();
#else
return PathDiagnosticLocation::getStmt(N);
#endif
}


class GCChecker
: public Checker<
eval::Call,
Expand All @@ -60,7 +67,7 @@ class GCChecker
template <typename callback>
void report_error(callback f, CheckerContext &C, const char *message) const;
void report_error(CheckerContext &C, const char *message) const {
return report_error([](BugReport *) {}, C, message);
return report_error([](PathSensitiveBugReport *) {}, C, message);
}
void
report_value_error(CheckerContext &C, SymbolRef Sym, const char *message,
Expand Down Expand Up @@ -227,11 +234,7 @@ class GCChecker

public:
void checkBeginFunction(CheckerContext &Ctx) const;
#if LLVM_VERSION_MAJOR >= 7
void checkEndFunction(const clang::ReturnStmt *RS, CheckerContext &Ctx) const;
#else
void checkEndFunction(CheckerContext &Ctx) const;
#endif
#if LLVM_VERSION_MAJOR >= 9
bool evalCall(const CallEvent &Call, CheckerContext &C) const;
#else
Expand All @@ -248,12 +251,7 @@ class GCChecker
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const;
void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
CheckerContext &) const;
class GCBugVisitor
#if LLVM_VERSION_MAJOR >= 7
: public BugReporterVisitor {
#else
: public BugReporterVisitorImpl<GCBugVisitor> {
#endif
class GCBugVisitor : public BugReporterVisitor {
public:
GCBugVisitor() {}

Expand All @@ -262,19 +260,10 @@ class GCChecker
ID.AddPointer(&X);
}

PDP VisitNode(const ExplodedNode *N,
#if LLVM_VERSION_MAJOR < 8
const ExplodedNode *PrevN,
#endif
BugReporterContext &BRC, BugReport &BR) override;
PDP VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override;
};

class GCValueBugVisitor
#if LLVM_VERSION_MAJOR >= 7
: public BugReporterVisitor {
#else
: public BugReporterVisitorImpl<GCValueBugVisitor> {
#endif
class GCValueBugVisitor : public BugReporterVisitor {
protected:
SymbolRef Sym;

Expand All @@ -288,17 +277,13 @@ class GCChecker
}

PDP ExplainNoPropagation(const ExplodedNode *N, PathDiagnosticLocation Pos,
BugReporterContext &BRC, BugReport &BR);
BugReporterContext &BRC, PathSensitiveBugReport &BR);
PDP ExplainNoPropagationFromExpr(const clang::Expr *FromWhere,
const ExplodedNode *N,
PathDiagnosticLocation Pos,
BugReporterContext &BRC, BugReport &BR);
BugReporterContext &BRC, PathSensitiveBugReport &BR);

PDP VisitNode(const ExplodedNode *N,
#if LLVM_VERSION_MAJOR < 8
const ExplodedNode *PrevN,
#endif
BugReporterContext &BRC, BugReport &BR) override;
PDP VisitNode(const ExplodedNode *N, BugReporterContext &BRC, PathSensitiveBugReport &BR) override;
}; // namespace
};

Expand Down Expand Up @@ -372,22 +357,19 @@ static const VarRegion *walk_back_to_global_VR(const MemRegion *Region) {
} // namespace Helpers

PDP GCChecker::GCBugVisitor::VisitNode(const ExplodedNode *N,
#if LLVM_VERSION_MAJOR < 8
const ExplodedNode *,
#endif
BugReporterContext &BRC, BugReport &BR) {
BugReporterContext &BRC, PathSensitiveBugReport &BR) {
const ExplodedNode *PrevN = N->getFirstPred();
unsigned NewGCDepth = N->getState()->get<GCDepth>();
unsigned OldGCDepth = PrevN->getState()->get<GCDepth>();
if (NewGCDepth != OldGCDepth) {
PathDiagnosticLocation Pos(PathDiagnosticLocation::getStmt(N),
PathDiagnosticLocation Pos(getStmtForDiagnostics(N),
BRC.getSourceManager(), N->getLocationContext());
return MakePDP(Pos, "GC frame changed here.");
}
unsigned NewGCState = N->getState()->get<GCDisabledAt>();
unsigned OldGCState = PrevN->getState()->get<GCDisabledAt>();
if (false /*NewGCState != OldGCState*/) {
PathDiagnosticLocation Pos(PathDiagnosticLocation::getStmt(N),
PathDiagnosticLocation Pos(getStmtForDiagnostics(N),
BRC.getSourceManager(), N->getLocationContext());
return MakePDP(Pos, "GC enabledness changed here.");
}
Expand All @@ -396,7 +378,7 @@ PDP GCChecker::GCBugVisitor::VisitNode(const ExplodedNode *N,

PDP GCChecker::GCValueBugVisitor::ExplainNoPropagationFromExpr(
const clang::Expr *FromWhere, const ExplodedNode *N,
PathDiagnosticLocation Pos, BugReporterContext &BRC, BugReport &BR) {
PathDiagnosticLocation Pos, BugReporterContext &BRC, PathSensitiveBugReport &BR) {
const MemRegion *Region =
N->getState()->getSVal(FromWhere, N->getLocationContext()).getAsRegion();
SymbolRef Parent = walkToRoot(
Expand Down Expand Up @@ -450,7 +432,7 @@ PDP GCChecker::GCValueBugVisitor::ExplainNoPropagationFromExpr(

PDP GCChecker::GCValueBugVisitor::ExplainNoPropagation(
const ExplodedNode *N, PathDiagnosticLocation Pos, BugReporterContext &BRC,
BugReport &BR) {
PathSensitiveBugReport &BR) {
if (N->getLocation().getAs<StmtPoint>()) {
const clang::Stmt *TheS = N->getLocation().castAs<StmtPoint>().getStmt();
const clang::CallExpr *CE = dyn_cast<CallExpr>(TheS);
Expand All @@ -476,15 +458,11 @@ PDP GCChecker::GCValueBugVisitor::ExplainNoPropagation(
}

PDP GCChecker::GCValueBugVisitor::VisitNode(const ExplodedNode *N,
#if LLVM_VERSION_MAJOR < 8
const ExplodedNode *,
#endif
BugReporterContext &BRC,
BugReport &BR) {
BugReporterContext &BRC, PathSensitiveBugReport &BR) {
const ExplodedNode *PrevN = N->getFirstPred();
const ValueState *NewValueState = N->getState()->get<GCValueMap>(Sym);
const ValueState *OldValueState = PrevN->getState()->get<GCValueMap>(Sym);
const Stmt *Stmt = PathDiagnosticLocation::getStmt(N);
const Stmt *Stmt = getStmtForDiagnostics(N);

PathDiagnosticLocation Pos;
if (Stmt)
Expand Down Expand Up @@ -555,7 +533,7 @@ void GCChecker::report_error(callback f, CheckerContext &C,

if (!BT)
BT.reset(new BugType(this, "Invalid GC thingy", categories::LogicError));
auto Report = make_unique<BugReport>(*BT, message, N);
auto Report = make_unique<PathSensitiveBugReport>(*BT, message, N);
Report->addVisitor(make_unique<GCBugVisitor>());
f(Report.get());
C.emitReport(std::move(Report));
Expand All @@ -571,7 +549,7 @@ void GCChecker::report_value_error(CheckerContext &C, SymbolRef Sym,

if (!BT)
BT.reset(new BugType(this, "Invalid GC thingy", categories::LogicError));
auto Report = make_unique<BugReport>(*BT, message, N);
auto Report = make_unique<PathSensitiveBugReport>(*BT, message, N);
Report->addVisitor(make_unique<GCValueBugVisitor>(Sym));
Report->addVisitor(make_unique<GCBugVisitor>());
Report->addVisitor(make_unique<ConditionBRVisitor>());
Expand Down Expand Up @@ -625,7 +603,7 @@ bool GCChecker::propagateArgumentRootedness(CheckerContext &C,
const ValueState *ValS = State->get<GCValueMap>(ArgSym);
if (!ValS) {
report_error(
[&](BugReport *Report) {
[&](PathSensitiveBugReport *Report) {
Report->addNote(
"Tried to find root for this parameter in inlined call",
PathDiagnosticLocation::create(P, C.getSourceManager()));
Expand Down Expand Up @@ -717,7 +695,10 @@ void GCChecker::checkEndFunction(const clang::ReturnStmt *RS,
C.addTransition(State);
if (!C.inTopFrame())
return;
if (C.getState()->get<GCDepth>() > 0)
// If `RS` is NULL, the function did not return normally.
// This could be either an abort/assertion failure or an exception throw.
// Do not require the GC frame to match in such case.
if (RS && C.getState()->get<GCDepth>() > 0)
report_error(C, "Non-popped GC frame present at end of function");
}

Expand Down Expand Up @@ -1649,11 +1630,8 @@ extern "C" const char clang_analyzerAPIVersionString[] =
CLANG_ANALYZER_API_VERSION_STRING;
extern "C" void clang_registerCheckers(CheckerRegistry &registry) {
registry.addChecker<GCChecker>(
"julia.GCChecker", "Validates julia gc invariants"
#if LLVM_VERSION_MAJOR >= 8
,
"julia.GCChecker", "Validates julia gc invariants",
"https://docs.julialang.org/en/v1/devdocs/gc-sa/"
#endif
);
}
#endif
1 change: 1 addition & 0 deletions src/llvm-alloc-opt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,7 @@ void Optimizer::splitOnStack(CallInst *orig_inst)
}
else if (auto call = dyn_cast<CallInst>(user)) {
auto callee = call->getCalledOperand();
assert(callee); // makes it clear for clang analyser that `callee` is not NULL
if (auto intrinsic = dyn_cast<IntrinsicInst>(call)) {
if (Intrinsic::ID id = intrinsic->getIntrinsicID()) {
if (id == Intrinsic::memset) {
Expand Down

0 comments on commit 7203b76

Please sign in to comment.