[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Sooooo... note tags <https://reviews.llvm.org/D58367>?



================
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
----------------
Will we ever emit a report against an escaped symbol? If not, there will never be a report to attach the note to.


================
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.
+
----------------
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`.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:521
+  if (Type.isSuppressOnSink()) {
+    const ExplodedNode *AcquireNode = getAcquireSite(ErrorNode, Sym, C);
+    if (AcquireNode) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70725