[PATCH] D116597: [analyzer] Don't track function calls as control dependencies

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

Szelethus created this revision.
Szelethus added reviewers: dkrupp, NoQ, steakhal, gamesh411, martong, balazske.
Szelethus added a project: clang.
Herald added subscribers: manas, ASDenysPetrov, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, kristof.beyls, xazax.hun.
Szelethus requested review of this revision.
Herald added a subscriber: cfe-commits.

I recently evaluated ~150 of bug reports on open source projects relating to my GSoC'19 project <https://szelethus.github.io/gsoc2019/>, which was about tracking control dependencies that were relevant to a bug report.

Here is what I found: when the condition is a function call, the extra notes were almost always unimportant, and often times intrusive:

  void f(int *x) {
    x = nullptr;
    if (alwaysTrue()) // We don't need a whole lot of explanation
                      // here, the function name is good enough.
      *x = 5;
  }

It almost always boiled down to a few "Returning null pointer, which participates in a condition later", or similar notes. I struggled to find a single case where the notes revealed anything interesting or some previously hidden correlation, which is kind of the point of condition tracking.

This patch checks whether the condition is a function call, and if so, bails out.

The argument against the patch is the popular feedback we hear from some of our users, namely that they can never have too much information. I was specifically fishing for examples that display best that my contribution did more good than harm, so admittedly I set the bar high, and one can argue that there can be non-trivial trickery inside functions, and function names may not be that descriptive.

My argument for the patch is all those reports that got longer without any notable improvement in the report intelligibility. I think the few exceptional cases where this patch would remove notable information are an acceptable sacrifice in favor of more reports being leaner.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116597

Files:
  clang/include/clang/Analysis/CFG.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/return-value-guaranteed.cpp
  clang/test/Analysis/track-control-dependency-conditions.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116597.397269.patch
Type: text/x-patch
Size: 12910 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220104/a5be8ad0/attachment.bin>