[all-commits] [llvm/llvm-project] fd8e57: [analyzer] Don't track function calls as control d...

[Kristóf Umann via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: fd8e5762f86f0a602ec08eea5c4c86927faba6dc
      https://github.com/llvm/llvm-project/commit/fd8e5762f86f0a602ec08eea5c4c86927faba6dc
  Author: Kristóf Umann <dkszelethus at gmail.com>
  Date:   2022-04-08 (Fri, 08 Apr 2022)

  Changed paths:
    M clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    M clang/test/Analysis/return-value-guaranteed.cpp
    M clang/test/Analysis/track-control-dependency-conditions.cpp

  Log Message:
  -----------
  [analyzer] Don't track function calls as control dependencies

I recently evaluated ~150 of bug reports on open source projects relating to my
GSoC'19 project, 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.

Differential Revision: https://reviews.llvm.org/D116597