[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
Charusso updated this revision to Diff 248110.
Charusso marked 4 inline comments as done.
Charusso added a comment.
Herald added subscribers: martong, steakhal.
- Make the tags robust and more unique.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73520/new/
https://reviews.llvm.org/D73520
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -14,6 +14,7 @@
#include "clang/Basic/Version.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringMap.h"
@@ -238,8 +239,9 @@
// FIXME: What should be reported here?
break;
case PathDiagnosticPiece::Event:
-return Piece.getTagStr() == "ConditionBRVisitor" ? Importance::Important
- : Importance::Essential;
+return Piece.getTag() == ConditionBRVisitor::getTag()
+ ? Importance::Important
+ : Importance::Essential;
case PathDiagnosticPiece::ControlFlow:
return Importance::Unimportant;
}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1,15 +1,10 @@
-//===- BugReporterVisitors.cpp - Helpers for reporting bugs ---===//
+//===- BugReporterVisitors.cpp - Bug explaining and suppression -*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===--===//
-//
-// This file defines a set of BugReporter "visitors" which can be used to
-// enhance the diagnostics reported for a bug.
-//
-//===--===//
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
#include "clang/AST/ASTContext.h"
@@ -82,18 +77,6 @@
return nullptr;
}
-/// Given that expression S represents a pointer that would be dereferenced,
-/// try to find a sub-expression from which the pointer came from.
-/// This is used for tracking down origins of a null or undefined value:
-/// "this is null because that is null because that is null" etc.
-/// We wipe away field and element offsets because they merely add offsets.
-/// We also wipe away all casts except lvalue-to-rvalue casts, because the
-/// latter represent an actual pointer dereference; however, we remove
-/// the final lvalue-to-rvalue cast before returning from this function
-/// because it demonstrates more clearly from where the pointer rvalue was
-/// loaded. Examples:
-/// x->y.z ==> x (lvalue)
-/// foo()->y.z ==> foo() (rvalue)
const Expr *bugreporter::getDerefExpr(const Stmt *S) {
const auto *E = dyn_cast(S);
if (!E)
@@ -362,21 +345,17 @@
SM(MmrMgr.getContext().getSourceManager()),
PP(MmrMgr.getContext().getPrintingPolicy()), TKind(TKind) {}
- void Profile(llvm::FoldingSetNodeID ) const override {
-static int Tag = 0;
-ID.AddPointer();
-ID.AddPointer(RegionOfInterest);
- }
-
- void *getTag() const {
-static int Tag = 0;
-return static_cast();
- }
-
PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
BugReporterContext ,
PathSensitiveBugReport ) override;
+ static const char *getTag() { return "NoStoreFuncVisitor"; }
+
+ void Profile(llvm::FoldingSetNodeID ) const override {
+ID.AddPointer(getTag());
+ID.AddPointer(RegionOfInterest);
+ }
+
private:
/// Attempts to find the region of interest in a given record decl,
/// by either following the base classes or fields.
@@ -837,10 +816,7 @@
R->getAs(), V));
}
- void* getTag() const {
-static int Tag = 0;
-return static_cast();
- }
+ static const char *getTag() { return "MacroNullReturnSuppressionVisitor"; }
void Profile(llvm::FoldingSetNodeID ) const override {
ID.AddPointer(getTag());
@@ -903,13 +879,10 @@
: CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
Options(Options), TKind(TKind) {}
- static void *getTag() {
-static int Tag = 0;
-return static_cast();
- }
+ static const char *getTag() { return
[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
Charusso added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:44
-/// BugReporterVisitors are used to add custom diagnostics along a path.
+/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug
+/// path. They also could serve false positive suppression.
Szelethus wrote:
> I suspect `\emoji` wasn't intentional here, given that it needs another
> argument :^)
Well, it is "So Pro" (- Apple) and I really like that. Someone wrote "along a
path", but we are definitely mentioning the path.
{F11475626}
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73520/new/
https://reviews.llvm.org/D73520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
Szelethus added a comment. Thanks! Bug report generation seems far less of a mess than is used to be :) Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:44 -/// BugReporterVisitors are used to add custom diagnostics along a path. +/// BugReporterVisitors are used to add custom diagnostics along a \emoji bug +/// path. They also could serve false positive suppression. I suspect `\emoji` wasn't intentional here, given that it needs another argument :^) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73520/new/ https://reviews.llvm.org/D73520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
NoQ added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:195
+
+ static const char *getTag() { return "FindLastStore"; }
+
Charusso wrote:
> I have made every tag a small-string.
This way it's harder to be sure that the tags are unique. And non-unique tags
may cause terrible pain while debugging.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:203
+ID.AddBoolean(EnableNullFPSuppression);
+ }
};
Charusso wrote:
> I have put every Profile into the header, so it is easier to see which
> members are not added to the Profile. I think the root cause of
> https://bugs.llvm.org/show_bug.cgi?id=42938 could be some issue with
> differentiating between visitors.
I like this!
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73520/new/
https://reviews.llvm.org/D73520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
Charusso marked 2 inline comments as done.
Charusso added inline comments.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:195
+
+ static const char *getTag() { return "FindLastStore"; }
+
I have made every tag a small-string.
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:203
+ID.AddBoolean(EnableNullFPSuppression);
+ }
};
I have put every Profile into the header, so it is easier to see which members
are not added to the Profile. I think the root cause of
https://bugs.llvm.org/show_bug.cgi?id=42938 could be some issue with
differentiating between visitors.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73520/new/
https://reviews.llvm.org/D73520
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73520: [analyzer] BugReporterVisitors: Refactor and documentation
Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
-
Repository:
rC Clang
https://reviews.llvm.org/D73520
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -14,6 +14,7 @@
#include "clang/Basic/Version.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringMap.h"
@@ -205,8 +206,9 @@
// FIXME: What should be reported here?
break;
case PathDiagnosticPiece::Event:
-return Piece.getTagStr() == "ConditionBRVisitor" ? Importance::Important
- : Importance::Essential;
+return Piece.getTag() == ConditionBRVisitor::getTag()
+ ? Importance::Important
+ : Importance::Essential;
case PathDiagnosticPiece::ControlFlow:
return Importance::Unimportant;
}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1,15 +1,10 @@
-//===- BugReporterVisitors.cpp - Helpers for reporting bugs ---===//
+//===- BugReporterVisitors.cpp - Bug explaining and suppression -*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===--===//
-//
-// This file defines a set of BugReporter "visitors" which can be used to
-// enhance the diagnostics reported for a bug.
-//
-//===--===//
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
#include "clang/AST/ASTContext.h"
@@ -82,18 +77,6 @@
return nullptr;
}
-/// Given that expression S represents a pointer that would be dereferenced,
-/// try to find a sub-expression from which the pointer came from.
-/// This is used for tracking down origins of a null or undefined value:
-/// "this is null because that is null because that is null" etc.
-/// We wipe away field and element offsets because they merely add offsets.
-/// We also wipe away all casts except lvalue-to-rvalue casts, because the
-/// latter represent an actual pointer dereference; however, we remove
-/// the final lvalue-to-rvalue cast before returning from this function
-/// because it demonstrates more clearly from where the pointer rvalue was
-/// loaded. Examples:
-/// x->y.z ==> x (lvalue)
-/// foo()->y.z ==> foo() (rvalue)
const Expr *bugreporter::getDerefExpr(const Stmt *S) {
const auto *E = dyn_cast(S);
if (!E)
@@ -362,21 +345,17 @@
SM(MmrMgr.getContext().getSourceManager()),
PP(MmrMgr.getContext().getPrintingPolicy()), TKind(TKind) {}
- void Profile(llvm::FoldingSetNodeID ) const override {
-static int Tag = 0;
-ID.AddPointer();
-ID.AddPointer(RegionOfInterest);
- }
-
- void *getTag() const {
-static int Tag = 0;
-return static_cast();
- }
-
PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
BugReporterContext ,
PathSensitiveBugReport ) override;
+ static const char *getTag() { return "NoStoreFunc"; }
+
+ void Profile(llvm::FoldingSetNodeID ) const override {
+ID.AddPointer(getTag());
+ID.AddPointer(RegionOfInterest);
+ }
+
private:
/// Attempts to find the region of interest in a given record decl,
/// by either following the base classes or fields.
@@ -837,10 +816,7 @@
R->getAs(), V));
}
- void* getTag() const {
-static int Tag = 0;
-return static_cast();
- }
+ static const char *getTag() { return "MacroNullRet"; }
void Profile(llvm::FoldingSetNodeID ) const override {
ID.AddPointer(getTag());
@@ -903,13 +879,10 @@
: CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
Options(Options), TKind(TKind) {}
- static void *getTag() {
-static int Tag = 0;
-return static_cast();
- }
+ static const char *getTag() { return "Return"; }
void
