Index: lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp +++ lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp @@ -234,6 +234,7 @@ static const char *ANNOTATION_PREFIX = "mx_"; static const char *HANDLE_TYPE_NAME = "mx_handle_t"; static const char *SYSCALL_RETURN_TYPE_NAME = "mx_status_t"; +static const int MAX_HANDLE_IN_ARRAY = 64; class MagentaHandleChecker : public CheckergetArg(CurArgSpec.getIndex()); + const Expr *InputSizeExpr = CE->getArg(CurArgSpec.getInputCArgIdx()); + if (CheckExprIsZero(HandleBufExpr, Ctx) || + CheckExprIsZero(InputSizeExpr, Ctx)) + return nullptr; + + bool ArgEscaped = isArgumentEscaped(State->getSVal(HandleBufExpr, LCtx)); + const MemRegion *MemHandleBuf = + State->getSVal(HandleBufExpr, LCtx).getAsRegion(); + // Determine how many handles should be allocated/released, it may have + // following cases: + // 1. handlebuf is a pointer to a local mx_handle_t variable, in this case + // only 1 handle should be allocated/released and InputSizeExpr is ignored + // 2. handlebuf is an array and InputSizeExpr can be resolved to a concrete + // int, let's denote it as inputsize. If 0 < inputsize <= 4, + // allocate/release exact number of handles as indicated by inputsize. + // If inputsize <= 0, consider it as an error and fallback to + // conservativeEvalCall. If inputsize > 4, in allocation, allocate only 4 + // handles. In release, release all handles that may bound to this array. + // 3. handlebuf is an array and InputSizeExpr cannot be resolved to a concrete + // integer. In allocation, assume inputsize is 4 and do the same thing as + // condition 2. In release, release all handles that may bound to this + // array. + // This is a workaround due to the fact that CSA will execute a loop for only + // 4 times even though the loop bound is known by default. + if (!MemHandleBuf) + return nullptr; + if (CurArgSpec.isAllocation()) { + // Acquire an array of handles, the output count should not be null. + if (CurArgSpec.getOutputCArgIdx() == -1) + return nullptr; + const Expr *OutputSizeExpr = CE->getArg(CurArgSpec.getOutputCArgIdx()); + if (CheckExprIsZero(OutputSizeExpr, Ctx)) + return nullptr; + + int64_t ExtVal = getArgValueFromExpr(InputSizeExpr, State, LCtx, 4); + if (ExtVal < 0) + return nullptr; + if (!isa(MemHandleBuf)) { + if (isa(MemHandleBuf)) { + // Condition 1 + State = allocateSingleHandle(CE, HandleBufExpr, State, Ctx); + if (!State) + return nullptr; + } else { + // The pointer to array may came from top-level function argument + // In this case, the handles are escaped at allocation. So do nothing. + return nullptr; + } + } else { + if (ExtVal > 4) + ExtVal = 4; + // It is condition 2 or 3 + const ElementRegion *EleRegion = dyn_cast(MemHandleBuf); + State = allocateHandlesForArray(EleRegion, CE, ExtVal, State, Ctx, + ArgEscaped); + if (!State) + return nullptr; + } + // Process the actual_handle returned + SVal ActHandleSVal = State->getSVal(OutputSizeExpr, LCtx); + SVal ConcreteInvSVal = Bldr.makeIntVal(llvm::APSInt::get(ExtVal)); + State = State->bindLoc(ActHandleSVal, ConcreteInvSVal, LCtx); + } else if (CurArgSpec.isRelease()) { + int64_t ExtVal = getArgValueFromExpr(InputSizeExpr, State, LCtx, -1); + if (!isa(MemHandleBuf)) { + if (isa(MemHandleBuf)) { + // It is condition 1 + ExtVal = 1; + SVal HandleBufSVal = State->getSVal(MemHandleBuf); + SymbolRef SymHandleBuf = HandleBufSVal.getAsSymbol(); + if (!SymHandleBuf) + return nullptr; + const HandleState *HS = State->get(SymHandleBuf); + if (HS) { + if (HS->isReleased()) { + reportDoubleRelease(SymHandleBuf, CE->getSourceRange(), Ctx); + } else { + State = State->set(SymHandleBuf, + HandleState::getReleased(HS)); + } + } + } else { + // The pointer to the array comes from top-level function argument + // Because caller is not in current TU, we don't know where the handles + // are allocated. Do noting here. + return nullptr; + } + } else { + const ElementRegion *EleRegion = dyn_cast(MemHandleBuf); + ProgramStateRef NewState = nullptr; + if (ExtVal < 0 || ExtVal > 5) { + // Condition 2 + NewState = releaseHandlesForArray(EleRegion, CE, MAX_HANDLE_IN_ARRAY, + State, Ctx); + } else { + // Condition 3 + NewState = releaseHandlesForArray(EleRegion, CE, ExtVal, State, Ctx); + } + if (!NewState) + return nullptr; + State = NewState; + } + } + return State; } // Process the handle escaped situation. @@ -658,6 +782,118 @@ return State->set(SymHandle, HS); } +int64_t MagentaHandleChecker::getArgValueFromExpr(const Expr *ArgExpr, + ProgramStateRef State, + const LocationContext *LCtx, + int64_t DefaultVal) const { + int64_t ExtVal = DefaultVal; + SVal InputSizeSVal = State->getSVal(ArgExpr, LCtx); + if (InputSizeSVal.getBaseKind() == SVal::NonLocKind && + InputSizeSVal.getSubKind() == nonloc::ConcreteIntKind) { + ExtVal = + InputSizeSVal.castAs().getValue().getExtValue(); + } else if (InputSizeSVal.getBaseKind() == SVal::LocKind && + InputSizeSVal.getSubKind() == loc::ConcreteIntKind) { + ExtVal = InputSizeSVal.castAs().getValue().getExtValue(); + } + return ExtVal; +} + +ProgramStateRef MagentaHandleChecker::allocateHandlesForArray( + const ElementRegion *EleRegion, const CallExpr *CE, int HandleCount, + ProgramStateRef State, CheckerContext &Ctx, bool EscapeOnAllocation) const { + if (!EleRegion) + return nullptr; + const SubRegion *SuperRegion = + dyn_cast(EleRegion->getSuperRegion()); + if (!SuperRegion) + return nullptr; + QualType EleType = dyn_cast(EleRegion)->getValueType(); + SValBuilder &Bldr = Ctx.getSValBuilder(); + MemRegionManager &MemRegionMgr = Bldr.getRegionManager(); + const LocationContext *LCtx = Ctx.getLocationContext(); + ASTContext &ASTCtx = State->getStateManager().getContext(); + + NonLoc StartIndexNonLoc = EleRegion->getIndex(); + int StartIndex = 0; + if (StartIndexNonLoc.getSubKind() == nonloc::ConcreteIntKind) + StartIndex = + StartIndexNonLoc.castAs().getValue().getExtValue(); + + for (int i = StartIndex; i < StartIndex + HandleCount; ++i) { + NonLoc AryIndex = Bldr.makeArrayIndex(i); + const ElementRegion *CurElementRegion = + MemRegionMgr.getElementRegion(EleType, AryIndex, SuperRegion, ASTCtx); + if (!CurElementRegion) + return nullptr; + Loc ElementLoc = Bldr.makeLoc(CurElementRegion); + DefinedOrUnknownSVal ConjuredHandleSVal = Bldr.conjureSymbolVal( + CurElementRegion, CE, LCtx, EleType, Ctx.blockCount()); + State = State->bindLoc(ElementLoc, ConjuredHandleSVal, LCtx); + SymbolRef SymHandle = ConjuredHandleSVal.getAsSymbol(); + if (!SymHandle) + return nullptr; + if (ConjuredHandleSVal.getBaseKind() == SVal::NonLocKind) + State = + State->assumeInclusiveRange(ConjuredHandleSVal, llvm::APSInt::get(1), + llvm::APSInt::get(INT_MAX), true); + HandleState HS = + HandleState::getAllocated(dyn_cast(CurElementRegion)); + if (EscapeOnAllocation) { + State = State->set(SymHandle, HandleState::getEscaped(&HS)); + } else { + State = State->set(SymHandle, HS); + } + } + return State; +} + +ProgramStateRef MagentaHandleChecker::releaseHandlesForArray( + const ElementRegion *EleRegion, const CallExpr *CE, int HandleCount, + ProgramStateRef State, CheckerContext &Ctx) const { + if (!EleRegion) + return nullptr; + const SubRegion *SuperRegion = + dyn_cast(EleRegion->getSuperRegion()); + if (!SuperRegion) + return nullptr; + QualType EleType = dyn_cast(EleRegion)->getValueType(); + SValBuilder &Bldr = Ctx.getSValBuilder(); + MemRegionManager &MemRegionMgr = Bldr.getRegionManager(); + ASTContext &ASTCtx = State->getStateManager().getContext(); + NonLoc StartIndexNonLoc = EleRegion->getIndex(); + int StartIndex = 0; + if (StartIndexNonLoc.getSubKind() == nonloc::ConcreteIntKind) + StartIndex = + StartIndexNonLoc.castAs().getValue().getExtValue(); + for (int i = StartIndex; i < StartIndex + HandleCount; ++i) { + NonLoc AryIndex = Bldr.makeArrayIndex(i); + const ElementRegion *CurElementRegion = + MemRegionMgr.getElementRegion(EleType, AryIndex, SuperRegion, ASTCtx); + if (!CurElementRegion) + return nullptr; + + SVal CurHandleSVal = State->getSVal(CurElementRegion); + SymbolRef SymHandle = CurHandleSVal.getAsSymbol(); + if (!SymHandle) + continue; + // If SymHandle is SymbolDerived type. It means no more handles in + // the rest of the array. Break + if (isa(SymHandle)) + break; + const HandleState *SymState = State->get(SymHandle); + if (!SymState) + continue; + if (SymState->isReleased()) { + reportDoubleRelease(SymHandle, CE->getSourceRange(), Ctx); + } else { + State = + State->set(SymHandle, HandleState::getReleased(SymState)); + } + } + return State; +} + bool MagentaHandleChecker::isLeaked(SymbolRef SymHandle, const HandleState &CurHandleState, bool isSymDead, @@ -779,6 +1015,18 @@ return StringRef(); } +bool MagentaHandleChecker::CheckExprIsZero(const Expr *ArgExpr, + CheckerContext &Ctx) const { + ProgramStateRef State = Ctx.getState(); + const LocationContext *LCtx = Ctx.getLocationContext(); + SVal ExprSVal = State->getSVal(ArgExpr, LCtx); + Optional DSVal = ExprSVal.getAs(); + if (DSVal.hasValue()) + return !State->assume(DSVal.getValue(), true); + + return false; +} + void MagentaHandleChecker::reportBug(SymbolRef Sym, ExplodedNode *ErrorNode, CheckerContext &C, const SourceRange *Range, Index: test/Analysis/mxhandle.c =================================================================== --- test/Analysis/mxhandle.c +++ test/Analysis/mxhandle.c @@ -20,6 +20,24 @@ extern mx_status_t mx_handle_close( MX_SYSCALL_PARAM_ATTR(handle_release_always) mx_handle_t handle); +extern mx_status_t mx_channel_read( +MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t handle, + uint32_t options, + void* bytes, + mx_handle_t* handles, + uint32_t num_bytes, + uint32_t num_handles, + uint32_t* actual_bytes, + uint32_t* actual_handles); + +extern mx_status_t mx_channel_write( +MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t handle, + uint32_t options, + const void* bytes, + uint32_t num_bytes, + const mx_handle_t* handles, + uint32_t num_handles); + // Escape is the default behavior for any function take a handle as parameter // It should work with/without explicit annotations void escapeMethod01(mx_handle_t *in); @@ -78,6 +96,44 @@ return sb; // no warning } +void checkNoLeak06(mx_handle_t handle) { + mx_handle_t handlebuf[4]; + uint32_t hcount; + mx_channel_read(handle, 0, NULL, handlebuf, 0, 4, 0, &hcount); + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } +} + +void checkNoLeak07(mx_handle_t handle, uint32_t hcount) { + mx_handle_t handlebuf[6]; + mx_channel_read(handle, 0, NULL, handlebuf, 0, hcount, 0, &hcount); + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } +} + +void checkNoLeak08(mx_handle_t handle) { + mx_handle_t handlebuf[4]; + uint32_t hcount; + mx_channel_read(handle, 0, NULL, handlebuf, 0, 4, 0, &hcount); + if (mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount) < 0) { + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } + } +} + +void checkNoLeak09(mx_handle_t handle, uint32_t hcount) { + mx_handle_t handlebuf[6]; + mx_channel_read(handle, 0, NULL, handlebuf, 0, hcount, 0, &hcount); + if (mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount) < 0) { + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } + } +} + void checkNoLeak10() { mx_handle_t sa, sb; if (mx_channel_create(0, &sa, &sb) < 0) { @@ -87,6 +143,18 @@ mx_handle_close(sb); } +void checkNoLeak11(mx_handle_t handle, uint32_t hcount) { + mx_handle_t handlebuf[6]; + mx_status_t r = mx_channel_read(handle, 0, NULL, + handlebuf, 0, hcount, 0, &hcount); + if (r < 0) { + return; + } + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } +} + void checkNoLeak12(int tag) { mx_handle_t sa, sb; if (mx_channel_create(0, &sa, &sb) < 0) { @@ -154,6 +222,52 @@ mx_handle_close(sb); // expected-warning {{Potential leak of handle}} } +void checkLeak03(mx_handle_t handle) { + mx_handle_t handlebuf[4]; + uint32_t hcount; + mx_status_t r = mx_channel_read(handle, 0, NULL, handlebuf, 0, 4, 0, &hcount); + if (r < 0) { + return; + } + for (int i = 0; i < hcount -1; i++) { + mx_handle_close(handlebuf[i]); + } +} // expected-warning {{Potential leak of handle}} + +void checkLeak04(mx_handle_t handle) { + mx_handle_t handlebuf[3]; + uint32_t hcount; + mx_status_t r = mx_channel_read(handle, 0, NULL, handlebuf, 0, 3, 0, &hcount); + if (r < 0) { + return; + } + if (mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount - 1) < 0) { + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } + } +} // expected-warning {{Potential leak of handle}} + +void checkLeak05(mx_handle_t handle, uint32_t hcount) { + mx_handle_t handlebuf[6]; + mx_status_t r = mx_channel_read( + handle, 0, NULL, handlebuf, 0, hcount, 0, &hcount); + if (r < 0) { + return; + } + if (mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount - 1) < 0) { + for (int i = 0; i < hcount; i++) { + mx_handle_close(handlebuf[i]); + } + } +} // expected-warning {{Potential leak of handle}} + +void checkLeak06(mx_handle_t handle, uint32_t hcount) { + mx_handle_t handlebuf[6]; + mx_channel_read(handle, 0, NULL, handlebuf, 0, hcount, 0, &hcount); + mx_channel_write(handle, 0, NULL, 0, handlebuf, hcount); // It may fail and handles are not released. +} // expected-warning {{Potential leak of handle}} + void checkLeak07() { mx_handle_t sa, sb; mx_channel_create(0, &sa, &sb);