[PATCH] D71041: [analyzer][discussion] Talk about escapes

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 7 inline comments as done.
xazax.hun added inline comments.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:211
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), &sb))
+    return;
----------------
This one and its variants were very popular in Fuchsia.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:232
+  // Maybe we only should it consider escaped if o escapes?
+  if (zx_channel_create(0, o.get_handle_address(), &sb))
+    return;
----------------
Here we will give a diagnoses for a leak at the moment. There are similar patterns in Fuchsia, but it is a false positive there, because `o` does store the handle and close it in its dtor.

But if we cannot inline the getter, the analyzer just assumes that this code leaks.

It is really hard to tell what should we do in this case. If we really always escape the returned symbol, we might basically end up escaping every return value from every method. 


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:243
+  if (tag)
+    escape2(sb);
+  else
----------------
This is the most basic case that is not working out of the box.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:261
+have_handle global;
+void escape_through_store02() {
+  zx_handle_t sa;
----------------
This one works.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:268
+
+have_handle escape_through_store03() {
+  zx_handle_t sa, sb;
----------------
This one also works.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:277
+void escape_structs(have_handle *);
+void escape_transitively01() {
+  zx_handle_t sa, sb;
----------------
This one also works.


================
Comment at: clang/test/Analysis/fuchsia_handle.cpp:301
+    // So the value in some sense escaped our analysis.
+    zx_handle_close(sa);
+  } else
----------------
This is also nasty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71041