haowei created this revision.
Herald added a subscriber: xazax.hun.
This patch adds "mx_create_sink" annotation support to MagentaHandleChecker to
address the false positives found in Magenta unit tests. After this patch, when
a call to a function contains this annotation, MagentaHandleChecker will create
a sink node which will suppress the handle leaks warnings if the leak is
post-dominated by this function (similar to a noreturn function).
The target problem it tries to solve can be illustrated in following example:
static bool handle_info_test(void) {
BEGIN_TEST;
mx_handle_t event;
ASSERT_EQ(mx_event_create(0u, &event), 0, "");
mx_handle_t duped;
mx_status_t status = mx_handle_duplicate(event, MX_RIGHT_SAME_RIGHTS,
&duped);
ASSERT_EQ(status, MX_OK, "");
// ....
}
Unlike the "assert" keyword in C++, the ASSERT_EQ macro will not terminate the
execution of the unit test program (in other words, it will not invoke a
noreturn function), instead it will return false and the error will be recorded
by the unit test runner. Therefore, before this patch, the MagentaHandleChecker
will report handle leaks on these assertion failures as the function reaches
return and symbol that contains the acquired handles are acquired. These
reports are not helpful as assertion failures in unit tests mean test failures.
With the help of this patch, we add a call to a function with "mx_create_sink"
annotation in the failure branch in definition "ASSERT_EQ". In this case, the
MagentaHandleChecker will create a sink node in each assertion failure and
suppress the warnings if the leaks are post-dominated by assertion failures.
https://reviews.llvm.org/D36475
Files:
lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
test/Analysis/mxhandle.c
Index: test/Analysis/mxhandle.c
===================================================================
--- test/Analysis/mxhandle.c
+++ test/Analysis/mxhandle.c
@@ -60,6 +60,8 @@
void noReturnFunc() __attribute__((__noreturn__));
+void sinkFunc() MX_SYSCALL_PARAM_ATTR(create_sink);
+
int leakingFunc(int argc, char* argv[]) {
mx_handle_t sa, sb;
mx_channel_create(0, &sa, &sb);
@@ -228,6 +230,14 @@
noReturnFunc(); // Should not report any bugs here
}
+void checkNoLeak17() {
+ mx_handle_t sa, sb;
+ if (mx_channel_create(0, &sa, &sb) < 0) {
+ return;
+ }
+ sinkFunc(); // Should not report any bugs here
+}
+
void checkLeak01() {
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
@@ -123,7 +123,9 @@
UNPROCESSED_FUNC,
// When a bug is found in function with this flag, do not report this bug
// Used to suppress known false positives.
- DONOTREPORT_FUNC
+ DONOTREPORT_FUNC,
+ // When a function has 'create_sink' annotation , create a sink node.
+ SINK_FUNC
};
enum AnnotationFlag {
@@ -144,7 +146,8 @@
// This value is not describing the possible return value of a annotated
// function. It basically tell the checker do not report any bugs found in
// this function. Used to suppress false positive reports.
- DONOTREPORT
+ DONOTREPORT,
+ CREATE_SINK
};
struct ArgSpec {
@@ -416,7 +419,8 @@
{"handle_escape", ESCAPE},
{"handle_use", NOESCAPE},
{"may_fail", BIFURCATE},
- {"suppress_warning", DONOTREPORT}};
+ {"suppress_warning", DONOTREPORT},
+ {"create_sink", CREATE_SINK}};
// Hard coded FuncSpec for mx_channel_read and mx_channel_write
// We currently don't have a clean way to annotate handles passed through
@@ -455,6 +459,10 @@
FuncKindMap[FuncDecl] == UNPROCESSED_FUNC ||
FuncKindMap[FuncDecl] == DONOTREPORT_FUNC)
return false;
+ if (FuncKindMap[FuncDecl] == SINK_FUNC) {
+ C.generateSink(C.getState(), C.getPredecessor());
+ return true;
+ }
}
// Check if the function has annotation and has already been processed before
if (FuncDeclMap.count(FuncDecl))
@@ -545,6 +553,11 @@
FuncKindMap[FD] = DONOTREPORT_FUNC;
return false;
}
+ if (FS.RetAction == CREATE_SINK) {
+ FuncKindMap[FD] = SINK_FUNC;
+ Ctx.generateSink(Ctx.getState(), Ctx.getPredecessor());
+ return true;
+ }
SmallVector<SVal, 3> invalidateSVal;
ProgramStateRef State = Ctx.getState();
ASTContext &ASTCtx = State->getStateManager().getContext();
@@ -1035,13 +1048,14 @@
HasValidAnnotation = true;
}
// Return type is not mx_status_t but has annotation other than
- // SYMBOLIC|DONOTREPORT Consider it as an error
+ // SYMBOLIC|DONOTREPORT|CREATE_SINK Consider it as an error
if (RetType.getAsString().find(SYSCALL_RETURN_TYPE_NAME) &&
- (FS.RetAction != SYMBOLIC && FS.RetAction != DONOTREPORT))
+ (FS.RetAction != SYMBOLIC && FS.RetAction != DONOTREPORT &&
+ FS.RetAction != CREATE_SINK))
return false;
if (FS.RetAction != BIFURCATE && FS.RetAction != SYMBOLIC &&
- FS.RetAction != DONOTREPORT)
+ FS.RetAction != DONOTREPORT && FS.RetAction != CREATE_SINK)
return false;
// Extract ParamDecl's annotation.
int ArgId = -1;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
