[PATCH] D83120: [Analyzer][StreamChecker] Using BugType::SuppressOnSink at resource leak report.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.


================
Comment at: clang/test/Analysis/stream.c:274-284
 // Check that "location uniqueing" works.
 // This results in reporting only one occurence of resource leak for a stream.
 void check_leak_noreturn_2() {
   FILE *F1 = tmpfile();
   if (!F1)
     return;
   if (Test == 1) {
----------------
balazske wrote:
> NoQ wrote:
> > balazske wrote:
> > > Szelethus wrote:
> > > > Why did this change? Is there a sink in the return branch?
> > > The change is probably because D83115. Because the "uniqueing" one resource leak is reported from the two possible, and the order changes somehow (probably not the shortest is found first).
> > The shortest should still be found first. I strongly suggest debugging this. Looks like a bug in suppress-on-sink.
> There is no code that ensures that the shortest path is reported. In this case one equivalence class is created with both bug reports. If `SuppressOnSink` is false the last one is returned from the list, otherwise the first one (`PathSensitiveBugReporter::findReportInEquivalenceClass`), this causes the difference (seems to be unrelated to D83115).
> There is no code that ensures that the shortest path is reported.

There absolutely should be -- See the summary of D65379 for more info, CTRL+F "shortest" helps quite a bit as well. For each bug report, we create a bug path (a path in the exploded graph from the root to the sepcific bug reports error node), and sort them by length.

It all feels super awkward -- `PathSensitiveBugReporter::findReportInEquivalenceClass` picks out a bug report from an equivalence class as you described, but that will only be reported if it is a `BasicBugReport` (as implemented by `PathSensitiveBugReporter::generateDiagnosticForConsumerMap`), otherwise it should go through the graph cutting process etc.

So at the end of the day, the shortest path should appear still? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83120