[clang] [analyzer] Set and display CSA analysis entry points as notes on debugging (PR #84823)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 5 07:07:48 PDT 2024


================
@@ -788,7 +791,7 @@ class PathDiagnostic : public llvm::FoldingSetNode {
   PathDiagnostic(StringRef CheckerName, const Decl *DeclWithIssue,
                  StringRef bugtype, StringRef verboseDesc, StringRef shortDesc,
                  StringRef category, PathDiagnosticLocation LocationToUnique,
-                 const Decl *DeclToUnique,
+                 const Decl *DeclToUnique, const Decl *AnalysisEntryPoint,
----------------
steakhal wrote:

> It looks like you have a nice solution that doesn't force each checker to fill it in manually. 

TBH I'm pretty discontent with my implementation. This was basically the third attempt and the so far the cleanest - but it's still bad. The whole diagnostic pipeline is just a mess.

> I mean, this is a debug feature. Debug features are great but it's probably ok to have them slightly incorrect or incomplete if it means that they're no longer associated with increased complexity, with paying for something we don't use.

Do you imply I should relax the PathDiagnostic ctor precondition to allow nullptr entry point decls?
>From the name `Path` I judged, the entry point must be always present.

> Out of those AST checkers, how many are emitting diagnostics outside of the entry function / decl-with-issue function? If you append
> 
> ```
> [debug] This bubble's decl name is 'foo()'
> ```
> 
> to _every_ message bubble, would that entirely cover all your needs?

I'm not sure what "bubble" refers to, but if it's a synonym for "note" or "message", then yes - plus:
The entry point decl would be also beneficial for a primitive caching mechanism to know what reports we need to invalidate if a given decl changes across subsequent static analyzer runs on different git commits. I'll present the idea at EuroLLVM next week, but the gist of it is basically that we associate each diagnostic with a decl, hash that decl; and if subsequent runs hash that decl to the same value, then we can replay the diagnostic and skipping the entry point. To achieve this, we of course need to know the entry point.

> I can easily see how there could be a checker that always warns outside of the entry function. If that's the case then it's probably incredibly useful for debugging to quickly know which entry function inspired the warning. But at the same time I'm not sure I can think of a good example when the same information wouldn't be useful for the _user_ as well; maybe the users could benefit from a user-facing note too? (Somewhat similarly to how Clang explains template instantiation stack when diagnosing problems in individual template instantiations. (Something we cannot mimic in static analysis tools because the instantiation stack is already lost.))

I'm natural for having these notes (or a slightly differently phrased notes) denoting the start of the analysis.
If you think it would be genuinely useful, I could add it.

https://github.com/llvm/llvm-project/pull/84823


More information about the cfe-commits mailing list