[PATCH] D36022: [analyzer] Add handle misuse analysis to MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei abandoned this revision.
haowei added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, Charusso, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.

An updated version was landed in f4c26d993bdc 
 . This 
diff is no longer needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D36022/new/

https://reviews.llvm.org/D36022

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36022: [analyzer] Add handle misuse analysis to MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei updated this revision to Diff 109237.
haowei added a comment.

Add 1 line fix for the constraint on allocated handle in function 
allocateSingleHandle at line 647


https://reviews.llvm.org/D36022

Files:
  lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
  test/Analysis/mxhandle.c

Index: test/Analysis/mxhandle.c
===
--- /dev/null
+++ test/Analysis/mxhandle.c
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.magenta.MagentaHandleChecker -analyzer-store=region -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int mx_status_t;
+typedef __typeof__(sizeof(int)) mx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define MX_SYSCALL_PARAM_ATTR(x)   __attribute__((annotate("mx_" #x)))
+#else
+#define MX_SYSCALL_PARAM_ATTR(x)   // no-op
+#endif
+
+extern mx_status_t mx_channel_create(
+uint32_t options,
+MX_SYSCALL_PARAM_ATTR(handle_acquire) mx_handle_t* out0,
+MX_SYSCALL_PARAM_ATTR(handle_acquire) mx_handle_t* out1);
+
+extern mx_status_t mx_handle_close(
+MX_SYSCALL_PARAM_ATTR(handle_release_always) mx_handle_t handle);
+
+// 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);
+
+void escapeMethod02(
+MX_SYSCALL_PARAM_ATTR(handle_escape)  mx_handle_t *in);
+
+void escapeMethod03(mx_handle_t in);
+
+void escapeMethod04(
+MX_SYSCALL_PARAM_ATTR(handle_escape)  mx_handle_t in);
+
+// Handle should not escape when mx_handle_use is present. It should work
+// for both value type and pointer type arguments.
+void useHandle01(
+MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t handle);
+
+void useHandle02(
+MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t *handle);
+
+// End of declaration
+
+void checkNoLeak01() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  mx_handle_t ay[2];
+  mx_channel_create(0, [0], [1]);
+  mx_handle_close(ay[0]);
+  mx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  mx_handle_t ay[2];
+  mx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++) {
+mx_handle_close(ay[i]);
+  }
+}
+
+mx_handle_t checkNoLeak04() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  mx_handle_close(sa);
+  return sb; // no warning
+}
+
+mx_handle_t checkNoLeak05(mx_handle_t *out1) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak10() {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak12(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod01();
+escapeMethod01();
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak13(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod02();
+escapeMethod02();
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak14(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod03(sa);
+escapeMethod03(sb);
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak15(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod04(sa);
+escapeMethod04(sb);
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkLeak01() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+} // expected-warning {{Potential leak of handle}}
+
+
+void checkLeak02(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkLeak07() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+
+  useHandle01(sa);
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkLeak08() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+
+  useHandle02();
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkDoubleRelease01(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  mx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  mx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  useHandle01(sa); // expected-warning {{Using a previously released handle}}
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
Index: lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
===
--- 

[PATCH] D36022: [analyzer] Add handle misuse analysis to MagentaHandleChecker

[Haowei Wu via Phabricator via cfe-commits]

haowei created this revision.
Herald added a subscriber: xazax.hun.

This commits add actual code for analyzing magenta handle misuse issues to 
MagentaHandleChecker, which is introduced in https://reviews.llvm.org/D35968. 
It supports magenta syscalls that acquires/release/use handle through pointers 
or values but not through arrays. The support for arrays will be added in a 
follow up commit.


https://reviews.llvm.org/D36022

Files:
  lib/StaticAnalyzer/Checkers/MagentaHandleChecker.cpp
  test/Analysis/mxhandle.c

Index: test/Analysis/mxhandle.c
===
--- /dev/null
+++ test/Analysis/mxhandle.c
@@ -0,0 +1,192 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.magenta.MagentaHandleChecker -analyzer-store=region -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int mx_status_t;
+typedef __typeof__(sizeof(int)) mx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define MX_SYSCALL_PARAM_ATTR(x)   __attribute__((annotate("mx_" #x)))
+#else
+#define MX_SYSCALL_PARAM_ATTR(x)   // no-op
+#endif
+
+extern mx_status_t mx_channel_create(
+uint32_t options,
+MX_SYSCALL_PARAM_ATTR(handle_acquire) mx_handle_t* out0,
+MX_SYSCALL_PARAM_ATTR(handle_acquire) mx_handle_t* out1);
+
+extern mx_status_t mx_handle_close(
+MX_SYSCALL_PARAM_ATTR(handle_release_always) mx_handle_t handle);
+
+// 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);
+
+void escapeMethod02(
+MX_SYSCALL_PARAM_ATTR(handle_escape)  mx_handle_t *in);
+
+void escapeMethod03(mx_handle_t in);
+
+void escapeMethod04(
+MX_SYSCALL_PARAM_ATTR(handle_escape)  mx_handle_t in);
+
+// Handle should not escape when mx_handle_use is present. It should work
+// for both value type and pointer type arguments.
+void useHandle01(
+MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t handle);
+
+void useHandle02(
+MX_SYSCALL_PARAM_ATTR(handle_use) mx_handle_t *handle);
+
+// End of declaration
+
+void checkNoLeak01() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  mx_handle_t ay[2];
+  mx_channel_create(0, [0], [1]);
+  mx_handle_close(ay[0]);
+  mx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  mx_handle_t ay[2];
+  mx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++) {
+mx_handle_close(ay[i]);
+  }
+}
+
+mx_handle_t checkNoLeak04() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  mx_handle_close(sa);
+  return sb; // no warning
+}
+
+mx_handle_t checkNoLeak05(mx_handle_t *out1) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak10() {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak12(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod01();
+escapeMethod01();
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak13(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod02();
+escapeMethod02();
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak14(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod03(sa);
+escapeMethod03(sb);
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkNoLeak15(int tag) {
+  mx_handle_t sa, sb;
+  if (mx_channel_create(0, , ) < 0) {
+return;
+  }
+  if (tag) {
+escapeMethod04(sa);
+escapeMethod04(sb);
+  }
+  mx_handle_close(sa);
+  mx_handle_close(sb);
+}
+
+void checkLeak01() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+} // expected-warning {{Potential leak of handle}}
+
+
+void checkLeak02(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkLeak07() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+
+  useHandle01(sa);
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkLeak08() {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+
+  useHandle02();
+  mx_handle_close(sb); // expected-warning {{Potential leak of handle}}
+}
+
+void checkDoubleRelease01(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  mx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  mx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  mx_handle_t sa, sb;
+  mx_channel_create(0, , );
+  if (tag) {
+mx_handle_close(sa);
+  }
+  useHandle01(sa); //