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:
> 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.
Or:
5. Since output arguments might be more common than escapes in practice, maybe
we could just not escape local regions at all by default only if there is a
special annotation or a modelling check asks for it. So users would get one
more tool to suppress false positives but we would not overdo invalidation by
default.
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
