xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:261
+
+ // TODO: do we want to emit notes for escapes?
+ // do we want to emit notes for for error checks? I.e. on which branch
----------------
NoQ wrote:
> Will we ever emit a report against an escaped symbol? If not, there will
> never be a report to attach the note to.
I think there might be cases where we go from an escaped symbol to released and
so on. But you are right, it would not contribute to understanding the actual
bug report.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:262-263
+ // TODO: do we want to emit notes for escapes?
+ // do we want to emit notes for for error checks? I.e. on which branch
+ // do we consider the acquire/release success or fail.
+
----------------
NoQ wrote:
> If the function is implemented as an eager state split, having a note is
> great and easy to implement.
>
> If it's implemented as a Schrödinger state split, then i think we should
> still emit a note at the collapse point (especially given that it may happen
> in a deeper stack frame than the call that produced the symbol). But this
> makes me recognize the need for note tags in `evalAssume`.
I agree.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:521
+ if (Type.isSuppressOnSink()) {
+ const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
+ if (AcquireNode) {
----------------
NoQ wrote:
> I wonder how terrible would it be to allow bug visitors (or note tags, which
> are just an "API sugar" for bug visitors) to mutate the uniqueing location.
> Because such information is naturally available in the visitor.
>
> It most likely won't work because the information is necessary before the
> visitors are run. But it would have been nice though, so i feel as if we're
> missing an opportunity here.
On one hand, it would be nice :) On the other hand we could have more than one
visitor and it would be a hell to debug when the visitors disagree an the
uniqueing location. If we can come up with an API that is not that easy to
misuse, I am in favor of a change like that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70725/new/
https://reviews.llvm.org/D70725
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
