haowei created this revision.
Herald added a subscriber: xazax.hun.
This commit adds support for syscalls that acquire/release handles in an array
for MagentaHandleChecker introduced in https://reviews.llvm.org/D35968 and
https://reviews.llvm.org/D36022.
Most magenta handle related syscalls will take a pointer to a local mx_handle_t
variable for handle acquisition and take a value of a local mx_handle variable
for handle release. A good example would be:
mx_status_t mx_channel_create(uint32_t options,
mx_handle_t* out0, mx_handle_t* out1);
and
mx_status_t mx_handle_close(mx_handle_t handle);
However, there are two exceptions, syscall
mx_status_t mx_channel_read(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);
Will read(acquire) "num_handles" of handles and save them to the array pointed
to by "handles". The actual number of acquired handles will be saved to the
variable pointed to by "actual_handles".
And syscall
mx_status_t mx_channel_write(mx_handle_t handle, uint32_t options,
void* bytes, uint32_t num_bytes,
mx_handle_t* handles, uint32_t num_handles)
Will release "num_handles" of handles in array "handles".
This patch adds support to handle acquire/release through arrays so these two
syscalls can be processed by this checker.
https://reviews.llvm.org/D36023
Files:
lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
test/Analysis/mxhandle.c
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);
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 Checker<check::DeadSymbols, check::PointerEscape, eval::Call,
@@ -285,6 +286,12 @@
// Extract magenta related annotation string from declarations
StringRef extractAnnotationWithoutPrefix(const Decl *D) const;
+ bool CheckExprIsZero(const Expr *ArgExpr, CheckerContext &Ctx) const;
+
+ int64_t getArgValueFromExpr(const Expr *ArgExpr, ProgramStateRef State,
+ const LocationContext *LCtx,
+ int64_t DefaultVal) const;
+
ProgramStateRef processHandleArrayArgWithFuncSpec(const CallExpr *CE,
ProgramStateRef State,
CheckerContext &Ctx,
@@ -297,6 +304,17 @@
ProgramStateRef State,
CheckerContext &Ctx) const;
+ ProgramStateRef allocateHandlesForArray(const ElementRegion *EleRegion,
+ const CallExpr *CE, int HandleCount,
+ ProgramStateRef State,
+ CheckerContext &Ctx,
+ bool EscapeOnAllocation) const;
+
+ ProgramStateRef releaseHandlesForArray(const ElementRegion *EleRegion,
+ const CallExpr *CE, int HandleCount,
+ ProgramStateRef State,
+ CheckerContext &Ctx) const;
+
// Return true only if it is certain that Sym will be Zero
static bool CheckSymbolConstraintToZero(SymbolRef Sym, ProgramStateRef State);
@@ -562,9 +580,115 @@
ProgramStateRef MagentaHandleChecker::processHandleArrayArgWithFuncSpec(
const CallExpr *CE, ProgramStateRef State, CheckerContext &Ctx,
ArgSpec &CurArgSpec) const {
- // A place holder. This function is used to process handle acquire/release
- // via an array. Its code will be included in follow up commits
- return nullptr;
+ if (CurArgSpec.getInputCArgIdx() == -1)
+ return nullptr;
+ const LocationContext *LCtx = Ctx.getLocationContext();
+ SValBuilder &Bldr = Ctx.getSValBuilder();
+ const Expr *HandleBufExpr = CE->getArg(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<ElementRegion>(MemHandleBuf)) {
+ if (isa<VarRegion>(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<ElementRegion>(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<ElementRegion>(MemHandleBuf)) {
+ if (isa<VarRegion>(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<HStateMap>(SymHandleBuf);
+ if (HS) {
+ if (HS->isReleased()) {
+ reportDoubleRelease(SymHandleBuf, CE->getSourceRange(), Ctx);
+ } else {
+ State = State->set<HStateMap>(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<ElementRegion>(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<HStateMap>(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<nonloc::ConcreteInt>().getValue().getExtValue();
+ } else if (InputSizeSVal.getBaseKind() == SVal::LocKind &&
+ InputSizeSVal.getSubKind() == loc::ConcreteIntKind) {
+ ExtVal = InputSizeSVal.castAs<loc::ConcreteInt>().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<SubRegion>(EleRegion->getSuperRegion());
+ if (!SuperRegion)
+ return nullptr;
+ QualType EleType = dyn_cast<TypedValueRegion>(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<nonloc::ConcreteInt>().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<MemRegion>(CurElementRegion));
+ if (EscapeOnAllocation) {
+ State = State->set<HStateMap>(SymHandle, HandleState::getEscaped(&HS));
+ } else {
+ State = State->set<HStateMap>(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<SubRegion>(EleRegion->getSuperRegion());
+ if (!SuperRegion)
+ return nullptr;
+ QualType EleType = dyn_cast<TypedValueRegion>(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<nonloc::ConcreteInt>().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<SymbolDerived>(SymHandle))
+ break;
+ const HandleState *SymState = State->get<HStateMap>(SymHandle);
+ if (!SymState)
+ continue;
+ if (SymState->isReleased()) {
+ reportDoubleRelease(SymHandle, CE->getSourceRange(), Ctx);
+ } else {
+ State =
+ State->set<HStateMap>(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<DefinedOrUnknownSVal> DSVal = ExprSVal.getAs<DefinedOrUnknownSVal>();
+ if (DSVal.hasValue())
+ return !State->assume(DSVal.getValue(), true);
+
+ return false;
+}
+
void MagentaHandleChecker::reportBug(SymbolRef Sym, ExplodedNode *ErrorNode,
CheckerContext &C,
const SourceRange *Range,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
