xazax.hun marked 2 inline comments as done.
xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:133-135
+ static HandleState getWithoutError(HandleState S) {
+ return HandleState(S.K, nullptr);
+ }
----------------
NoQ wrote:
> It already makes me mildly uncomfortable that our data is not "normalized",
> i.e. you can indicate the Schrödinger state by either adding an error symbol
> or changing from `Allocated` to `MaybeAllocated`. Do i understand it
> correctly that you're now adding a special state where the handle is still
> `MaybeAllocated` but there's no error symbol? Please comment this up.
Yeah, the reason is that, when the handle is a return value, we have no idea
where the error symbol is. This would only happen if someone do not follow the
API guidelines (in Fuchsia). I'll add a comment.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:415
+ if (CurItem.second.getErrorSym())
+ SymReaper.markLive(CurItem.first);
+ }
----------------
NoQ wrote:
> I think it should be possible to implement it without `checkLiveSymbols`,
> because i don't have any reasons to believe that you can find the handle
> symbol by looking at the error symbol, or vice versa, so i think they aren't
> supposed to keep each other alive.
>
> I.e., i think you can implement this by intentionally leaving zombie handles
> in your state until the respective error symbol dies. After all, you don't
> rely on any other metadata attached to your handles (eg., range constraints),
> you only need your map, right?
>
> I'm open to discussion here. I understand that intentional zombies sound
> scary, but they also look slightly more correct to me. //"The handle is
> definitely dead by now, but the successfulness of its birth is still a 'known
> unknown', so let's delay our warning and keep the corpse frozen until we find
> out if it was ever born to begin with"// - sounds about right :/ It's
> probably not a big deal in any case, but i'm curious WDYT.
I see, yeah this makes sense to me. I will try this approach out and report
back :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73151/new/
https://reviews.llvm.org/D73151
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
