Index: clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp =================================================================== --- clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp +++ clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp @@ -32,15 +32,30 @@ using namespace ento; namespace { -class MIGChecker : public Checker> { +class MIGChecker : public Checker, + check::EndFunction> { BugType BT{this, "Use-after-free (MIG calling convention violation)", categories::MemoryError}; CallDescription vm_deallocate { "vm_deallocate", 3 }; + void checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const; + public: void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const; + + // HACK: We're making two attempts to find the bug: checkEndFunction + // should normally be enough but it fails when the return value is a literal + // that never gets put into the Environment and ends of function with multiple + // returns get agglutinated across returns, preventing us from obtaining + // the return value. The problem is similar to https://reviews.llvm.org/D25326 + // but now we step into it in the top-level function. + void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } + void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const { + checkReturnAux(RS, C); + } class Visitor : public BugReporterVisitor { public: @@ -140,7 +155,7 @@ C.addTransition(C.getState()->set(PVD)); } -void MIGChecker::checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const { +void MIGChecker::checkReturnAux(const ReturnStmt *RS, CheckerContext &C) const { // It is very unlikely that a MIG callback will be called from anywhere // within the project under analysis and the caller isn't itself a routine // that follows the MIG calling convention. Therefore we're safe to believe Index: clang/test/Analysis/mig.mm =================================================================== --- clang/test/Analysis/mig.mm +++ clang/test/Analysis/mig.mm @@ -56,6 +56,31 @@ // expected-note@-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}} } +// Make sure we find the bug when the object is destroyed within an +// automatic destructor. +MIG_SERVER_ROUTINE +kern_return_t test_vm_deallocate_in_automatic_dtor(mach_port_name_t port, vm_address_t address, vm_size_t size) { + struct WillDeallocate { + mach_port_name_t port; + vm_address_t address; + vm_size_t size; + ~WillDeallocate() { + vm_deallocate(port, address, size); // expected-note{{Value passed through parameter 'address' is deallocated}} + } + } will_deallocate{port, address, size}; + + if (size > 10) { + // expected-note@-1{{Assuming 'size' is > 10}} + // expected-note@-2{{Taking true branch}} + return KERN_ERROR; + // expected-note@-1{{Calling '~WillDeallocate'}} + // expected-note@-2{{Returning from '~WillDeallocate'}} + // expected-warning@-3{{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@-4 {{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; +} + // 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;