r354641 - [analyzer] MIGChecker: Improve intermediate diagnostic notes.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 21 16:06:30 PST 2019
Author: dergachev
Date: Thu Feb 21 16:06:30 2019
New Revision: 354641
URL: http://llvm.org/viewvc/llvm-project?rev=354641&view=rev
Log:
[analyzer] MIGChecker: Improve intermediate diagnostic notes.
Add a BugReporterVisitor for highlighting the events of deallocating a
parameter. All such events are relevant to the emitted report (as long as the
report is indeed emitted), so all of them will get highlighted.
Add a trackExpressionValue visitor for highlighting where does the error return
code come from.
Do not add a trackExpressionValue visitor for highlighting how the deallocated
argument(s) was(were) copied around. This still remains to be implemented.
rdar://problem/35380337
Differential Revision: https://reviews.llvm.org/D58368
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
cfe/trunk/test/Analysis/mig.mm
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp?rev=354641&r1=354640&r2=354641&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp Thu Feb 21 16:06:30 2019
@@ -41,19 +41,56 @@ class MIGChecker : public Checker<check:
public:
void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
+
+ class Visitor : public BugReporterVisitor {
+ public:
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ static int X = 0;
+ ID.AddPointer(&X);
+ }
+
+ std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ BugReporterContext &BRC, BugReport &R);
+ };
};
} // end anonymous namespace
-REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, bool)
+// FIXME: It's a 'const ParmVarDecl *' but there's no ready-made GDM traits
+// specialization for this sort of types.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ReleasedParameter, const void *)
+
+std::shared_ptr<PathDiagnosticPiece>
+MIGChecker::Visitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+ BugReport &R) {
+ const auto *NewPVD = static_cast<const ParmVarDecl *>(
+ N->getState()->get<ReleasedParameter>());
+ const auto *OldPVD = static_cast<const ParmVarDecl *>(
+ N->getFirstPred()->getState()->get<ReleasedParameter>());
+ if (OldPVD == NewPVD)
+ return nullptr;
+
+ assert(NewPVD && "What is deallocated cannot be un-deallocated!");
+ SmallString<64> Str;
+ llvm::raw_svector_ostream OS(Str);
+ OS << "Value passed through parameter '" << NewPVD->getName()
+ << "' is deallocated";
+
+ PathDiagnosticLocation Loc =
+ PathDiagnosticLocation::create(N->getLocation(), BRC.getSourceManager());
+ return std::make_shared<PathDiagnosticEventPiece>(Loc, OS.str());
+}
-static bool isCurrentArgSVal(SVal V, CheckerContext &C) {
+static const ParmVarDecl *getOriginParam(SVal V, CheckerContext &C) {
SymbolRef Sym = V.getAsSymbol();
if (!Sym)
- return false;
+ return nullptr;
const auto *VR = dyn_cast_or_null<VarRegion>(Sym->getOriginRegion());
- return VR && VR->hasStackParametersStorage() &&
- VR->getStackFrame()->inTopFrame();
+ if (VR && VR->hasStackParametersStorage() &&
+ VR->getStackFrame()->inTopFrame())
+ return cast<ParmVarDecl>(VR->getDecl());
+
+ return nullptr;
}
static bool isInMIGCall(CheckerContext &C) {
@@ -96,8 +133,11 @@ void MIGChecker::checkPostCall(const Cal
// TODO: Unhardcode "1".
SVal Arg = Call.getArgSVal(1);
- if (isCurrentArgSVal(Arg, C))
- C.addTransition(C.getState()->set<ReleasedParameter>(true));
+ const ParmVarDecl *PVD = getOriginParam(Arg, C);
+ if (!PVD)
+ return;
+
+ C.addTransition(C.getState()->set<ReleasedParameter>(PVD));
}
void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const {
@@ -139,6 +179,9 @@ void MIGChecker::checkPreStmt(const Retu
"deallocate it again",
N);
+ R->addRange(RS->getSourceRange());
+ bugreporter::trackExpressionValue(N, RS->getRetValue(), *R, false);
+ R->addVisitor(llvm::make_unique<Visitor>());
C.emitReport(std::move(R));
}
Modified: cfe/trunk/test/Analysis/mig.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/mig.mm?rev=354641&r1=354640&r2=354641&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/mig.mm (original)
+++ cfe/trunk/test/Analysis/mig.mm Thu Feb 21 16:06:30 2019
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,alpha.osx.MIG\
-// RUN: -fblocks -verify %s
+// RUN: -analyzer-output=text -fblocks -verify %s
// XNU APIs.
@@ -20,9 +20,11 @@ kern_return_t vm_deallocate(mach_port_na
MIG_SERVER_ROUTINE
kern_return_t basic_test(mach_port_name_t port, vm_address_t address, vm_size_t size) {
- vm_deallocate(port, address, size);
- if (size > 10) {
+ vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
+ if (size > 10) { // expected-note{{Assuming 'size' is > 10}}
+ // expected-note at -1{{Taking true branch}}
return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
}
return KERN_SUCCESS;
}
@@ -42,6 +44,18 @@ kern_return_t no_crash(mach_port_name_t
vm_deallocate(port, address, size);
}
+// When releasing two parameters, add a note for both of them.
+// Also when returning a variable, explain why do we think that it contains
+// a non-success code.
+MIG_SERVER_ROUTINE
+kern_return_t release_twice(mach_port_name_t port, vm_address_t addr1, vm_address_t addr2, vm_size_t size) {
+ kern_return_t ret = KERN_ERROR; // expected-note{{'ret' initialized to 1}}
+ vm_deallocate(port, addr1, size); // expected-note{{Value passed through parameter 'addr1' is deallocated}}
+ vm_deallocate(port, addr2, size); // expected-note{{Value passed through parameter 'addr2' is deallocated}}
+ return ret; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+}
+
// Check that we work on Objective-C messages and blocks.
@interface I
- (kern_return_t)fooAtPort:(mach_port_name_t)port withAddress:(vm_address_t)address ofSize:(vm_size_t)size;
@@ -51,8 +65,9 @@ kern_return_t no_crash(mach_port_name_t
- (kern_return_t)fooAtPort:(mach_port_name_t)port
withAddress:(vm_address_t)address
ofSize:(vm_size_t)size MIG_SERVER_ROUTINE {
- vm_deallocate(port, address, size);
+ vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
}
@end
@@ -60,8 +75,9 @@ void test_block() {
kern_return_t (^block)(mach_port_name_t, vm_address_t, vm_size_t) =
^MIG_SERVER_ROUTINE (mach_port_name_t port,
vm_address_t address, vm_size_t size) {
- vm_deallocate(port, address, size);
+ vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}}
 return KERN_ERROR; // expected-warning{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
+ // expected-note at -1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
};
}
More information about the cfe-commits
mailing list