xazax.hun marked an inline comment as done.
xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624
+        if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+          if (!MR->hasStackStorage())
+            Escaped.push_back(State->getSVal(MR, Pointee));
----------------
xazax.hun wrote:
> NoQ wrote:
> > Ok, so this patch interacts with D71152 in a non-trivial manner. We should 
> > re-use the logic that decides whether an escape on bind occurs.
> Yeah, I just started to look into it and some skeletons fell out. 
> 
> Let's consider the following example:
> ```
> void leak() {
>   zx_handle_t sa, sb;
>   if (zx_channel_create(0, &sa, &sb))
>     return;
>   zx_handle_close(sb);
> } 
> ```
> 
> After using the same logic we can no longer detect the leak. The reason is, 
> after `zx_channel_create` both `sa` and `sb` regions are escaped (and they 
> become escaped during the conservative call) and then after the PostCall 
> callback, when we attached the traits to our symbols, we will trigger pointer 
> escape immediately, and lose the traits we just added.
> 
> I am not immediately sure what to do here. I feel like the main source of the 
> problem is the fact that `zx_channel_create` should not escape anything and 
> the checker has now way to communicate that to the analyzer core. Any ideas?
Suddenly, I see four ways forward, none of which is optimal. Hopefully, you can 
come up with something better.
1. Do not care about escaped local regions in this code path. So basically we 
would tune the invalidation down a bit. 
2. Make it possible for modeling checkers to prevent escaping. It would be 
great if there would be a way to do that without doing EvalCall.
3. Require the users to annotate out parameters. And we could assume that out 
parameters do not escape.
4. Do some heuristics like if a checker just attached some info in a PostCall, 
it is likely that we should not escape that symbol.


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

https://reviews.llvm.org/D71224



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

Reply via email to