[PATCH] D24278: [analyzer] Extend bug reports with extra notes.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 6 13:20:56 PDT 2016


NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin, teemperor, v.g.vassilev.
NoQ added a subscriber: cfe-commits.

This patch allows injecting extra note-like diagnostics into bug reports, which are separate from path diagnostics. Previously, we could only display single-piece warnings and auto-generated path diagnostics; the latter could have been extended with the means of bug reporter visitors, however diagnostics injected by the visitors were still tied to the execution paths.

The primary purpose of this patch is to get the Clone checker integrated into the clang static analyzer's bug reporter mechanism. It is absolutely vital to this checker to display a warning that refers to multiple source ranges (code clones). This patch properly integrates the Clone checker, so that its warnings were visible in the analyzer GUIs such as scan-view.

Other checkers, even path-sensitive ones, could also benefit from the ability to add extra diagnostic pieces that are separate from path events. For example, if a bug report refers to a declaration of a class member, it is a good idea to highlight the relevant declaration, which may be far away from the execution path. This patch applies the new technique to the ObjCDealloc checker. Here we also see how the new extra notes interact with other path pieces.

HTML diagnostic consumer (`-analyzer-output=html`) is updated to use the new technique. Extra notes appear as blue planks (unlike the yellow path event planks and grey control flow planks), they are not numbered, and cannot be navigated through arrows, but they are included in the header. This represents the idea of how extra notes should ideally look like. The color is, of course, is chosen arbitrarily. Here are some examples of the newly generated HTML reports: {F2409277} {F2409279} {F2409280} {F2409281}

Null diagnostic consumer (with `-analyzer-output` unspecified) displays extra notes as "note:" diagnostics. It makes testing extra note pieces easier. Note that normal path notes are not displayed at this mode, so i'm not absolutely sure this is the right decision; however, this lets clone checker tests work without changes.

Text diagnostic consumer (with `-analyzer-output=text`) displays extra notes as "note:" diagnostics before the path diagnostics.

Plist diagnostic consumer was not updated yet(!) Currently, it ignores extra notes. This is because it is important to agree upon the format, so this decision will be delayed. Feedback on this matter is welcome :)

An option is added, `-analyzer-config extra-notes-as-events=true` (defaults to `false`), which converts extra notes to path events before sending to any diagnostic consumer. This allows to see the newly added notes in the Plist diagnostic consumer and display them without any changes to the viewer application (such as Xcode), however the looks would probably not be ideal or user-friendly (see also "HTML diagnostic consumer" above). The extra notes are put at the beginning, before any path events.

I slightly fixed the clone checker diagnostics. In particular, diagnostics cannot end with a period, and i capitalized the first letter. Also made slight changes to the suspicious clone structure, because path pieces require more than source ranges to operate (didn't look why though).

This patch includes a minor refactoring in `FlushReport()`, which introduces the `BugReport::isPathSensitive()` method and uses it. This should have no functional change, however it adds a stronger assertion, and, most importantly, this refactoring was useful for me to understand this piece of code and realize that i'm putting my code in the right place, so i decided that i should keep it.

https://reviews.llvm.org/D24278

Files:
  include/clang/Analysis/CloneDetection.h
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  lib/Analysis/CloneDetection.cpp
  lib/Rewrite/HTMLRewrite.cpp
  lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
  lib/StaticAnalyzer/Checkers/CloneChecker.cpp
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  lib/StaticAnalyzer/Core/PathDiagnostic.cpp
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/DeallocMissingRelease.m
  test/Analysis/PR2978.m
  test/Analysis/copypaste/blocks.cpp
  test/Analysis/copypaste/function-try-block.cpp
  test/Analysis/copypaste/functions.cpp
  test/Analysis/copypaste/macro-complexity.cpp
  test/Analysis/copypaste/macros.cpp
  test/Analysis/copypaste/objc-methods.m
  test/Analysis/copypaste/plist-diagnostics-extra-notes-as-events.cpp
  test/Analysis/copypaste/plist-diagnostics.cpp
  test/Analysis/copypaste/sub-sequences.cpp
  test/Analysis/copypaste/suspicious-clones.cpp
  test/Analysis/copypaste/text-diagnostics.cpp
  test/Analysis/properties.m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24278.70457.patch
Type: text/x-patch
Size: 62554 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160906/51142a93/attachment-0001.bin>


More information about the cfe-commits mailing list