xazax.hun created this revision.
xazax.hun added reviewers: NoQ, dcoughlin, Szelethus, baloghadamsoftware, 
haowei.
xazax.hun added a project: clang.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet.

I was running the FuchsiaHandleCheck over some code and tried to collect and 
distill some false positives due to symbol escaping not working properly for 
integers.

After writing down all those test cases I feel like it is not always trivial 
what to do, so I decided to upload and annotate them so we can discuss here in 
depth what is the right way to deal with them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71041

Files:
  clang/test/Analysis/fuchsia_handle.cpp

Index: clang/test/Analysis/fuchsia_handle.cpp
===================================================================
--- clang/test/Analysis/fuchsia_handle.cpp
+++ clang/test/Analysis/fuchsia_handle.cpp
@@ -28,6 +28,7 @@
 
 void escape1(zx_handle_t *in);
 void escape2(zx_handle_t in);
+void (*escape3)(zx_handle_t) = escape2; 
 
 void use1(const zx_handle_t *in ZX_HANDLE_USE);
 void use2(zx_handle_t in ZX_HANDLE_USE);
@@ -114,14 +115,6 @@
   zx_handle_close(sb);
 } 
 
-void checkNoLeak07(int tag) {
-  zx_handle_t sa, sb;
-  if (zx_channel_create(0, &sa, &sb) < 0)
-    return;
-  escape1(&sa);
-  escape2(sb);
-}
-
 void checkLeak01(int tag) {
   zx_handle_t sa, sb;
   if (zx_channel_create(0, &sa, &sb)) // expected-note    {{Handle allocated here}}
@@ -204,3 +197,109 @@
   sc = t + sa;
   zx_handle_close(sc);
 }
+
+// Various escaping scenarios
+
+zx_handle_t *get_handle_address();
+
+void escape_last_pointer_die01() {
+  zx_handle_t sb;
+  // When the result of get_handle_address dies its pointee should be escaped,
+  // as it might be modified elsewhere.
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), &sb))
+    return;
+  zx_handle_close(sb);
+}
+
+struct object {
+  zx_handle_t *get_handle_address();
+};
+
+void escape_last_pointer_die02(object &o) {
+  zx_handle_t sb;
+  // Same as above.
+  if (zx_channel_create(0, o.get_handle_address(), &sb))
+    return;
+  zx_handle_close(sb);
+}
+
+void escape_last_pointer_die03(object o) {
+  zx_handle_t sb;
+  // Should we consider the pointee of get_handle_address escaped?
+  // Maybe we only should it consider escaped if o escapes?
+  if (zx_channel_create(0, o.get_handle_address(), &sb))
+    return;
+  zx_handle_close(sb);
+}
+
+void escape_through_call(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb) < 0)
+    return;
+  escape1(&sa);
+  if (tag)
+    escape2(sb);
+  else
+    escape3(sb);
+}
+
+struct have_handle {
+  zx_handle_t h;
+  zx_handle_t *hp;
+};
+
+void escape_through_store01(have_handle *handle) {
+  zx_handle_t sa;
+  if (zx_channel_create(0, &sa, handle->hp) < 0)
+    return;
+  handle->h = sa;
+}
+
+have_handle global;
+void escape_through_store02() {
+  zx_handle_t sa;
+  if (zx_channel_create(0, &sa, global.hp) < 0)
+    return;
+  global.h = sa;
+}
+
+have_handle escape_through_store03() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+    return {0, nullptr};
+  zx_handle_close(sb);
+  return {sa, nullptr};
+}
+
+void escape_structs(have_handle *);
+void escape_transitively01() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+    return;
+  have_handle hs[2];
+  hs[1] = {sa, &sb};
+  escape_structs(hs);
+}
+
+void escape_top_level_pointees(zx_handle_t *h) {
+  zx_handle_t h2;
+  if (zx_channel_create(0, h, &h2))
+    return;
+  zx_handle_close(h2);
+} // *h should be escaped here. Right?
+
+void quasi_escape() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, &sa, &sb))
+    return;
+  if (sa == 5) {
+    // Here we no longer have a symbol. Is that right?
+    // So we can no longer refer to the state in the GDM.
+    // So the value in some sense escaped our analysis.
+    zx_handle_close(sa);
+  } else
+    zx_handle_close(sa);
+  zx_handle_close(sb);
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to