[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-30 Thread Balázs Kéri via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb22b97b3d0c0: [Analyzer] Use of BugType in 
DereferenceChecker (NFC). (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::LogicError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) 
const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), 
*report);
   C.emitReport(std::move(report));
 }
@@ -219,9 +211,10 @@
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(location);
 
-  // The explicit NULL case.
   if (nullState) {
 if (!notNullState) {
+  // We know that 'location' can only be null.  This is what
+  // we call an "explicit" null dereference.
   const Expr *expr = getDereferenceExpr(S);
   if (!suppressReport(expr)) {
 reportBug(nullState, expr, C);


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::LogicError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
   C.emitReport(std::move(report));
 }
@@ -219,9 +211,10 @@
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(location);
 
-  // The explicit NULL case.
   if (nullState) {
 if (!notNullState) {
+  // We know that 'location' can only be null.  This is what
+  // we call an "explicit" null dereference.
   const 

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-29 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-29 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko accepted this revision.
vsavchenko added a comment.

Awesome, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske marked 2 inline comments as done.
balazske added a comment.

The category string seems to be not really important, better not to change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-29 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 281631.
balazske added a comment.

Changed MemoryError back to LogicError.
Improved placement and text of changed comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::LogicError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) 
const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), 
*report);
   C.emitReport(std::move(report));
 }
@@ -219,9 +211,10 @@
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(location);
 
-  // The explicit NULL case.
   if (nullState) {
 if (!notNullState) {
+  // We know that 'location' can only be null.  This is what
+  // we call an "explicit" null dereference.
   const Expr *expr = getDereferenceExpr(S);
   if (!suppressReport(expr)) {
 reportBug(nullState, expr, C);


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::LogicError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::LogicError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
   C.emitReport(std::move(report));
 }
@@ -219,9 +211,10 @@
   ProgramStateRef notNullState, nullState;
   std::tie(notNullState, nullState) = state->assume(location);
 
-  // The explicit NULL case.
   if (nullState) {
 if (!notNullState) {
+  // We know that 'location' can only be null.  This is what
+  // we call an "explicit" null dereference.
   const Expr *expr = 

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-28 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> `BuiltinBug` is deprecated and we should be using `BugType` anyway

Like, i've never heard of `BuiltinBug` being deprecated but i'm pretty happy 
that it gets removed :)




Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:215
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.

vsavchenko wrote:
> Maybe "is definitely null" here? Otherwise it can be pretty confusing with a 
> double negation.
Also this entire comment should be inside the if-statement, on the same level 
as the next comment.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist:270
descriptionDereference of null pointer (loaded from 
variable x)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

balazske wrote:
> vsavchenko wrote:
> > I might've missed some discussions on that matter, so please correct me on 
> > that.
> > In my opinion, null pointer dereference is not a memory error.  Null 
> > address is not a correct memory and never was, so it is a logic error that 
> > a special value is being interpreted as the pointer to something.
> I can not decide which is better, "logic error" can be said for almost every 
> possible bug. Here the problem is at least related to memory handling. The 
> reference of undefined pointer value is "logic error" too (it is known that 
> the value was not initialized) but a memory error (try to access invalid or 
> valid but wrong address). Probably "pointer handling error" is better?
> (One value for bug category is not a good approach, it is never possible to 
> classify a bug to exactly one.)
Maybe let's not change it then, if there are no clear pros or cons? Users 
already got used to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-28 Thread Balázs Kéri via Phabricator via cfe-commits
balazske added inline comments.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist:270
descriptionDereference of null pointer (loaded from 
variable x)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

vsavchenko wrote:
> I might've missed some discussions on that matter, so please correct me on 
> that.
> In my opinion, null pointer dereference is not a memory error.  Null address 
> is not a correct memory and never was, so it is a logic error that a special 
> value is being interpreted as the pointer to something.
I can not decide which is better, "logic error" can be said for almost every 
possible bug. Here the problem is at least related to memory handling. The 
reference of undefined pointer value is "logic error" too (it is known that the 
value was not initialized) but a memory error (try to access invalid or valid 
but wrong address). Probably "pointer handling error" is better?
(One value for bug category is not a good approach, it is never possible to 
classify a bug to exactly one.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-28 Thread Valeriy Savchenko via Phabricator via cfe-commits
vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:215
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.

Maybe "is definitely null" here? Otherwise it can be pretty confusing with a 
double negation.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist:270
descriptionDereference of null pointer (loaded from 
variable x)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

I might've missed some discussions on that matter, so please correct me on that.
In my opinion, null pointer dereference is not a memory error.  Null address is 
not a correct memory and never was, so it is a logic error that a special value 
is being interpreted as the pointer to something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-28 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added reviewers: NoQ, vsavchenko.
Szelethus accepted this revision.
Szelethus added a comment.

Lets make sure that we invite others from the community to participate, here 
and on other patches as well. Otherwise, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-27 Thread Gabor Marton via Phabricator via cfe-commits
martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

AFAIK, BuiltinBug is deprecated and we should be using BugType anyway.
LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-24 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 280385.
balazske added a comment.

Fixed failed tests because change of bug category from "Logic error" to "Memory 
error".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
  clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/edges-new.mm.plist
  clang/test/Analysis/Inputs/expected-plists/inline-plist.c.plist
  clang/test/Analysis/Inputs/expected-plists/inline-unique-reports.c.plist
  clang/test/Analysis/Inputs/expected-plists/null-deref-path-notes.m.plist
  clang/test/Analysis/Inputs/expected-plists/objc-arc.m.plist
  
clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/plist-macros.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output-alternate.m.plist
  clang/test/Analysis/Inputs/expected-plists/plist-output.m.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Analysis/Inputs/expected-plists/unix-fns.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/plist-multi-file.c.plist
  
clang/test/Analysis/diagnostics/Inputs/expected-plists/undef-value-param.c.plist
  
clang/test/Analysis/inlining/Inputs/expected-plists/eager-reclamation-path-notes.c.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.c.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist

Index: clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
===
--- clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
+++ clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.m.plist
@@ -334,7 +334,7 @@
 

descriptionDereference of null pointer
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1506,7 +1506,7 @@
 

descriptionDereference of null pointer (loaded from variable x)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

Index: clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
===
--- clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
+++ clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
@@ -478,7 +478,7 @@
 

descriptionDereference of null pointer (loaded from variable p)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -718,7 +718,7 @@
 

descriptionDereference of null pointer (loaded from variable p)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1050,7 +1050,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1380,7 +1380,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -1710,7 +1710,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2042,7 +2042,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2377,7 +2377,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -2685,7 +2685,7 @@
 

descriptionDereference of null pointer (loaded from variable globalPtr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer
check_namecore.NullDereference

@@ -3810,7 +3810,7 @@
 

descriptionDereference of null pointer (loaded from field ptr)
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

2020-07-24 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, 
baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Use of BuiltinBug is replaced by BugType.
Class BuiltinBug seems to have no benefits and is confusing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84494

Files:
  clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", 
categories::MemoryError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::MemoryError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) 
const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), 
*report);
   C.emitReport(std::move(report));
 }
@@ -220,6 +212,8 @@
   std::tie(notNullState, nullState) = state->assume(location);
 
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.
   if (nullState) {
 if (!notNullState) {
   const Expr *expr = getDereferenceExpr(S);


Index: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -30,8 +30,9 @@
 : public Checker< check::Location,
   check::Bind,
   EventDispatcher > {
-  mutable std::unique_ptr BT_null;
-  mutable std::unique_ptr BT_undef;
+  BugType BT_Null{this, "Dereference of null pointer", categories::MemoryError};
+  BugType BT_Undef{this, "Dereference of undefined pointer value",
+   categories::MemoryError};
 
   void reportBug(ProgramStateRef State, const Stmt *S, CheckerContext ) const;
 
@@ -123,11 +124,6 @@
   if (!N)
 return;
 
-  // We know that 'location' cannot be non-null.  This is what
-  // we call an "explicit" null dereference.
-  if (!BT_null)
-BT_null.reset(new BuiltinBug(this, "Dereference of null pointer"));
-
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
@@ -180,7 +176,7 @@
   }
 
   auto report = std::make_unique(
-  *BT_null, buf.empty() ? BT_null->getDescription() : StringRef(buf), N);
+  BT_Null, buf.empty() ? BT_Null.getDescription() : StringRef(buf), N);
 
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
 
@@ -196,12 +192,8 @@
   // Check for dereference of an undefined value.
   if (l.isUndef()) {
 if (ExplodedNode *N = C.generateErrorNode()) {
-  if (!BT_undef)
-BT_undef.reset(
-new BuiltinBug(this, "Dereference of undefined pointer value"));
-
   auto report = std::make_unique(
-  *BT_undef, BT_undef->getDescription(), N);
+  BT_Undef, BT_Undef.getDescription(), N);
   bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *report);
   C.emitReport(std::move(report));
 }
@@ -220,6 +212,8 @@
   std::tie(notNullState, nullState) = state->assume(location);
 
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.
   if (nullState) {
 if (!notNullState) {
   const Expr *expr = getDereferenceExpr(S);