[clang-tools-extra] d896c9f - Fix an unused variable warning

2021-11-15 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2021-11-15T15:45:43+01:00
New Revision: d896c9f40a22690d80cdf88152adbe591fd83d90

URL: 
https://github.com/llvm/llvm-project/commit/d896c9f40a22690d80cdf88152adbe591fd83d90
DIFF: 
https://github.com/llvm/llvm-project/commit/d896c9f40a22690d80cdf88152adbe591fd83d90.diff

LOG: Fix an unused variable warning

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 35fe51f11cd8..d29e631641dc 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -184,7 +184,7 @@ const Expr *digThroughConstructorsConversions(const Expr 
*E) {
   // If this is a conversion (as iterators commonly convert into their const
   // iterator counterparts), dig through that as well.
   if (const auto *ME = dyn_cast(E))
-if (const auto *D = dyn_cast(ME->getMethodDecl()))
+if (isa(ME->getMethodDecl()))
   return 
digThroughConstructorsConversions(ME->getImplicitObjectArgument());
   return E;
 }



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


[clang-tools-extra] 29a8d45 - [clang-tidy] Fix a crash in modernize-loop-convert around conversion operators

2021-11-15 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2021-11-15T13:11:29+01:00
New Revision: 29a8d45c5a239c9fa6b8634a9de3d655064816d1

URL: 
https://github.com/llvm/llvm-project/commit/29a8d45c5a239c9fa6b8634a9de3d655064816d1
DIFF: 
https://github.com/llvm/llvm-project/commit/29a8d45c5a239c9fa6b8634a9de3d655064816d1.diff

LOG: [clang-tidy] Fix a crash in modernize-loop-convert around conversion 
operators

modernize-loop-convert checks and fixes when a loop that iterates over the
elements of a container can be rewritten from a for(...; ...; ...) style into
the "new" C++11 for-range format. For that, it needs to parse the elements of
that loop, like its init-statement, such as ItType it = cont.begin().
modernize-loop-convert checks whether the loop variable is initialized by a
begin() member function.

When an iterator is initialized with a conversion operator (e.g. for
(const_iterator it = non_const_container.begin(); ...), attempts to retrieve the
name of the initializer expression resulted in an assert, as conversion
operators don't have a valid IdentifierInfo.

I fixed this by making digThroughConstructors dig through conversion operators
as well.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h

clang-tools-extra/test/clang-tidy/checkers/Inputs/modernize-loop-convert/structures.h
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-basic.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index e78f96d1f0c2..432d929057d1 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -304,8 +304,8 @@ StatementMatcher makePseudoArrayLoopMatcher() {
 static const Expr *getContainerFromBeginEndCall(const Expr *Init, bool IsBegin,
 bool *IsArrow, bool IsReverse) 
{
   // FIXME: Maybe allow declaration/initialization outside of the for loop.
-  const auto *TheCall =
-  dyn_cast_or_null(digThroughConstructors(Init));
+  const auto *TheCall = dyn_cast_or_null(
+  digThroughConstructorsConversions(Init));
   if (!TheCall || TheCall->getNumArgs() != 0)
 return nullptr;
 

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index 74dd4a3047ea..35fe51f11cd8 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -152,20 +152,21 @@ bool DeclFinderASTVisitor::VisitTypeLoc(TypeLoc TL) {
   return true;
 }
 
-/// Look through conversion/copy constructors to find the explicit
-/// initialization expression, returning it is found.
+/// Look through conversion/copy constructors and member functions to find the
+/// explicit initialization expression, returning it is found.
 ///
 /// The main idea is that given
 ///   vector v;
 /// we consider either of these initializations
 ///   vector::iterator it = v.begin();
 ///   vector::iterator it(v.begin());
+///   vector::const_iterator it(v.begin());
 /// and retrieve `v.begin()` as the expression used to initialize `it` but do
 /// not include
 ///   vector::iterator it;
 ///   vector::iterator it(v.begin(), 0); // if this constructor existed
 /// as being initialized from `v.begin()`
-const Expr *digThroughConstructors(const Expr *E) {
+const Expr *digThroughConstructorsConversions(const Expr *E) {
   if (!E)
 return nullptr;
   E = E->IgnoreImplicit();
@@ -178,8 +179,13 @@ const Expr *digThroughConstructors(const Expr *E) {
 E = ConstructExpr->getArg(0);
 if (const auto *Temp = dyn_cast(E))
   E = Temp->getSubExpr();
-return digThroughConstructors(E);
+return digThroughConstructorsConversions(E);
   }
+  // If this is a conversion (as iterators commonly convert into their const
+  // iterator counterparts), dig through that as well.
+  if (const auto *ME = dyn_cast(E))
+if (const auto *D = dyn_cast(ME->getMethodDecl()))
+  return 
digThroughConstructorsConversions(ME->getImplicitObjectArgument());
   return E;
 }
 
@@ -357,7 +363,7 @@ static bool isAliasDecl(ASTContext *Context, const Decl 
*TheDecl,
   bool OnlyCasts = true;
   const Expr *Init = VDecl->getInit()->IgnoreParenImpCasts();
   if (isa_and_nonnull(Init)) {
-Init = digThroughConstructors(Init);
+Init = digThroughConstructorsConversions(Init);
 OnlyCasts = false;
   }
   if (!Init)

diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 7dd2c8ef107f..8b288efd1b04 100644
--- 

[clang] 479ea2a - [analyzer] Check the checker name, rather than the ProgramPointTag when silencing a checker

2021-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2021-05-19T12:40:09+02:00
New Revision: 479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb

URL: 
https://github.com/llvm/llvm-project/commit/479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb
DIFF: 
https://github.com/llvm/llvm-project/commit/479ea2a8ed95544d2f5aaede34bfe5c298ae8bdb.diff

LOG: [analyzer] Check the checker name, rather than the ProgramPointTag when 
silencing a checker

The program point created by the checker, even if it is an error node,
might not be the same as the name under which the report is emitted.
Make sure we're checking the name of the checker, because thats what
we're silencing after all.

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

Added: 
clang/test/Analysis/silence-checkers-malloc.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/malloc.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index b64c0798d7e2..4608ee5cd40b 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1988,12 +1988,11 @@ PathDiagnosticBuilder::generate(const 
PathDiagnosticConsumer *PDC) const {
 
   const SourceManager  = getSourceManager();
   const AnalyzerOptions  = getAnalyzerOptions();
-  StringRef ErrorTag = ErrorNode->getLocation().getTag()->getTagDescription();
 
   // See whether we need to silence the checker/package.
   // FIXME: This will not work if the report was emitted with an incorrect tag.
   for (const std::string  : Opts.SilencedCheckersAndPackages) 
{
-if (ErrorTag.startswith(CheckerOrPackage))
+if (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
   return nullptr;
   }
 

diff  --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index 21e8e79e1a89..2bbf26ac2cda 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -1,7 +1,32 @@
-// RUN: %clang_analyze_cc1 -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -verify %s
-// RUN: %clang_analyze_cc1 -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-unknown-linux-gnu -w 
-analyzer-checker=core,alpha.deadcode.UnreachableCode,alpha.core.CastSize,unix.Malloc,cplusplus.NewDelete
 -analyzer-store=region -DTEST_INLINABLE_ALLOCATORS -verify %s
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -w -verify %s -DTEST_INLINABLE_ALLOCATORS \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
+// RUN:   -analyzer-checker=alpha.core.CastSize \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
 
 #include "Inputs/system-header-simulator-cxx.h"
 

diff  --git a/clang/test/Analysis/silence-checkers-malloc.cpp 
b/clang/test/Analysis/silence-checkers-malloc.cpp
new file mode 100644
index ..2f6a9dd2d5b8
--- /dev/null
+++ b/clang/test/Analysis/silence-checkers-malloc.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_analyze_cc1 -verify="no-silence" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete
+
+// RUN: %clang_analyze_cc1 -verify="unix-silenced" %s \
+// RUN:   -triple i386-unknown-linux-gnu \
+// RUN:   -analyzer-checker=core,apiModeling \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=cplusplus.NewDelete\
+// RUN:   -analyzer-config silence-checkers="unix"
+
+#include 

[clang] 33e731e - [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables that appear in an assignment

2021-02-12 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2021-02-12T16:19:20+01:00
New Revision: 33e731e62dae49d5143410248234963fc7a5e1db

URL: 
https://github.com/llvm/llvm-project/commit/33e731e62dae49d5143410248234963fc7a5e1db
DIFF: 
https://github.com/llvm/llvm-project/commit/33e731e62dae49d5143410248234963fc7a5e1db.diff

LOG: [analyzer][Liveness][NFC] Remove an unneeded pass to collect variables 
that appear in an assignment

Suppose you stumble across a DeclRefExpr in the AST, that references a VarDecl.
How would you know that that variable is written in the containing statement, or
not? One trick would be to ascend the AST through Stmt::getParent, and see
whether the variable appears on the left hand side of the assignment.

Liveness does something similar, but instead of ascending the AST, it descends
into it with a StmtVisitor, and after finding an assignment, it notes that the
LHS appears in the context of an assignemnt. However, as [1] demonstrates, the
analysis isn't ran on the AST of an entire function, but rather on CFG, where
the order of the statements, visited in order, would make it impossible to know
this information by descending.

void f() {
  int i;

  i = 5;
}

`-FunctionDecl 0x55a6e1b070b8  line:1:6 f 'void ()'
  `-CompoundStmt 0x55a6e1b07298 
|-DeclStmt 0x55a6e1b07220 
| `-VarDecl 0x55a6e1b071b8  col:7 used i 'int'
`-BinaryOperator 0x55a6e1b07278  'int' lvalue '='
  |-DeclRefExpr 0x55a6e1b07238  'int' lvalue Var 0x55a6e1b071b8 'i' 
'int'
  `-IntegerLiteral 0x55a6e1b07258  'int' 5

void f()
 [B2 (ENTRY)]
   Succs (1): B1

 [B1]
   1: int i;
   2: 5
   3: i
   4: [B1.3] = [B1.2]
   Preds (1): B2
   Succs (1): B0

 [B0 (EXIT)]
   Preds (1): B1

You can see that the arguments (rightfully so, they need to be evaluated first)
precede the assignment operator. For this reason, Liveness implemented a pass to
scan the CFG and note which variables appear in an assignment.

BUT.

This problem only exists if we traverse a CFGBlock in order. And Liveness in
fact does it reverse order. So a distinct pass is indeed unnecessary, we can
note the appearance of the assignment by the time we reach the variable.

[1] http://lists.llvm.org/pipermail/cfe-dev/2020-July/066330.html

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

Added: 


Modified: 
clang/include/clang/Analysis/CFG.h
clang/lib/Analysis/LiveVariables.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index 43fb523c863a..a0f8c6d21a67 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1307,6 +1307,12 @@ class CFG {
 
   iterator nodes_begin() { return iterator(Blocks.begin()); }
   iterator nodes_end() { return iterator(Blocks.end()); }
+
+  llvm::iterator_range nodes() { return {begin(), end()}; }
+  llvm::iterator_range const_nodes() const {
+return {begin(), end()};
+  }
+
   const_iterator nodes_begin() const { return const_iterator(Blocks.begin()); }
   const_iterator nodes_end() const { return const_iterator(Blocks.end()); }
 
@@ -1315,6 +1321,13 @@ class CFG {
   const_reverse_iteratorrbegin()  const{ return Blocks.rbegin(); }
   const_reverse_iteratorrend()const{ return Blocks.rend(); }
 
+  llvm::iterator_range reverse_nodes() {
+return {rbegin(), rend()};
+  }
+  llvm::iterator_range const_reverse_nodes() const {
+return {rbegin(), rend()};
+  }
+
   CFGBlock &getEntry() { return *Entry; }
   const CFGBlock &  getEntry()const{ return *Entry; }
   CFGBlock &getExit()  { return *Exit; }

diff  --git a/clang/lib/Analysis/LiveVariables.cpp 
b/clang/lib/Analysis/LiveVariables.cpp
index 8cdc4cc5bd61..6c601c290c92 100644
--- a/clang/lib/Analysis/LiveVariables.cpp
+++ b/clang/lib/Analysis/LiveVariables.cpp
@@ -325,6 +325,11 @@ static bool writeShouldKill(const VarDecl *VD) {
 }
 
 void TransferFunctions::VisitBinaryOperator(BinaryOperator *B) {
+  if (LV.killAtAssign && B->getOpcode() == BO_Assign) {
+if (const auto *DR = dyn_cast(B->getLHS()->IgnoreParens())) {
+  LV.inAssignment[DR] = 1;
+}
+  }
   if (B->isAssignmentOp()) {
 if (!LV.killAtAssign)
   return;
@@ -513,29 +518,8 @@ LiveVariables::computeLiveness(AnalysisDeclContext , 
bool killAtAssign) {
   llvm::BitVector everAnalyzedBlock(cfg->getNumBlockIDs());
 
   // FIXME: we should enqueue using post order.
-  for (CFG::const_iterator it = cfg->begin(), ei = cfg->end(); it != ei; ++it) 
{
-const CFGBlock *block = *it;
-worklist.enqueueBlock(block);
-
-// FIXME: Scan for DeclRefExprs using in the LHS of an assignment.
-// We need to do this because we lack context in the reverse analysis
-// to determine if a DeclRefExpr appears in such a context, and thus
-// doesn't constitute a "use".
-if (killAtAssign)
-  for 

[clang] 22e7182 - [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

2020-11-02 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-11-02T16:41:17+01:00
New Revision: 22e7182002b5396539c69603b1c8c924b5f661e7

URL: 
https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7
DIFF: 
https://github.com/llvm/llvm-project/commit/22e7182002b5396539c69603b1c8c924b5f661e7.diff

LOG: [analyzer][ReturnPtrRangeChecker] Fix a false positive on end() iterator

ReturnPtrRange checker emits a report if a function returns a pointer which
points out of the buffer. However, end() iterator of containers is always such
a pointer, so this always results a false positive report. This false positive
case is now eliminated.

This patch resolves these tickets:
https://bugs.llvm.org/show_bug.cgi?id=20929
https://bugs.llvm.org/show_bug.cgi?id=25226
https://bugs.llvm.org/show_bug.cgi?id=27701

Patch by Tibor Brunner!

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
clang/test/Analysis/misc-ps-region-store.m
clang/test/Analysis/return-ptr-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
index 599d4f306aa1..1a94ccdc2825 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -58,6 +58,11 @@ void ReturnPointerRangeChecker::checkPreStmt(const 
ReturnStmt *RS,
   DefinedOrUnknownSVal ElementCount = getDynamicElementCount(
   state, ER->getSuperRegion(), C.getSValBuilder(), ER->getValueType());
 
+  // We assume that the location after the last element in the array is used as
+  // end() iterator. Reporting on these would return too many false positives.
+  if (Idx == ElementCount)
+return;
+
   ProgramStateRef StInBound = state->assumeInBound(Idx, ElementCount, true);
   ProgramStateRef StOutBound = state->assumeInBound(Idx, ElementCount, false);
   if (StOutBound && !StInBound) {
@@ -70,7 +75,7 @@ void ReturnPointerRangeChecker::checkPreStmt(const ReturnStmt 
*RS,
 // types explicitly reference such exploit categories (when applicable).
 if (!BT)
   BT.reset(new BuiltinBug(
-  this, "Return of pointer value outside of expected range",
+  this, "Buffer overflow",
   "Returned pointer value points outside the original object "
   "(potential buffer overflow)"));
 

diff  --git a/clang/test/Analysis/misc-ps-region-store.m 
b/clang/test/Analysis/misc-ps-region-store.m
index b8710ff6f638..9c31324e7e59 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -463,7 +463,7 @@ void element_region_with_symbolic_superregion(int* p) {
 
 static int test_cwe466_return_outofbounds_pointer_a[10];
 int *test_cwe466_return_outofbounds_pointer() {
-  int *p = test_cwe466_return_outofbounds_pointer_a+10;
+  int *p = test_cwe466_return_outofbounds_pointer_a+11;
   return p; // expected-warning{{Returned pointer value points outside the 
original object}}
 }
 

diff  --git a/clang/test/Analysis/return-ptr-range.cpp 
b/clang/test/Analysis/return-ptr-range.cpp
index dd5dcd5d5d19..9763b51264e6 100644
--- a/clang/test/Analysis/return-ptr-range.cpp
+++ b/clang/test/Analysis/return-ptr-range.cpp
@@ -25,3 +25,47 @@ int *test_element_index_lifetime_with_local_ptr() {
   } while (0);
   return local_ptr; // expected-warning{{Returned pointer value points outside 
the original object (potential buffer overflow)}}
 }
+
+template 
+T* end(T ()[N]) {
+  return arr + N; // no-warning, because we want to avoid false positives on 
returning the end() iterator of a container.
+}
+
+void get_end_of_array() {
+  static int arr[10];
+  end(arr);
+}
+
+template 
+class Iterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  Iterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish; }
+};
+
+void use_iterable_object() {
+  Iterable<20> iter;
+  iter.end();
+}
+
+template 
+class BadIterable {
+  int buffer[N];
+  int *start, *finish;
+
+public:
+  BadIterable() : start(buffer), finish(buffer + N) {}
+
+  int* begin() { return start; }
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer 
value points outside the original object (potential buffer overflow)}}
+};
+
+void use_bad_iterable_object() {
+  BadIterable<20> iter;
+  iter.end();
+}



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


[clang] 032b78a - [analyzer] Revert the accidental commit of D82122

2020-07-24 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-24T21:33:18+02:00
New Revision: 032b78a0762bee129f33e4255ada6d374aa70c71

URL: 
https://github.com/llvm/llvm-project/commit/032b78a0762bee129f33e4255ada6d374aa70c71
DIFF: 
https://github.com/llvm/llvm-project/commit/032b78a0762bee129f33e4255ada6d374aa70c71.diff

LOG: [analyzer] Revert the accidental commit of D82122

Was accidentally squished into
rGb6cbe6cb0399d4671e5384dcc326af56bc6bd122. The assert fires on the code
snippet included in this commit.

More discussion can be found in https://reviews.llvm.org/D82598.

Added: 
clang/test/Analysis/live-stmts.mm

Modified: 
clang/lib/StaticAnalyzer/Core/Environment.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/Environment.cpp 
b/clang/lib/StaticAnalyzer/Core/Environment.cpp
index 9e6d79bb7dcc..1ccf4c6104a6 100644
--- a/clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -183,18 +183,12 @@ EnvironmentManager::removeDeadBindings(Environment Env,
  F.getTreeFactory());
 
   // Iterate over the block-expr bindings.
-  for (Environment::iterator I = Env.begin(), E = Env.end(); I != E; ++I) {
+  for (Environment::iterator I = Env.begin(), E = Env.end();
+   I != E; ++I) {
 const EnvironmentEntry  = I.getKey();
 const SVal  = I.getData();
 
-const bool IsBlkExprLive =
-SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext());
-
-assert((isa(BlkExpr.getStmt()) || !IsBlkExprLive) &&
-   "Only Exprs can be live, LivenessAnalysis argues about the liveness 
"
-   "of *values*!");
-
-if (IsBlkExprLive) {
+if (SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext())) {
   // Copy the binding to the new map.
   EBMapRef = EBMapRef.add(BlkExpr, X);
 

diff  --git a/clang/test/Analysis/live-stmts.mm 
b/clang/test/Analysis/live-stmts.mm
new file mode 100644
index ..a6ddd03ca5d8
--- /dev/null
+++ b/clang/test/Analysis/live-stmts.mm
@@ -0,0 +1,101 @@
+// RUN: %clang_analyze_cc1 -w -fblocks %s \
+// RUN:   -analyzer-checker=debug.DumpLiveStmts \
+// RUN:   2>&1 | FileCheck %s
+
+@interface Item
+// ...
+@end
+
+@interface Collection
+// ...
+@end
+
+typedef void (^Blk)();
+
+struct RAII {
+  Blk blk;
+
+public:
+  RAII(Blk blk): blk(blk) {}
+
+// CHECK: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+
+  ~RAII() { blk(); }
+
+// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+};
+
+void foo(Collection *coll) {
+  RAII raii(^{});
+  for (Item *item in coll) {}
+}
+// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B2 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclStmt {{.*}}
+// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 
'coll' 'Collection *'
+// CHECK-EMPTY:
+// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B3 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclStmt {{.*}}
+// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 
'coll' 'Collection *'
+// CHECK-EMPTY:
+// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B4 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclStmt {{.*}}
+// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-EMPTY:
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'Collection *' 
+// CHECK-NEXT: `-DeclRefExpr {{.*}} 'Collection *' lvalue ParmVar {{.*}} 
'coll' 'Collection *'
+// CHECK-EMPTY:
+// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B5 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-NEXT: DeclStmt {{.*}}
+// CHECK-NEXT: `-VarDecl {{.*}}  item 'Item *'
+// CHECK-EMPTY:
+// CHECK-NEXT: CompoundStmt {{.*}}
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B0 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+// CHECK-NEXT: [ B1 (live statements at block exit) ]
+// CHECK-EMPTY:
+// CHECK-EMPTY:
+



___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[clang] cfd6b4b - [analyzer] Don't allow hidden checkers to emit diagnostics

2020-07-06 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-06T15:34:51+02:00
New Revision: cfd6b4b811aa8bb193f744264e36e83de4bb3650

URL: 
https://github.com/llvm/llvm-project/commit/cfd6b4b811aa8bb193f744264e36e83de4bb3650
DIFF: 
https://github.com/llvm/llvm-project/commit/cfd6b4b811aa8bb193f744264e36e83de4bb3650.diff

LOG: [analyzer] Don't allow hidden checkers to emit diagnostics

Hidden checkers (those marked with Hidden in Checkers.td) are meant for
development purposes only, and are only displayed under
-analyzer-checker-help-developer, so users shouldn't see reports from them.

I moved StdLibraryFunctionsArg checker to the unix package from apiModeling as
it violated this rule. I believe this change doesn't deserve a different
revision because it is in alpha, and the name is so bad anyways I don't
immediately care where it is, because we'll have to revisit it soon enough.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
clang/test/Analysis/weak-dependencies.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index c1baa52a69b6..dc1269890f93 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -375,18 +375,6 @@ def TrustNonnullChecker : Checker<"TrustNonnull">,
 
 } // end "apiModeling"
 
-let ParentPackage = APIModelingAlpha in {
-
-def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
-  HelpText<"Check constraints of arguments of C standard library functions, "
-   "such as whether the parameter of isalpha is in the range [0, 255] "
-   "or is EOF.">,
-  Dependencies<[StdCLibraryFunctionsChecker]>,
-  WeakDependencies<[NonNullParamChecker]>,
-  Documentation;
-
-} // end "alpha.apiModeling"
-
 
//===--===//
 // Evaluate "builtin" functions.
 
//===--===//
@@ -545,6 +533,14 @@ def BlockInCriticalSectionChecker : 
Checker<"BlockInCriticalSection">,
   HelpText<"Check for calls to blocking functions inside a critical section">,
   Documentation;
 
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+  HelpText<"Check constraints of arguments of C standard library functions, "
+   "such as whether the parameter of isalpha is in the range [0, 255] "
+   "or is EOF.">,
+  Dependencies<[StdCLibraryFunctionsChecker]>,
+  WeakDependencies<[NonNullParamChecker]>,
+  Documentation;
+
 } // end "alpha.unix"
 
 
//===--===//

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index a832b90ddf0e..7df8dea56055 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2108,8 +2108,8 @@ void BuiltinBug::anchor() {}
 // Methods for BugReport and subclasses.
 
//===--===//
 
-static bool isDependency(const CheckerRegistryData ,
- StringRef CheckerName) {
+LLVM_ATTRIBUTE_USED static bool
+isDependency(const CheckerRegistryData , StringRef CheckerName) {
   for (const std::pair  : Registry.Dependencies) {
 if (Pair.second == CheckerName)
   return true;
@@ -2117,6 +2117,17 @@ static bool isDependency(const CheckerRegistryData 
,
   return false;
 }
 
+LLVM_ATTRIBUTE_USED static bool isHidden(const CheckerRegistryData ,
+ StringRef CheckerName) {
+  for (const CheckerInfo  : Registry.Checkers) {
+if (Checker.FullName == CheckerName)
+  return Checker.IsHidden;
+  }
+  llvm_unreachable(
+  "Checker name not found in CheckerRegistry -- did you retrieve it "
+  "correctly from CheckerManager::getCurrentCheckerName?");
+}
+
 PathSensitiveBugReport::PathSensitiveBugReport(
 const BugType , StringRef shortDesc, StringRef desc,
 const ExplodedNode *errorNode, PathDiagnosticLocation LocationToUnique,
@@ -2132,6 +2143,16 @@ PathSensitiveBugReport::PathSensitiveBugReport(
  "Some checkers depend on this one! We don't allow dependency "
  "checkers to emit warnings, because checkers should depend on "
  "*modeling*, not *diagnostics*.");
+
+  assert(
+  bt.getCheckerName().startswith("debug") ||
+  !isHidden(ErrorNode->getState()
+->getAnalysisManager()
+.getCheckerManager()
+

[clang] b295607 - [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

2020-07-06 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-06T14:51:37+02:00
New Revision: b2956076976cd0f435c46257f7cf9a0e5376d0fa

URL: 
https://github.com/llvm/llvm-project/commit/b2956076976cd0f435c46257f7cf9a0e5376d0fa
DIFF: 
https://github.com/llvm/llvm-project/commit/b2956076976cd0f435c46257f7cf9a0e5376d0fa.diff

LOG: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

The thrilling conclusion to the barrage of patches I uploaded lately! This is a
big milestone towards the goal set out in 
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html.
I hope to accompany this with a patch where the a coreModeling package is added,
from which package diagnostics aren't allowed either, is an implicit dependency
of all checkers, and the core package for the first time can be safely disabled.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 51565524db1e..27bc0dda1f1c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -135,7 +136,7 @@ class BugReport {
   SmallVector Fixits;
 
   BugReport(Kind kind, const BugType , StringRef desc)
-  : K(kind), BT(bt), Description(desc) {}
+  : BugReport(kind, bt, "", desc) {}
 
   BugReport(Kind K, const BugType , StringRef ShortDescription,
 StringRef Description)
@@ -369,16 +370,13 @@ class PathSensitiveBugReport : public BugReport {
 public:
   PathSensitiveBugReport(const BugType , StringRef desc,
  const ExplodedNode *errorNode)
-  : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
- : SourceRange()) {}
+  : PathSensitiveBugReport(bt, desc, desc, errorNode) {}
 
   PathSensitiveBugReport(const BugType , StringRef shortDesc, StringRef 
desc,
  const ExplodedNode *errorNode)
-  : BugReport(Kind::PathSensitive, bt, shortDesc, desc),
-ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
- : SourceRange()) {}
+  : PathSensitiveBugReport(bt, shortDesc, desc, errorNode,
+   /*LocationToUnique*/ {},
+   /*DeclToUnique*/ nullptr) {}
 
   /// Create a PathSensitiveBugReport with a custom uniqueing location.
   ///
@@ -391,11 +389,13 @@ class PathSensitiveBugReport : public BugReport {
  const ExplodedNode *errorNode,
  PathDiagnosticLocation LocationToUnique,
  const Decl *DeclToUnique)
-  : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : 
SourceRange()),
-UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
-assert(errorNode);
-  }
+  : PathSensitiveBugReport(bt, desc, desc, errorNode, LocationToUnique,
+   DeclToUnique) {}
+
+  PathSensitiveBugReport(const BugType , StringRef shortDesc, StringRef 
desc,
+ const ExplodedNode *errorNode,
+ PathDiagnosticLocation LocationToUnique,
+ const Decl *DeclToUnique);
 
   static bool classof(const BugReport *R) {
 return R->getKind() == Kind::PathSensitive;

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index f4fd495956aa..a832b90ddf0e 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -34,6 +34,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/CheckerRegistryData.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
@@ -2107,6 +2108,32 @@ void BuiltinBug::anchor() {}
 // Methods for BugReport and 

[clang] 690ff37 - [analyzer] Force dependency checkers to be hidden

2020-07-06 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-06T13:05:45+02:00
New Revision: 690ff37a286991f142584f25842e50c6cb23aba6

URL: 
https://github.com/llvm/llvm-project/commit/690ff37a286991f142584f25842e50c6cb23aba6
DIFF: 
https://github.com/llvm/llvm-project/commit/690ff37a286991f142584f25842e50c6cb23aba6.diff

LOG: [analyzer] Force dependency checkers to be hidden

Since strong dependencies aren't user-facing (its hardly ever legal to disable
them), lets enforce that they are hidden. Modeling checkers that aren't
dependencies are of course not impacted, but there is only so much you can do
against developers shooting themselves in the foot :^)

I also made some changes to the test files, reversing the "test" package for,
well, testing.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 0ceedfb8771b..c1baa52a69b6 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -130,7 +130,8 @@ def CallAndMessageModeling : 
Checker<"CallAndMessageModeling">,
"function/method call. For instance, if we can't reason about the "
"nullability of the implicit this parameter after a method call, "
"this checker conservatively assumes it to be non-null">,
-  Documentation;
+  Documentation,
+  Hidden;
 
 def CallAndMessageChecker : Checker<"CallAndMessage">,
   HelpText<"Check for logical errors for function calls and Objective-C "
@@ -220,7 +221,8 @@ def StackAddrEscapeChecker : Checker<"StackAddressEscape">,
 
 def DynamicTypePropagation : Checker<"DynamicTypePropagation">,
   HelpText<"Generate dynamic type information">,
-  Documentation;
+  Documentation,
+  Hidden;
 
 def NonnullGlobalConstantsChecker: Checker<"NonnilStringConstants">,
   HelpText<"Assume that const string-like globals are non-null">,
@@ -363,7 +365,8 @@ def StdCLibraryFunctionsChecker : 
Checker<"StdCLibraryFunctions">,
   "false",
   InAlpha>
   ]>,
-  Documentation;
+  Documentation,
+  Hidden;
 
 def TrustNonnullChecker : Checker<"TrustNonnull">,
   HelpText<"Trust that returns from framework methods annotated with _Nonnull "

diff  --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp 
b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index a68820523eaf..528284ca8985 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -310,6 +310,13 @@ template  void 
CheckerRegistry::resolveDependencies() {
DependencyIt->FullName == Entry.second &&
"Failed to find the dependency of a checker!");
 
+// We do allow diagnostics from unit test/example dependency checkers.
+assert((DependencyIt->FullName.startswith("test") ||
+DependencyIt->FullName.startswith("example") || IsWeak ||
+DependencyIt->IsHidden) &&
+   "Strong dependencies are modeling checkers, and as such "
+   "non-user facing! Mark them hidden in Checkers.td!");
+
 if (IsWeak)
   CheckerIt->WeakDependencies.emplace_back(&*DependencyIt);
 else

diff  --git a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp 
b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
index c7c75955f97f..60b8aafa0d84 100644
--- a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -42,17 +42,16 @@ class CustomChecker : public Checker {
 
 void addCustomChecker(AnalysisASTConsumer ,
   AnalyzerOptions ) {
-  AnOpts.CheckersAndPackages = {{"custom.CustomChecker", true}};
+  AnOpts.CheckersAndPackages = {{"test.CustomChecker", true}};
   AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry ) {
-Registry.addChecker("custom.CustomChecker", "Description",
-   "");
+Registry.addChecker("test.CustomChecker", "Description", 
"");
   });
 }
 
 TEST(RegisterCustomCheckers, RegisterChecker) {
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCode("void f() {;}", Diags));
-  EXPECT_EQ(Diags, "custom.CustomChecker:Custom diagnostic description\n");
+  EXPECT_EQ(Diags, "test.CustomChecker:Custom diagnostic description\n");
 }
 
 
//===--===//
@@ -121,7 +120,7 @@ bool shouldRegisterCheckerRegistrationOrderPrinter(const 
CheckerManager ) {
 void addCheckerRegistrationOrderPrinter(CheckerRegistry ) {
   

[clang] b6cbe6c - [analyzer][NFC] Move the data structures from CheckerRegistry to the Core library

2020-07-04 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-07-04T12:31:51+02:00
New Revision: b6cbe6cb0399d4671e5384dcc326af56bc6bd122

URL: 
https://github.com/llvm/llvm-project/commit/b6cbe6cb0399d4671e5384dcc326af56bc6bd122
DIFF: 
https://github.com/llvm/llvm-project/commit/b6cbe6cb0399d4671e5384dcc326af56bc6bd122.diff

LOG: [analyzer][NFC] Move the data structures from CheckerRegistry to the Core 
library

If you were around the analyzer for a while now, you must've seen a lot of
patches that awkwardly puts code from one library to the other:

* D75360 moves the constructors of CheckerManager, which lies in the Core
  library, to the Frontend library. Most the patch itself was a struggle along
  the library lines.
* D78126 had to be reverted because dependency information would be utilized
  in the Core library, but the actual data lied in the frontend.
  D78126#inline-751477 touches on this issue as well.

This stems from the often mentioned problem: the Frontend library depends on
Core and Checkers, Checkers depends on Core. The checker registry functions
(`registerMallocChecker`, etc) lie in the Checkers library in order to keep each
checker its own module. What this implies is that checker registration cannot
take place in the Core, but the Core might still want to use the data that
results from it (which checker/package is enabled, dependencies, etc).

D54436 was the patch that initiated this. Back in the days when CheckerRegistry
was super dumb and buggy, it implemented a non-documented solution to this
problem by keeping the data in the Core, and leaving the logic in the Frontend.
At the time when the patch landed, the merger to the Frontend made sense,
because the data hadn't been utilized anywhere, and the whole workaround without
any documentation made little sense to me.

So, lets put the data back where it belongs, in the Core library. This patch
introduces `CheckerRegistryData`, and turns `CheckerRegistry` into a short lived
wrapper around this data that implements the logic of checker registration. The
data is tied to CheckerManager because it is required to parse it.

Side note: I can't help but cringe at the fact how ridiculously awkward the
library lines are. I feel like I'm thinking too much inside the box, but I guess
this is just the price of keeping the checkers so modularized.

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

Added: 
clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h
clang/lib/StaticAnalyzer/Core/CheckerRegistryData.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/Environment.cpp
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 1f3c6a1b780b..d2f71baa56a4 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -43,6 +43,7 @@ class CallEvent;
 class CheckerBase;
 class CheckerContext;
 class CheckerRegistry;
+struct CheckerRegistryData;
 class ExplodedGraph;
 class ExplodedNode;
 class ExplodedNodeSet;
@@ -130,7 +131,7 @@ class CheckerManager {
   const Preprocessor *PP = nullptr;
   CheckerNameRef CurrentCheckerName;
   DiagnosticsEngine 
-  std::unique_ptr Registry;
+  std::unique_ptr RegistryData;
 
 public:
   // These constructors are defined in the Frontend library, because
@@ -152,8 +153,8 @@ class CheckerManager {
   : CheckerManager(Context, AOptions, PP, {}, {}) {}
 
   /// Constructs a CheckerManager without requiring an AST. No checker
-  /// registration will take place. Only useful for retrieving the
-  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  /// registration will take place. Only useful when one needs to print the
+  /// help flags through CheckerRegistryData, and the AST is unavalaible.
   CheckerManager(AnalyzerOptions , const LangOptions ,
  DiagnosticsEngine , ArrayRef plugins);
 
@@ -172,7 +173,9 @@ class CheckerManager {
 assert(PP);
 return *PP;
   }
-  const CheckerRegistry () const { return *Registry; }
+  const CheckerRegistryData () const {
+return *RegistryData;
+  }
   DiagnosticsEngine () const { return Diags; }
   ASTContext () const {
 assert(Context);

diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerRegistryData.h
new file mode 100644
index ..e7a7671a7576
--- /dev/null
+++ 

[clang] 1614e35 - [analyzer][MallocChecker] PR46253: Correctly recognize standard realloc

2020-06-16 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-06-16T17:50:06+02:00
New Revision: 1614e35408275478b24b45621395e2d9a2e9aa68

URL: 
https://github.com/llvm/llvm-project/commit/1614e35408275478b24b45621395e2d9a2e9aa68
DIFF: 
https://github.com/llvm/llvm-project/commit/1614e35408275478b24b45621395e2d9a2e9aa68.diff

LOG: [analyzer][MallocChecker] PR46253: Correctly recognize standard realloc

https://bugs.llvm.org/show_bug.cgi?id=46253

This is an obvious hack because realloc isn't any more affected than other
functions modeled by MallocChecker (or any user of CallDescription really),
but the nice solution will take some time to implement.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/malloc.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fb6d02b9ed60..d5b0a5b2220f 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -47,6 +47,7 @@
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
@@ -1037,9 +1038,44 @@ void MallocChecker::checkKernelMalloc(const CallEvent 
,
   C.addTransition(State);
 }
 
-void MallocChecker::checkRealloc(const CallEvent ,
- CheckerContext ,
+static bool isStandardRealloc(const CallEvent ) {
+  const FunctionDecl *FD = dyn_cast(Call.getDecl());
+  assert(FD);
+  ASTContext  = FD->getASTContext();
+
+  if (isa(FD))
+return false;
+
+  return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
+ FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
+ FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
+ AC.getSizeType();
+}
+
+static bool isGRealloc(const CallEvent ) {
+  const FunctionDecl *FD = dyn_cast(Call.getDecl());
+  assert(FD);
+  ASTContext  = FD->getASTContext();
+
+  if (isa(FD))
+return false;
+
+  return FD->getDeclaredReturnType().getDesugaredType(AC) == AC.VoidPtrTy &&
+ FD->getParamDecl(0)->getType().getDesugaredType(AC) == AC.VoidPtrTy &&
+ FD->getParamDecl(1)->getType().getDesugaredType(AC) ==
+ AC.UnsignedLongTy;
+}
+
+void MallocChecker::checkRealloc(const CallEvent , CheckerContext ,
  bool ShouldFreeOnFail) const {
+  // HACK: CallDescription currently recognizes non-standard realloc functions
+  // as standard because it doesn't check the type, or wether its a non-method
+  // function. This should be solved by making CallDescription smarter.
+  // Mind that this came from a bug report, and all other functions suffer from
+  // this.
+  // https://bugs.llvm.org/show_bug.cgi?id=46253
+  if (!isStandardRealloc(Call) && !isGRealloc(Call))
+return;
   ProgramStateRef State = C.getState();
   State = ReallocMemAux(C, Call, ShouldFreeOnFail, State, AF_Malloc);
   State = ProcessZeroAllocCheck(Call, 1, State);

diff  --git a/clang/test/Analysis/malloc.cpp b/clang/test/Analysis/malloc.cpp
index cc7fefe23d75..21e8e79e1a89 100644
--- a/clang/test/Analysis/malloc.cpp
+++ b/clang/test/Analysis/malloc.cpp
@@ -172,3 +172,21 @@ void test_delete_ZERO_SIZE_PTR() {
   // ZERO_SIZE_PTR is specially handled but only for malloc family
   delete Ptr; // expected-warning{{Argument to 'delete' is a constant address 
(16)}}
 }
+
+namespace pr46253_class {
+class a {
+  void *realloc(int, bool = false) { realloc(1); } // no-crash
+};
+} // namespace pr46253_class
+
+namespace pr46253_retty{
+void realloc(void *ptr, size_t size) { realloc(ptr, size); } // no-crash
+} // namespace pr46253_retty
+
+namespace pr46253_paramty{
+void *realloc(void **ptr, size_t size) { realloc(ptr, size); } // no-crash
+} // namespace pr46253_paramty
+
+namespace pr46253_paramty2{
+void *realloc(void *ptr, int size) { realloc(ptr, size); } // no-crash
+} // namespace pr46253_paramty2



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


[clang] 33fb9cb - [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

2020-06-12 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-06-12T14:59:48+02:00
New Revision: 33fb9cbe211d1b43d4b84edf34e11001f04cddf0

URL: 
https://github.com/llvm/llvm-project/commit/33fb9cbe211d1b43d4b84edf34e11001f04cddf0
DIFF: 
https://github.com/llvm/llvm-project/commit/33fb9cbe211d1b43d4b84edf34e11001f04cddf0.diff

LOG: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

The thrilling conclusion to the barrage of patches I uploaded lately! This is a
big milestone towards the goal set out in 
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html.
I hope to accompany this with a patch where the a coreModeling package is added,
from which package diagnostics aren't allowed either, is an implicit dependency
of all checkers, and the core package for the first time can be safely disabled.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 51565524db1e..27bc0dda1f1c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -135,7 +136,7 @@ class BugReport {
   SmallVector Fixits;
 
   BugReport(Kind kind, const BugType , StringRef desc)
-  : K(kind), BT(bt), Description(desc) {}
+  : BugReport(kind, bt, "", desc) {}
 
   BugReport(Kind K, const BugType , StringRef ShortDescription,
 StringRef Description)
@@ -369,16 +370,13 @@ class PathSensitiveBugReport : public BugReport {
 public:
   PathSensitiveBugReport(const BugType , StringRef desc,
  const ExplodedNode *errorNode)
-  : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
- : SourceRange()) {}
+  : PathSensitiveBugReport(bt, desc, desc, errorNode) {}
 
   PathSensitiveBugReport(const BugType , StringRef shortDesc, StringRef 
desc,
  const ExplodedNode *errorNode)
-  : BugReport(Kind::PathSensitive, bt, shortDesc, desc),
-ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
- : SourceRange()) {}
+  : PathSensitiveBugReport(bt, shortDesc, desc, errorNode,
+   /*LocationToUnique*/ {},
+   /*DeclToUnique*/ nullptr) {}
 
   /// Create a PathSensitiveBugReport with a custom uniqueing location.
   ///
@@ -391,11 +389,13 @@ class PathSensitiveBugReport : public BugReport {
  const ExplodedNode *errorNode,
  PathDiagnosticLocation LocationToUnique,
  const Decl *DeclToUnique)
-  : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : 
SourceRange()),
-UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
-assert(errorNode);
-  }
+  : PathSensitiveBugReport(bt, desc, desc, errorNode, LocationToUnique,
+   DeclToUnique) {}
+
+  PathSensitiveBugReport(const BugType , StringRef shortDesc, StringRef 
desc,
+ const ExplodedNode *errorNode,
+ PathDiagnosticLocation LocationToUnique,
+ const Decl *DeclToUnique);
 
   static bool classof(const BugReport *R) {
 return R->getKind() == Kind::PathSensitive;

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 7b72fae0fefe..795067cba582 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -135,7 +135,7 @@ class CheckerRegistry {
  "Invalid development status!");
 }
 
-LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+LLVM_DUMP_METHOD void dump() const;
 LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
   };
 
@@ -195,7 +195,7 @@ class CheckerRegistry {
 

[clang] e22f1c0 - [analyzer] Introduce weak dependencies to express *preferred* checker callback evaluation order

2020-06-12 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-06-12T14:08:38+02:00
New Revision: e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0

URL: 
https://github.com/llvm/llvm-project/commit/e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0
DIFF: 
https://github.com/llvm/llvm-project/commit/e22f1c02a27f4471af1b9ae3aa6d8324b86ab2d0.diff

LOG: [analyzer] Introduce weak dependencies to express *preferred* checker 
callback evaluation order

Checker dependencies were added D54438 to solve a bug where the checker names
were incorrectly registered, for example, InnerPointerChecker would incorrectly
emit diagnostics under the name MallocChecker, or vice versa [1]. Since the
system over the course of about a year matured, our expectations of what a role
of a dependency and a dependent checker should be crystallized a bit more --
D77474 and its summary, as well as a variety of patches in the stack
demonstrates how we try to keep dependencies to play a purely modeling role. In
fact, D78126 outright forbids diagnostics under a dependency checkers name.

These dependencies ensured the registration order and enabling only when all
dependencies are satisfied. This was a very "strong" contract however, that
doesn't fit the dependency added in D79420. As its summary suggests, this
relation is directly in between diagnostics, not modeling -- we'd prefer a more
specific warning over a general one.

To support this, I added a new dependency kind, weak dependencies. These are not
as strict of a contract, they only express a preference in registration order.
If a weak dependency isn't satisfied, the checker may still be enabled, but if
it is, checker registration, and transitively, checker callback evaluation order
is ensured.

If you are not familiar with the TableGen changes, a rather short description
can be found in the summary of D75360. A lengthier one is in D58065.

[1] https://www.youtube.com/watch?v=eqKeqHRAhQM

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

Added: 
clang/test/Analysis/weak-dependencies.c

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
clang/test/Analysis/analyzer-enabled-checkers.c
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
clang/utils/TableGen/ClangSACheckersEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td 
b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
index 6625d79559f5..98d26aaa637d 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/CheckerBase.td
@@ -112,6 +112,8 @@ class Checker {
   list CheckerOptions;
   // This field is optional.
   list   Dependencies;
+  // This field is optional.
+  list   WeakDependencies;
   bits<2> Documentation;
   Package ParentPackage;
   bit Hidden = 0;
@@ -122,8 +124,13 @@ class CheckerOptions opts> {
   list CheckerOptions = opts;
 }
 
-/// Describes dependencies in between checkers. For example, 
InnerPointerChecker
-/// relies on information MallocBase gathers.
+/// Describes (strong) dependencies in between checkers. This is important for
+/// modeling checkers, for example, MallocBase depends on the proper modeling 
of
+/// string operations, so it depends on CStringBase. A checker may only be
+/// enabled if none of its dependencies (transitively) is disabled. 
Dependencies
+/// are always registered before the dependent checker, and its checker
+/// callbacks are also evaluated sooner.
+/// One may only depend on a purely modeling checker (that emits no 
diagnostis).
 /// Example:
 ///   def InnerPointerChecker : Checker<"InnerPointer">,
 /// HelpText<"Check for inner pointers of C++ containers used after "
@@ -133,6 +140,24 @@ class Dependencies Deps = []> {
   list Dependencies = Deps;
 }
 
+/// Describes preferred registration and evaluation order in between checkers.
+/// Unlike strong dependencies, this expresses dependencies in between
+/// diagnostics, and *not* modeling. In the case of an unsatisfied (disabled)
+/// weak dependency, the dependent checker might still be registered. If the
+/// weak dependency is satisfied, it'll be registered, and its checker
+/// callbacks will be evaluated before the dependent checker. This can be used
+/// to ensure that a more specific warning would be displayed in place of a
+/// generic one, should multiple checkers detect the same bug. For example,
+/// non-null parameter bugs are detected by NonNullParamChecker due to the
+/// nonnull attribute, and StdLibraryFunctionsChecker as it models standard
+/// functions, and the former is the more specific one. While freeing a
+/// dangling pointer is a bug, if it is also a double free, 

[clang] d61b1f8 - [analyzer][NFC] Change checker dependency unit tests to check for the registration order

2020-06-12 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-06-12T12:43:56+02:00
New Revision: d61b1f8534c6ee0f9dad528cda641d1429920ed4

URL: 
https://github.com/llvm/llvm-project/commit/d61b1f8534c6ee0f9dad528cda641d1429920ed4
DIFF: 
https://github.com/llvm/llvm-project/commit/d61b1f8534c6ee0f9dad528cda641d1429920ed4.diff

LOG: [analyzer][NFC] Change checker dependency unit tests to check for the 
registration order

Exactly what it says on the tin! "Strong" dependencies are mentioned in contrast
to a new kind of dependency introduced in a followup patch.

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

Added: 


Modified: 
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp 
b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
index 51d52c45c1c8..c88dfca224b3 100644
--- a/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
+++ b/clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp
@@ -16,7 +16,9 @@
 #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
 #include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace ento {
@@ -60,7 +62,7 @@ class LocIncDecChecker : public Checker {
 public:
   void checkLocation(SVal Loc, bool IsLoad, const Stmt *S,
  CheckerContext ) const {
-auto UnaryOp = dyn_cast(S);
+const auto *UnaryOp = dyn_cast(S);
 if (UnaryOp && !IsLoad) {
   EXPECT_FALSE(UnaryOp->isIncrementOp());
 }
@@ -85,62 +87,90 @@ TEST(RegisterCustomCheckers, CheckLocationIncDec) {
 // Unsatisfied checker dependency
 
//===--===//
 
-class PrerequisiteChecker : public Checker {
+class CheckerRegistrationOrderPrinter
+: public Checker> {
+  std::unique_ptr BT =
+  std::make_unique(this, "Registration order");
+
 public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager ,
-BugReporter ) const {
-BR.EmitBasicReport(D, this, "Prerequisite", categories::LogicError,
-   "This is the prerequisite checker",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+  void checkPreStmt(const DeclStmt *DS, CheckerContext ) const {
+ExplodedNode *N = nullptr;
+N = C.generateErrorNode();
+llvm::SmallString<200> Buf;
+llvm::raw_svector_ostream OS(Buf);
+C.getAnalysisManager()
+.getCheckerManager()
+->getCheckerRegistry()
+.printEnabledCheckerList(OS);
+// Strip a newline off.
+auto R =
+std::make_unique(*BT, OS.str().drop_back(1), 
N);
+C.emitReport(std::move(R));
   }
 };
 
-void registerPrerequisiteChecker(CheckerManager ) {
-  mgr.registerChecker();
+void registerCheckerRegistrationOrderPrinter(CheckerManager ) {
+  mgr.registerChecker();
 }
 
-bool shouldRegisterPrerequisiteChecker(const CheckerManager ) {
-  return false;
+bool shouldRegisterCheckerRegistrationOrderPrinter(const CheckerManager ) {
+  return true;
 }
 
-class DependentChecker : public Checker {
-public:
-  void checkASTCodeBody(const Decl *D, AnalysisManager ,
-BugReporter ) const {
-BR.EmitBasicReport(D, this, "Dependent", categories::LogicError,
-   "This is the Dependent Checker",
-   PathDiagnosticLocation(D, Mgr.getSourceManager()), {});
+void addCheckerRegistrationOrderPrinter(CheckerRegistry ) {
+  Registry.addChecker(registerCheckerRegistrationOrderPrinter,
+  shouldRegisterCheckerRegistrationOrderPrinter,
+  "custom.RegistrationOrder", "Description", "", false);
+}
+
+#define UNITTEST_CHECKER(CHECKER_NAME, DIAG_MSG)   
\
+  class CHECKER_NAME : public Checker> {  
\
+std::unique_ptr BT =   
\
+std::make_unique(this, DIAG_MSG);  
\
+   
\
+  public:  
\
+void checkPreStmt(const DeclStmt *DS, CheckerContext ) const {}  
\
+  };   
\
+   
\
+  void register##CHECKER_NAME(CheckerManager ) {   
\
+mgr.registerChecker();   
\
+  }
\
+   
\
+  bool shouldRegister##CHECKER_NAME(const CheckerManager ) {   
\
+ 

[clang] 6bedfaf - [analyzer][MallocChecker] Fix the incorrect retrieval of the from argument in realloc()

2020-06-01 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-06-01T22:38:29+02:00
New Revision: 6bedfaf5200474f9a72b059f0d99dd39ece1c03e

URL: 
https://github.com/llvm/llvm-project/commit/6bedfaf5200474f9a72b059f0d99dd39ece1c03e
DIFF: 
https://github.com/llvm/llvm-project/commit/6bedfaf5200474f9a72b059f0d99dd39ece1c03e.diff

LOG: [analyzer][MallocChecker] Fix the incorrect retrieval of the from argument 
in realloc()

In the added testfile, the from argument was recognized as
{SymRegion{reg_$0},-1 S64b,long}
instead of
reg_$0.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/malloc.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fa69bc253fbd..fb6d02b9ed60 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2470,7 +2470,7 @@ MallocChecker::ReallocMemAux(CheckerContext , const 
CallEvent ,
   Kind = OAR_DoNotTrackAfterFailure;
 
 // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, 
size).
-SymbolRef FromPtr = arg0Val.getAsSymbol();
+SymbolRef FromPtr = arg0Val.getLocSymbolInBase();
 SVal RetVal = C.getSVal(CE);
 SymbolRef ToPtr = RetVal.getAsSymbol();
 assert(FromPtr && ToPtr &&

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index a8aabf9f9ace..714c73c3c793 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1848,6 +1848,13 @@ variable 'buf', which is not memory allocated by 
malloc() [unix.Malloc]}}
 crash_b() { crash_a(); } // no-crash
 // expected-warning@-1{{type specifier missing}} 
expected-warning@-1{{non-void}}
 
+long *global_a;
+void realloc_crash() {
+  long *c = global_a;
+  c--;
+  realloc(c, 8); // no-crash
+} // expected-warning{{Potential memory leak [unix.Malloc]}}
+
 // 
 // False negatives.
 



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


[clang] 77e1181 - [analyzer] Add dumps to CheckerRegistry

2020-05-31 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-31T22:53:02+02:00
New Revision: 77e1181df446b54391acad08512b540e174cf6e6

URL: 
https://github.com/llvm/llvm-project/commit/77e1181df446b54391acad08512b540e174cf6e6
DIFF: 
https://github.com/llvm/llvm-project/commit/77e1181df446b54391acad08512b540e174cf6e6.diff

LOG: [analyzer] Add dumps to CheckerRegistry

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 4e98ba2e10d2..c3494d0ebeef 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -13,6 +13,7 @@
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
 #include 
 #include 
 
@@ -133,6 +134,9 @@ class CheckerRegistry {
   DevelopmentStatus == "released") &&
  "Invalid development status!");
 }
+
+LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
   };
 
   using CmdLineOptionList = llvm::SmallVector;
@@ -189,6 +193,9 @@ class CheckerRegistry {
 
 // Used for lower_bound.
 explicit CheckerInfo(StringRef FullName) : FullName(FullName) {}
+
+LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
   };
 
   using StateFromCmdLine = CheckerInfo::StateFromCmdLine;
@@ -206,6 +213,9 @@ class CheckerRegistry {
 }
 
 explicit PackageInfo(StringRef FullName) : FullName(FullName) {}
+
+LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
   };
 
   using PackageInfoList = llvm::SmallVector;

diff  --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp 
b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index 62ac1ed252dd..f4d5db1e7a4b 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -27,6 +27,10 @@ using namespace clang;
 using namespace ento;
 using llvm::sys::DynamicLibrary;
 
+//===--===//
+// Utilities.
+//===--===//
+
 using RegisterCheckersFn = void (*)(CheckerRegistry &);
 
 static bool isCompatibleAPIVersion(const char *VersionString) {
@@ -86,6 +90,63 @@ static bool isInPackage(const CheckerRegistry::CheckerInfo 
,
   return false;
 }
 
+//===--===//
+// Methods of CmdLineOption, PackageInfo and CheckerInfo.
+//===--===//
+
+LLVM_DUMP_METHOD void
+CheckerRegistry::CmdLineOption::dumpToStream(llvm::raw_ostream ) const {
+  // The description can be just checked in Checkers.inc, the point here is to
+  // debug whether we succeeded in parsing it.
+  Out << OptionName << " (" << OptionType << ", "
+  << (IsHidden ? "hidden, " : "") << DevelopmentStatus << ") default: \""
+  << DefaultValStr;
+}
+
+static StringRef toString(CheckerRegistry::StateFromCmdLine Kind) {
+  switch (Kind) {
+  case CheckerRegistry::StateFromCmdLine::State_Disabled:
+return "Disabled";
+  case CheckerRegistry::StateFromCmdLine::State_Enabled:
+return "Enabled";
+  case CheckerRegistry::StateFromCmdLine::State_Unspecified:
+return "Unspecified";
+  }
+}
+
+LLVM_DUMP_METHOD void
+CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream ) const {
+  // The description can be just checked in Checkers.inc, the point here is to
+  // debug whether we succeeded in parsing it. Same with documentation uri.
+  Out << FullName << " (" << toString(State) << (IsHidden ? ", hidden" : "")
+  << ")\n";
+  Out << "  Options:\n";
+  for (const CmdLineOption  : CmdLineOptions) {
+Out << "";
+Option.dumpToStream(Out);
+Out << '\n';
+  }
+  Out << "  Dependencies:\n";
+  for (const CheckerInfo *Dependency : Dependencies) {
+Out << "  " << Dependency->FullName << '\n';
+  }
+}
+
+LLVM_DUMP_METHOD void
+CheckerRegistry::PackageInfo::dumpToStream(llvm::raw_ostream ) const {
+  Out << FullName << "\n";
+  Out << "  Options:\n";
+  for (const CmdLineOption  : CmdLineOptions) {
+Out << "";
+Option.dumpToStream(Out);
+Out << '\n';
+  }
+}
+
+//===--===//
+// Methods of CheckerRegistry.
+//===--===//
+
 

[clang] efd1a8e - [analyzer][MallocChecker] Make NewDeleteLeaks depend on DynamicMemoryModeling rather than NewDelete

2020-05-26 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2020-05-27T00:03:53+02:00
New Revision: efd1a8e66eaa13afff709ebf16ff6280caa82ead

URL: 
https://github.com/llvm/llvm-project/commit/efd1a8e66eaa13afff709ebf16ff6280caa82ead
DIFF: 
https://github.com/llvm/llvm-project/commit/efd1a8e66eaa13afff709ebf16ff6280caa82ead.diff

LOG: [analyzer][MallocChecker] Make NewDeleteLeaks depend on 
DynamicMemoryModeling rather than NewDelete

If you remember the mail [1] I sent out about how I envision the future of the
already existing checkers to look dependencywise, one my main points was that no
checker that emits diagnostics should be a dependency. This is more problematic
for some checkers (ahem, RetainCount [2]) more than for others, like this one.

The MallocChecker family is a mostly big monolithic modeling class some small
reporting checkers that only come to action when we are constructing a warning
message, after the actual bug was detected. The implication of this is that
NewDeleteChecker doesn't really do anything to depend on, so this change was
relatively simple.

The only thing that complicates this change is that FreeMemAux (MallocCheckers
method that models general memory deallocation) returns after calling a bug
reporting method, regardless whether the report was ever emitted (which may not
always happen, for instance, if the checker responsible for the report isn't
enabled). This return unfortunately happens before cleaning up the maps in the
GDM keeping track of the state of symbols (whether they are released, whether
that release was successful, etc). What this means is that upon disabling some
checkers, we would never clean up the map and that could've lead to false
positives, e.g.:

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/NewDelete-intersections.mm Line 66: Potential leak 
of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 73: Potential leak 
of memory pointed to by 'p'
  File clang/test/Analysis/NewDelete-intersections.mm Line 77: Potential leak 
of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 111: Undefined or 
garbage value returned to caller
  File clang/test/Analysis/NewDelete-checker-test.cpp Line 200: Potential leak 
of memory pointed to by 'p'

error: 'warning' diagnostics seen but not expected:
  File clang/test/Analysis/new.cpp Line 137: Potential leak of memory pointed 
to by 'x'
There two possible approaches I had in mind:

Make bug reporting methods of MallocChecker returns whether they succeeded, and
proceed with the rest of FreeMemAux if not,
Halt execution with a sink node upon failure. I decided to go with this, as
described in the code.
As you can see from the removed/changed test files, before the big checker
dependency effort landed, there were tests to check for all the weird
configurations of enabled/disabled checkers and their messy interactions, I
largely repurposed these.

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html
[2] http://lists.llvm.org/pipermail/cfe-dev/2019-August/063205.html

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/NewDelete-intersections.mm
clang/test/Analysis/new.cpp

Removed: 
clang/test/Analysis/Malloc+NewDelete_intersections.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index bc4b7d00e2d4..2ba3881c6135 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -556,13 +556,13 @@ def NewDeleteChecker : Checker<"NewDelete">,
 
 def NewDeleteLeaksChecker : Checker<"NewDeleteLeaks">,
   HelpText<"Check for memory leaks. Traces memory managed by new/delete.">,
-  Dependencies<[NewDeleteChecker]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation;
 
 def PlacementNewChecker : Checker<"PlacementNew">,
   HelpText<"Check if default placement new is provided with pointers to "
"sufficient storage capacity">,
-  Dependencies<[NewDeleteChecker]>,
+  Dependencies<[DynamicMemoryModeling]>,
   Documentation;
 
 def CXXSelfAssignmentChecker : Checker<"SelfAssignment">,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a7c62a7e8046..fa69bc253fbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -684,41 +684,42 @@ class MallocChecker
   static bool SummarizeValue(raw_ostream , SVal V);
   static bool SummarizeRegion(raw_ostream , const MemRegion *MR);
 
-  void 

[clang] 6f54318 - [analyzer][RetainCount] Remove the CheckOSObject option

2020-05-26 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-26T13:22:58+02:00
New Revision: 6f5431846bbf3270d8fc605324e8843c5aaf579b

URL: 
https://github.com/llvm/llvm-project/commit/6f5431846bbf3270d8fc605324e8843c5aaf579b
DIFF: 
https://github.com/llvm/llvm-project/commit/6f5431846bbf3270d8fc605324e8843c5aaf579b.diff

LOG: [analyzer][RetainCount] Remove the CheckOSObject option

As per http://lists.llvm.org/pipermail/cfe-dev/2019-August/063215.html, lets 
get rid of this option.

It presents 2 issues that have bugged me for years now:

* OSObject is NOT a boolean option. It in fact has 3 states:
  * osx.OSObjectRetainCount is enabled but OSObject it set to false: RetainCount
regards the option as disabled.
  * sx.OSObjectRetainCount is enabled and OSObject it set to true: RetainCount
regards the option as enabled.
  * osx.OSObjectRetainCount is disabled: RetainCount regards the option as
disabled.
* The hack involves directly modifying AnalyzerOptions::ConfigTable, which
  shouldn't even be public in the first place.

This still isn't really ideal, because it would be better to preserve the option
and remove the checker (we want visible checkers to be associated with
diagnostics, and hidden options like this one to be associated with changing how
the modeling is done), but backwards compatibility is an issue.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/test-separate-retaincount.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index f0ad8326929e..bc4b7d00e2d4 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1094,15 +1094,6 @@ def NSErrorChecker : Checker<"NSError">,
 def RetainCountChecker : Checker<"RetainCount">,
   HelpText<"Check for leaks and improper reference count management">,
   CheckerOptions<[
-CmdLineOption,
 CmdLineOptiongetValue() == Value;
-  return false;
-}
-
 void ento::registerRetainCountChecker(CheckerManager ) {
   auto *Chk = Mgr.getChecker();
   Chk->TrackObjCAndCFObjects = true;
-  Chk->TrackNSCFStartParam = getOption(Mgr.getAnalyzerOptions(),
-   "TrackNSCFStartParam",
-   "true");
+  Chk->TrackNSCFStartParam = Mgr.getAnalyzerOptions().getCheckerBooleanOption(
+  Mgr.getCurrentCheckerName(), "TrackNSCFStartParam");
 }
 
 bool ento::shouldRegisterRetainCountChecker(const CheckerManager ) {
@@ -1509,10 +1494,7 @@ bool ento::shouldRegisterRetainCountChecker(const 
CheckerManager ) {
 
 void ento::registerOSObjectRetainCountChecker(CheckerManager ) {
   auto *Chk = Mgr.getChecker();
-  if (!getOption(Mgr.getAnalyzerOptions(),
- "CheckOSObject",
- "false"))
-Chk->TrackOSObjects = true;
+  Chk->TrackOSObjects = true;
 }
 
 bool ento::shouldRegisterOSObjectRetainCountChecker(const CheckerManager ) 
{

diff  --git a/clang/test/Analysis/analyzer-config.c 
b/clang/test/Analysis/analyzer-config.c
index 778467387382..cb3d40688e91 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -99,7 +99,6 @@
 // CHECK-NEXT: 
optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = 
false
 // CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
 // CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
-// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
 // CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2

diff  --git a/clang/test/Analysis/test-separate-retaincount.cpp 
b/clang/test/Analysis/test-separate-retaincount.cpp
index 621e1d120bbb..41efad452e5a 100644
--- a/clang/test/Analysis/test-separate-retaincount.cpp
+++ b/clang/test/Analysis/test-separate-retaincount.cpp
@@ -5,10 +5,6 @@
 // RUN: %clang_analyze_cc1 -std=c++14 -DNO_OS_OBJECT -verify %s \
 // RUN:   -analyzer-checker=core,osx \
 // RUN:   -analyzer-disable-checker osx.OSObjectRetainCount
-//
-// RUN: %clang_analyze_cc1 -std=c++14 -DNO_OS_OBJECT -verify %s \
-// RUN:   -analyzer-checker=core,osx \
-// RUN:   -analyzer-config "osx.cocoa.RetainCount:CheckOSObject=false"
 
 #include "os_object_base.h"
 



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


[clang] 429f030 - Revert "[analyzer] Change the default output type to PD_TEXT_MINIMAL in the frontend, error if an output loc is missing for PathDiagConsumers that need it"

2020-05-22 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-22T20:18:16+02:00
New Revision: 429f03089951d62fb370026905c87f1f25cf220f

URL: 
https://github.com/llvm/llvm-project/commit/429f03089951d62fb370026905c87f1f25cf220f
DIFF: 
https://github.com/llvm/llvm-project/commit/429f03089951d62fb370026905c87f1f25cf220f.diff

LOG: Revert "[analyzer] Change the default output type to PD_TEXT_MINIMAL in 
the frontend, error if an output loc is missing for PathDiagConsumers that need 
it"

This reverts commit fe1a3a7e8c8be33968b9a768666489823dabab10.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 
clang/test/Analysis/output_types.cpp



diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index fdca8532ab53..d010a7dfb2de 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -358,9 +358,6 @@ def err_analyzer_checker_option_invalid_input : Error<
   "invalid input for checker option '%0', that expects %1">;
 def err_analyzer_checker_incompatible_analyzer_option : Error<
   "checker cannot be enabled with analyzer option '%0' == %1">;
-def err_analyzer_missing_output_loc : Error<
-  "analyzer output type '%0' requires an output %1 to be specified with "
-  "-o ">;
 
 def err_drv_invalid_hvx_length : Error<
   "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index 373caa30bbc9..d2df24a6e21b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -206,7 +206,7 @@ class AnalyzerOptions : public 
RefCountedBase {
   ConfigTable Config;
   AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
-  AnalysisDiagClients AnalysisDiagOpt = PD_TEXT_MINIMAL;
+  AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
 
   std::string AnalyzeSpecificFunction;

diff  --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 8ce3aa2e081e..184fdcfb3d4b 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -21,7 +21,6 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
-#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -138,14 +137,18 @@ void ento::createHTMLDiagnosticConsumer(
 const std::string , const Preprocessor ,
 const cross_tu::CrossTranslationUnitContext ) {
 
-  if (OutputDir.empty()) {
-PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
-  << "html" << "directory";
+  // FIXME: HTML is currently our default output type, but if the output
+  // directory isn't specified, it acts like if it was in the minimal text
+  // output mode. This doesn't make much sense, we should have the minimal text
+  // as our default. In the case of backward compatibility concerns, this could
+  // be preserved with -analyzer-config-compatibility-mode=true.
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
+
+  // TODO: Emit an error here.
+  if (OutputDir.empty())
 return;
-  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
-  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
@@ -153,11 +156,9 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
 const std::string , const Preprocessor ,
 const cross_tu::CrossTranslationUnitContext ) {
 
-  if (OutputDir.empty()) {
-PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
-  << "html-single-file" << "directory";
+  // TODO: Emit an error here.
+  if (OutputDir.empty())
 return;
-  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, false));
   createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
@@ -167,13 +168,6 @@ void ento::createPlistHTMLDiagnosticConsumer(
 AnalyzerOptions , PathDiagnosticConsumers ,
 const std::string , const Preprocessor ,
 const cross_tu::CrossTranslationUnitContext ) {
-
-  if (prefix.empty()) {
-

[clang] 1c8f999 - [analyzer][CallAndMessage] Add checker options for each bug type

2020-05-21 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-21T15:31:37+02:00
New Revision: 1c8f999e0b59731a4214f76528f83e4196e1fcc3

URL: 
https://github.com/llvm/llvm-project/commit/1c8f999e0b59731a4214f76528f83e4196e1fcc3
DIFF: 
https://github.com/llvm/llvm-project/commit/1c8f999e0b59731a4214f76528f83e4196e1fcc3.diff

LOG: [analyzer][CallAndMessage] Add checker options for each bug type

iAs listed in the summary D77846, we have 5 different categories of bugs we're
checking for in CallAndMessage. I think the documentation placed in the code
explains my thought process behind my decisions quite well.

A non-obvious change I had here is removing the entry for
CallAndMessageUnInitRefArg. In fact, I removed the CheckerNameRef typed field
back in D77845 (it was dead code), so that checker didn't really exist in any
meaningful way anyways.

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

Added: 
clang/test/Analysis/call-and-message.c
clang/test/Analysis/call-and-message.cpp
clang/test/Analysis/call-and-message.m
clang/test/Analysis/call-and-message.mm

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
clang/test/Analysis/PR40625.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/analyzer-enabled-checkers.c
clang/test/Analysis/exercise-ps.c
clang/test/Analysis/uninit-const.c
clang/test/Analysis/uninit-const.cpp

Removed: 
clang/test/Analysis/reference.mm
clang/test/Analysis/uninit-msg-expr.m



diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index d06c64ad118b..93c4d964d772 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -122,14 +122,70 @@ def FuchsiaAlpha : Package<"fuchsia">, 
ParentPackage;
 
 let ParentPackage = Core in {
 
-def DereferenceChecker : Checker<"NullDereference">,
-  HelpText<"Check for dereferences of null pointers">,
+def CallAndMessageModeling : Checker<"CallAndMessageModeling">,
+  HelpText<"Responsible for essential modeling and assumptions after a "
+   "function/method call. For instance, if we can't reason about the "
+   "nullability of the implicit this parameter after a method call, "
+   "this checker conservatively assumes it to be non-null">,
   Documentation;
 
 def CallAndMessageChecker : Checker<"CallAndMessage">,
   HelpText<"Check for logical errors for function calls and Objective-C "
"message expressions (e.g., uninitialized arguments, null function "
"pointers)">,
+  CheckerOptions<[
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+  ]>,
+  Documentation,
+  Dependencies<[CallAndMessageModeling]>;
+
+def DereferenceChecker : Checker<"NullDereference">,
+  HelpText<"Check for dereferences of null pointers">,
   Documentation;
 
 def NonNullParamChecker : Checker<"NonNullParamChecker">,
@@ -211,13 +267,6 @@ def SizeofPointerChecker : Checker<"SizeofPtr">,
   HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
   Documentation;
 
-def CallAndMessageUnInitRefArg : Checker<"CallAndMessageUnInitRefArg">,
-  HelpText<"Check for logical errors for function calls and Objective-C "
-   "message expressions (e.g., uninitialized arguments, null function "
-   "pointers, and pointer to undefined variables)">,
-  Dependencies<[CallAndMessageChecker]>,
-  Documentation;
-
 def TestAfterDivZeroChecker : Checker<"TestAfterDivZero">,
   HelpText<"Check for division by variable that is later compared against 0. "
"Either the comparison is useless or there is division by zero.">,
@@ -295,7 +344,7 @@ let ParentPackage = APIModeling in {
 
 def StdCLibraryFunctionsChecker : Checker<"StdCLibraryFunctions">,
   HelpText<"Improve modeling of the C standard library functions">,
-  Dependencies<[NonNullParamChecker, CallAndMessageChecker]>,
+  Dependencies<[NonNullParamChecker, CallAndMessageModeling]>,
   CheckerOptions<[
 CmdLineOption BT_call_few_args;
 
 public:
-  enum CheckKind { CK_CallAndMessageUnInitRefArg, CK_NumCheckKinds };
+  // These correspond with the checker options. Looking at other checkers such
+  // as MallocChecker and CStringChecker, this is similar as to how they pull
+  // off having a modeling class, but emitting diagnostics under a smaller
+  // checker's name that can be safely disabled without disturbing the
+  // underlaying modeling engine.
+  // The reason behind having *checker options* rather then actual *checkers*
+  // here is that CallAndMessage is among the oldest checkers out there, and 
can
+  // be 

[clang] eeff1a9 - [analyzer][CallAndMessage][NFC] Split up checkPreCall

2020-05-21 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-21T12:54:56+02:00
New Revision: eeff1a970a6bb09d3c046313e229e2871929cd63

URL: 
https://github.com/llvm/llvm-project/commit/eeff1a970a6bb09d3c046313e229e2871929cd63
DIFF: 
https://github.com/llvm/llvm-project/commit/eeff1a970a6bb09d3c046313e229e2871929cd63.diff

LOG: [analyzer][CallAndMessage][NFC] Split up checkPreCall

The patch aims to use CallEvents interface in a more principled manner, and also
to highlight what this checker really does. It in fact checks for 5 different
kinds of errors (from checkPreCall, that is):

 * Invalid function pointer related errors
 * Call of methods from an invalid C++ this object
 * Function calls with incorrect amount of parameters
 * Invalid arguments for operator delete
 * Pass of uninitialized values to pass-by-value parameters

In a previous patch I complained that this checker is responsible for emitting
a lot of different diagnostics all under core.CallAndMessage's name, and this
patch shows where we could start to assign different diagnostics to different
entities.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 00fbe2c8dcb7..58eb329d1bf9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -60,6 +60,25 @@ class CallAndMessageChecker
 
   void checkPreCall(const CallEvent , CheckerContext ) const;
 
+  ProgramStateRef checkFunctionPointerCall(const CallExpr *CE,
+   CheckerContext ,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXMethodCall(const CXXInstanceCall *CC,
+ CheckerContext ,
+ ProgramStateRef State) const;
+
+  ProgramStateRef checkParameterCount(const CallEvent , CheckerContext ,
+  ProgramStateRef State) const;
+
+  ProgramStateRef checkCXXDeallocation(const CXXDeallocatorCall *DC,
+   CheckerContext ,
+   ProgramStateRef State) const;
+
+  ProgramStateRef checkArgInitializedness(const CallEvent ,
+  CheckerContext ,
+  ProgramStateRef State) const;
+
 private:
   bool PreVisitProcessArg(CheckerContext , SVal V, SourceRange ArgRange,
   const Expr *ArgEx, int ArgumentNumber,
@@ -309,123 +328,131 @@ bool 
CallAndMessageChecker::PreVisitProcessArg(CheckerContext ,
   return false;
 }
 
-void CallAndMessageChecker::checkPreCall(const CallEvent ,
- CheckerContext ) const {
-  ProgramStateRef State = C.getState();
-  if (const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr())) {
-const Expr *Callee = CE->getCallee()->IgnoreParens();
-const LocationContext *LCtx = C.getLocationContext();
-SVal L = State->getSVal(Callee, LCtx);
-
-if (L.isUndef()) {
-  if (!BT_call_undef)
-BT_call_undef.reset(new BuiltinBug(
-this, "Called function pointer is an uninitialized pointer 
value"));
-  emitBadCall(BT_call_undef.get(), C, Callee);
-  return;
-}
+ProgramStateRef CallAndMessageChecker::checkFunctionPointerCall(
+const CallExpr *CE, CheckerContext , ProgramStateRef State) const {
 
-ProgramStateRef StNonNull, StNull;
-std::tie(StNonNull, StNull) =
-State->assume(L.castAs());
+  const Expr *Callee = CE->getCallee()->IgnoreParens();
+  const LocationContext *LCtx = C.getLocationContext();
+  SVal L = State->getSVal(Callee, LCtx);
+
+  if (L.isUndef()) {
+if (!BT_call_undef)
+  BT_call_undef.reset(new BuiltinBug(
+  this, "Called function pointer is an uninitialized pointer value"));
+emitBadCall(BT_call_undef.get(), C, Callee);
+return nullptr;
+  }
 
-if (StNull && !StNonNull) {
-  if (!BT_call_null)
-BT_call_null.reset(new BuiltinBug(
-this, "Called function pointer is null (null dereference)"));
-  emitBadCall(BT_call_null.get(), C, Callee);
-  return;
-}
+  ProgramStateRef StNonNull, StNull;
+  std::tie(StNonNull, StNull) = 
State->assume(L.castAs());
 
-State = StNonNull;
+  if (StNull && !StNonNull) {
+if (!BT_call_null)
+  BT_call_null.reset(new BuiltinBug(
+  this, "Called function pointer is null (null dereference)"));
+emitBadCall(BT_call_null.get(), C, Callee);
+return nullptr;
   }
 
-  // If this is a call to a C++ method, check if the callee is null or
-  // undefined.
-  if (const CXXInstanceCall *CC = dyn_cast()) {
-   

[clang] 48a8c7d - [analyzer] Make buildbots happy

2020-05-20 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-21T01:54:50+02:00
New Revision: 48a8c7dcbfb90e917920e90fa2b3ec402e72f4cd

URL: 
https://github.com/llvm/llvm-project/commit/48a8c7dcbfb90e917920e90fa2b3ec402e72f4cd
DIFF: 
https://github.com/llvm/llvm-project/commit/48a8c7dcbfb90e917920e90fa2b3ec402e72f4cd.diff

LOG: [analyzer] Make buildbots happy

Added: 


Modified: 
clang/test/Analysis/malloc.c

Removed: 




diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 2cd9d2845877..a8aabf9f9ace 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1843,9 +1843,10 @@ variable 'buf', which is not memory allocated by 
malloc() [unix.Malloc]}}
   }
 }
 
-(*crash_a)();
+(*crash_a)(); // expected-warning{{type specifier missing}}
 // A CallEvent without a corresponding FunctionDecl.
 crash_b() { crash_a(); } // no-crash
+// expected-warning@-1{{type specifier missing}} 
expected-warning@-1{{non-void}}
 
 // 
 // False negatives.



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


[clang] 1d393ea - [analyzer] Fix a null FunctionDecl dereference bug after D75432

2020-05-20 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-21T01:05:15+02:00
New Revision: 1d393eac8f6907074138612e18d5d1da803b4ad0

URL: 
https://github.com/llvm/llvm-project/commit/1d393eac8f6907074138612e18d5d1da803b4ad0
DIFF: 
https://github.com/llvm/llvm-project/commit/1d393eac8f6907074138612e18d5d1da803b4ad0.diff

LOG: [analyzer] Fix a null FunctionDecl dereference bug after D75432

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/malloc.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index f5f4dd0eaea5..7fae3a62211d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1204,6 +1204,8 @@ void MallocChecker::checkOwnershipAttr(const CallEvent 
,
   if (!CE)
 return;
   const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD)
+return;
   if (ShouldIncludeOwnershipAnnotatedFunctions ||
   ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
 // Check all the attributes, if there are any.

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index b7a29db274b4..2cd9d2845877 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -2,7 +2,7 @@
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=alpha.deadcode.UnreachableCode \
 // RUN:   -analyzer-checker=alpha.core.CastSize \
-// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=unix \
 // RUN:   -analyzer-checker=debug.ExprInspection
 
 #include "Inputs/system-header-simulator.h"
@@ -1843,6 +1843,10 @@ variable 'buf', which is not memory allocated by 
malloc() [unix.Malloc]}}
   }
 }
 
+(*crash_a)();
+// A CallEvent without a corresponding FunctionDecl.
+crash_b() { crash_a(); } // no-crash
+
 // 
 // False negatives.
 



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


[clang] 3a6ee4f - [analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers rather then core.StackAddrEscapeBase

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T02:26:40+02:00
New Revision: 3a6ee4fefec0bc97a0340ddc33e7a8ffd4590ad5

URL: 
https://github.com/llvm/llvm-project/commit/3a6ee4fefec0bc97a0340ddc33e7a8ffd4590ad5
DIFF: 
https://github.com/llvm/llvm-project/commit/3a6ee4fefec0bc97a0340ddc33e7a8ffd4590ad5.diff

LOG: [analyzer][StackAddressEscape] Tie warnings to the diagnostic checkers 
rather then core.StackAddrEscapeBase

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
clang/test/Analysis/incorrect-checker-names.cpp
clang/test/Analysis/incorrect-checker-names.mm

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
index 237053df7e44..49ab25eca2dd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugType.h
@@ -78,13 +78,16 @@ class BuiltinBug : public BugType {
  const char *description)
   : BugType(checker, name, categories::LogicError), desc(description) {}
 
+  BuiltinBug(class CheckerNameRef checker, const char *name)
+  : BugType(checker, name, categories::LogicError), desc(name) {}
+
   BuiltinBug(const CheckerBase *checker, const char *name)
   : BugType(checker, name, categories::LogicError), desc(name) {}
 
   StringRef getDescription() const { return desc; }
 };
 
-} // end ento namespace
+} // namespace ento
 
 } // end clang namespace
 #endif

diff  --git a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
index 21fc65a8e4f1..b5c9356322fc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
@@ -43,6 +43,7 @@ class StackAddrEscapeChecker
   };
 
   DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  CheckerNameRef CheckNames[CK_NumCheckKinds];
 
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPreStmt(const ReturnStmt *RS, CheckerContext ) const;
@@ -156,7 +157,8 @@ void StackAddrEscapeChecker::EmitStackError(CheckerContext 
,
 return;
   if (!BT_returnstack)
 BT_returnstack = std::make_unique(
-this, "Return of address to stack-allocated memory");
+CheckNames[CK_StackAddrEscapeChecker],
+"Return of address to stack-allocated memory");
   // Generate a report for this bug.
   SmallString<128> buf;
   llvm::raw_svector_ostream os(buf);
@@ -195,7 +197,8 @@ void 
StackAddrEscapeChecker::checkAsyncExecutedBlockCaptures(
   continue;
 if (!BT_capturedstackasync)
   BT_capturedstackasync = std::make_unique(
-  this, "Address of stack-allocated memory is captured");
+  CheckNames[CK_StackAddrAsyncEscapeChecker],
+  "Address of stack-allocated memory is captured");
 SmallString<128> Buf;
 llvm::raw_svector_ostream Out(Buf);
 SourceRange Range = genName(Out, Region, C.getASTContext());
@@ -218,7 +221,8 @@ void StackAddrEscapeChecker::checkReturnedBlockCaptures(
   continue;
 if (!BT_capturedstackret)
   BT_capturedstackret = std::make_unique(
-  this, "Address of stack-allocated memory is captured");
+  CheckNames[CK_StackAddrEscapeChecker],
+  "Address of stack-allocated memory is captured");
 SmallString<128> Buf;
 llvm::raw_svector_ostream Out(Buf);
 SourceRange Range = genName(Out, Region, C.getASTContext());
@@ -277,7 +281,7 @@ void StackAddrEscapeChecker::checkPreStmt(const ReturnStmt 
*RS,
 
   // The CK_CopyAndAutoreleaseBlockObject cast causes the block to be copied
   // so the stack address is not escaping here.
-  if (auto *ICE = dyn_cast(RetE)) {
+  if (const auto *ICE = dyn_cast(RetE)) {
 if (isa(R) &&
 ICE->getCastKind() == CK_CopyAndAutoreleaseBlockObject) {
   return;
@@ -333,7 +337,8 @@ void StackAddrEscapeChecker::checkEndFunction(const 
ReturnStmt *RS,
 
   if (!BT_stackleak)
 BT_stackleak = std::make_unique(
-this, "Stack address stored into global variable",
+CheckNames[CK_StackAddrEscapeChecker],
+"Stack address stored into global variable",
 "Stack address was saved into a global variable. "
 "This is dangerous because the address will become "
 "invalid after returning from the function");
@@ -371,14 +376,13 @@ bool ento::shouldRegisterStackAddrEscapeBase(const 
CheckerManager ) {
 
 #define REGISTER_CHECKER(name) 
\
   void ento::register##name(CheckerManager ) { 
\
-StackAddrEscapeChecker *Chk =  
\
-Mgr.getChecker();  

[clang] 392222d - [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2020-05-20T02:03:31+02:00
New Revision: 39dd72657244f13d7f99cc6a497cc78eba2e

URL: 
https://github.com/llvm/llvm-project/commit/39dd72657244f13d7f99cc6a497cc78eba2e
DIFF: 
https://github.com/llvm/llvm-project/commit/39dd72657244f13d7f99cc6a497cc78eba2e.diff

LOG: [analyzer][NFC][MallocChecker] Convert many parameters into CallEvent

Exactly what it says on the tin! This is clearly not the end of the road in this
direction, the parameters could be merged far more with the use of CallEvent or
a better value type in the CallDescriptionMap, but this was shockingly difficult
enough on its own. I expect that simplifying the file further will be far easier
moving forward.

The end goal is to research how we could create a more mature checker
interaction infrastructure for more complicated C++ modeling, and I'm pretty
sure that being able successfully split up our giants is the first step in this
direction.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index edcee6e2d052..f5f4dd0eaea5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -47,7 +47,10 @@
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
@@ -63,10 +66,12 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
@@ -268,7 +273,7 @@ REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, 
ReallocPair)
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
 static bool isStandardNewDelete(const CallEvent ) {
-  if (!Call.getDecl())
+  if (!Call.getDecl() || !isa(Call.getDecl()))
 return false;
   return isStandardNewDelete(cast(Call.getDecl()));
 }
@@ -283,9 +288,8 @@ class MallocChecker
 : public Checker,
  check::EndFunction, check::PreCall, check::PostCall,
- check::PostStmt, check::NewAllocator,
- check::PostStmt, check::PostObjCMessage,
- check::Location, eval::Assume> {
+ check::NewAllocator, check::PostStmt,
+ check::PostObjCMessage, check::Location, eval::Assume> {
 public:
   /// In pessimistic mode, the checker assumes that it does not know which
   /// functions might free the memory.
@@ -314,8 +318,6 @@ class MallocChecker
 
   void checkPreCall(const CallEvent , CheckerContext ) const;
   void checkPostCall(const CallEvent , CheckerContext ) const;
-  void checkPostStmt(const CXXNewExpr *NE, CheckerContext ) const;
-  void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext ) const;
   void checkNewAllocator(const CXXAllocatorCall , CheckerContext ) 
const;
   void checkPostObjCMessage(const ObjCMethodCall , CheckerContext ) 
const;
   void checkPostStmt(const BlockExpr *BE, CheckerContext ) const;
@@ -351,7 +353,7 @@ class MallocChecker
   mutable std::unique_ptr BT_UseZerroAllocated[CK_NumCheckKinds];
 
 #define CHECK_FN(NAME) 
\
-  void NAME(CheckerContext , const CallExpr *CE, ProgramStateRef State) 
const;
+  void NAME(const CallEvent , CheckerContext ) const;
 
   CHECK_FN(checkFree)
   CHECK_FN(checkIfNameIndex)
@@ -369,12 +371,11 @@ class MallocChecker
   CHECK_FN(checkReallocN)
   CHECK_FN(checkOwnershipAttr)
 
-  void checkRealloc(CheckerContext , const CallExpr *CE,
-ProgramStateRef State, bool ShouldFreeOnFail) const;
+  void checkRealloc(const CallEvent , CheckerContext ,
+bool ShouldFreeOnFail) const;
 
-  using CheckFn =
-  std::function;
+  using CheckFn = std::function;
 
   const CallDescriptionMap FreeingMemFnMap{
   {{"free", 1}, ::checkFree},
@@ -412,13 +413,13 @@ class MallocChecker
 
   CallDescriptionMap ReallocatingMemFnMap{
   {{"realloc", 2},
-   std::bind(::checkRealloc, _1, _2, _3, _4, false)},
+   std::bind(::checkRealloc, _1, _2, _3, false)},
   

[clang] fe1a3a7 - [analyzer] Change the default output type to PD_TEXT_MINIMAL in the frontend, error if an output loc is missing for PathDiagConsumers that need it

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T01:36:06+02:00
New Revision: fe1a3a7e8c8be33968b9a768666489823dabab10

URL: 
https://github.com/llvm/llvm-project/commit/fe1a3a7e8c8be33968b9a768666489823dabab10
DIFF: 
https://github.com/llvm/llvm-project/commit/fe1a3a7e8c8be33968b9a768666489823dabab10.diff

LOG: [analyzer] Change the default output type to PD_TEXT_MINIMAL in the 
frontend, error if an output loc is missing for PathDiagConsumers that need it

The title and the included test file sums everything up -- the only thing I'm
mildly afraid of is whether anyone actually depends on the weird behavior of
HTMLDiagnostics pretending to be TextDiagnostics if an output directory is not
supplied. If it is, I guess we would need to resort to tiptoeing around the
compatibility flag.

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

Added: 
clang/test/Analysis/output_types.cpp

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index d010a7dfb2de..fdca8532ab53 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -358,6 +358,9 @@ def err_analyzer_checker_option_invalid_input : Error<
   "invalid input for checker option '%0', that expects %1">;
 def err_analyzer_checker_incompatible_analyzer_option : Error<
   "checker cannot be enabled with analyzer option '%0' == %1">;
+def err_analyzer_missing_output_loc : Error<
+  "analyzer output type '%0' requires an output %1 to be specified with "
+  "-o ">;
 
 def err_drv_invalid_hvx_length : Error<
   "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index d2df24a6e21b..373caa30bbc9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -206,7 +206,7 @@ class AnalyzerOptions : public 
RefCountedBase {
   ConfigTable Config;
   AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
-  AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
+  AnalysisDiagClients AnalysisDiagOpt = PD_TEXT_MINIMAL;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
 
   std::string AnalyzeSpecificFunction;

diff  --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 184fdcfb3d4b..8ce3aa2e081e 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -21,6 +21,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -137,18 +138,14 @@ void ento::createHTMLDiagnosticConsumer(
 const std::string , const Preprocessor ,
 const cross_tu::CrossTranslationUnitContext ) {
 
-  // FIXME: HTML is currently our default output type, but if the output
-  // directory isn't specified, it acts like if it was in the minimal text
-  // output mode. This doesn't make much sense, we should have the minimal text
-  // as our default. In the case of backward compatibility concerns, this could
-  // be preserved with -analyzer-config-compatibility-mode=true.
-  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
-
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
+  if (OutputDir.empty()) {
+PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+  << "html" << "directory";
 return;
+  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
@@ -156,9 +153,11 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
 const std::string , const Preprocessor ,
 const cross_tu::CrossTranslationUnitContext ) {
 
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
+  if (OutputDir.empty()) {
+PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+  << "html-single-file" << "directory";
 return;
+  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, false));
   createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, 

[clang] f2be30d - [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T00:56:10+02:00
New Revision: f2be30def37d248a7666cfac3c922ca9db308c2a

URL: 
https://github.com/llvm/llvm-project/commit/f2be30def37d248a7666cfac3c922ca9db308c2a
DIFF: 
https://github.com/llvm/llvm-project/commit/f2be30def37d248a7666cfac3c922ca9db308c2a.diff

LOG: [analyzer][NFC] Merge checkNewAllocator's paramaters into CXXAllocatorCall

Party based on this thread:
http://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html.

This patch merges two of CXXAllocatorCall's parameters, so that we are able to
supply a CallEvent object to check::NewAllocatorCall (see the description of
D75430 to see why this would be great).

One of the things mentioned by @NoQ was the following:

  I think at this point we might actually do a good job sorting out this
  check::NewAllocator issue because we have a "separate" "Environment" to hold
  the other SVal, which is "objects under construction"! - so we should probably
  simply teach CXXAllocatorCall to extract the value from the
  objects-under-construction trait of the program state and we're good.

I had MallocChecker in my crosshair for now, so I admittedly threw together
something as a proof of concept. Now that I know that this effort is worth
pursuing though, I'll happily look for a solution better then demonstrated in
this patch.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/Checker.h
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h 
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index 0c7acdbc3a97..fdba49664615 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -285,9 +285,9 @@ class BranchCondition {
 
 class NewAllocator {
   template 
-  static void _checkNewAllocator(void *checker, const CXXNewExpr *NE,
- SVal Target, CheckerContext ) {
-((const CHECKER *)checker)->checkNewAllocator(NE, Target, C);
+  static void _checkNewAllocator(void *checker, const CXXAllocatorCall ,
+ CheckerContext ) {
+((const CHECKER *)checker)->checkNewAllocator(Call, C);
   }
 
 public:

diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index f34f5c239290..820ccfceecf6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -37,6 +37,7 @@ class TranslationUnitDecl;
 namespace ento {
 
 class AnalysisManager;
+class CXXAllocatorCall;
 class BugReporter;
 class CallEvent;
 class CheckerBase;
@@ -361,11 +362,9 @@ class CheckerManager {
  ExprEngine );
 
   /// Run checkers between C++ operator new and constructor calls.
-  void runCheckersForNewAllocator(const CXXNewExpr *NE, SVal Target,
-  ExplodedNodeSet ,
-  ExplodedNode *Pred,
-  ExprEngine ,
-  bool wasInlined = false);
+  void runCheckersForNewAllocator(const CXXAllocatorCall ,
+  ExplodedNodeSet , ExplodedNode *Pred,
+  ExprEngine , bool wasInlined = false);
 
   /// Run checkers for live symbols.
   ///
@@ -506,7 +505,7 @@ class CheckerManager {
   CheckerFn;
 
   using CheckNewAllocatorFunc =
-  CheckerFn;
+  CheckerFn;
 
   using CheckDeadSymbolsFunc =
   CheckerFn;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index 871875b45b1b..8b84b79651bd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -39,6 +39,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -1010,6 +1011,12 @@ class CXXAllocatorCall : public AnyFunctionCall {
 return getOriginExpr()->getOperatorNew();
   }
 
+  SVal getObjectUnderConstruction() const {
+return ExprEngine::getObjectUnderConstruction(getState(), getOriginExpr(),
+

[clang] 3d0d2fe - analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T00:37:59+02:00
New Revision: 3d0d2fefc0a18f13e45a17be04a3da8d2b1299f8

URL: 
https://github.com/llvm/llvm-project/commit/3d0d2fefc0a18f13e45a17be04a3da8d2b1299f8
DIFF: 
https://github.com/llvm/llvm-project/commit/3d0d2fefc0a18f13e45a17be04a3da8d2b1299f8.diff

LOG: analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

The following series of patches has something similar in mind with D77474, with
the same goal to finally end incorrect checker names for good. Despite
CallAndMessage not suffering from this particular issue, it is a dependency for
many other checkers, which is problematic, because we don't really want
dependencies to also emit diagnostics (reasoning for this is also more detailed
in D77474).

CallAndMessage also has another problem, namely that it is responsible for a lot
of reports. You'll soon learn that this isn't really easy to solve for
compatibility reasons, but that is the topic of followup patches.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 82bc200ba4ce..00fbe2c8dcb7 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -11,9 +11,10 @@
 //
 
//===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -21,6 +22,7 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace clang;
@@ -29,11 +31,8 @@ using namespace ento;
 namespace {
 
 class CallAndMessageChecker
-  : public Checker< check::PreStmt,
-check::PreStmt,
-check::PreObjCMessage,
-check::ObjCMessageNil,
-check::PreCall > {
+: public Checker {
   mutable std::unique_ptr BT_call_null;
   mutable std::unique_ptr BT_call_undef;
   mutable std::unique_ptr BT_cxx_call_null;
@@ -48,11 +47,10 @@ class CallAndMessageChecker
   mutable std::unique_ptr BT_call_few_args;
 
 public:
-  DefaultBool Check_CallAndMessageUnInitRefArg;
-  CheckerNameRef CheckName_CallAndMessageUnInitRefArg;
+  enum CheckKind { CK_CallAndMessageUnInitRefArg, CK_NumCheckKinds };
+
+  DefaultBool ChecksEnabled[CK_NumCheckKinds];
 
-  void checkPreStmt(const CallExpr *CE, CheckerContext ) const;
-  void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext ) const;
   void checkPreObjCMessage(const ObjCMethodCall , CheckerContext ) const;
 
   /// Fill in the return value that results from messaging nil based on the
@@ -144,7 +142,7 @@ bool CallAndMessageChecker::uninitRefOrPointer(
 CheckerContext , const SVal , SourceRange ArgRange, const Expr *ArgEx,
 std::unique_ptr , const ParmVarDecl *ParamDecl, const char *BD,
 int ArgumentNumber) const {
-  if (!Check_CallAndMessageUnInitRefArg)
+  if (!ChecksEnabled[CK_CallAndMessageUnInitRefArg])
 return false;
 
   // No parameter declaration available, i.e. variadic function argument.
@@ -311,63 +309,36 @@ bool 
CallAndMessageChecker::PreVisitProcessArg(CheckerContext ,
   return false;
 }
 
-void CallAndMessageChecker::checkPreStmt(const CallExpr *CE,
- CheckerContext ) const{
-
-  const Expr *Callee = CE->getCallee()->IgnoreParens();
+void CallAndMessageChecker::checkPreCall(const CallEvent ,
+ CheckerContext ) const {
   ProgramStateRef State = C.getState();
-  const LocationContext *LCtx = C.getLocationContext();
-  SVal L = State->getSVal(Callee, LCtx);
-
-  if (L.isUndef()) {
-if (!BT_call_undef)
-  BT_call_undef.reset(new BuiltinBug(
-  this, "Called function pointer is an uninitialized pointer value"));
-emitBadCall(BT_call_undef.get(), C, Callee);
-return;
-  }
-
-  ProgramStateRef StNonNull, StNull;
-  std::tie(StNonNull, StNull) = 
State->assume(L.castAs());
-
-  if (StNull && !StNonNull) {
-if (!BT_call_null)
-  BT_call_null.reset(new BuiltinBug(
-  this, "Called function pointer is null (null dereference)"));
-emitBadCall(BT_call_null.get(), C, Callee);
-return;
-  }
-
-  C.addTransition(StNonNull);
-}

[clang] 66224d3 - [analyzer][ObjCGenerics] Don't emit diagnostics under the name core.DynamicTypePropagation

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T00:19:20+02:00
New Revision: 66224d309d08c048b3bea257ceafe4873ce166c6

URL: 
https://github.com/llvm/llvm-project/commit/66224d309d08c048b3bea257ceafe4873ce166c6
DIFF: 
https://github.com/llvm/llvm-project/commit/66224d309d08c048b3bea257ceafe4873ce166c6.diff

LOG: [analyzer][ObjCGenerics] Don't emit diagnostics under the name 
core.DynamicTypePropagation

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
clang/test/Analysis/Inputs/expected-plists/generics.m.plist

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
index 1502c0f9d656..14ba5d769969 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
@@ -55,6 +55,7 @@ class DynamicTypePropagation:
 check::PostStmt,
 check::PreObjCMessage,
 check::PostObjCMessage > {
+
   const ObjCObjectType *getObjectTypeForAllocAndNew(const ObjCMessageExpr 
*MsgE,
 CheckerContext ) const;
 
@@ -69,8 +70,8 @@ class DynamicTypePropagation:
   mutable std::unique_ptr ObjCGenericsBugType;
   void initBugType() const {
 if (!ObjCGenericsBugType)
-  ObjCGenericsBugType.reset(
-  new BugType(this, "Generics", categories::CoreFoundationObjectiveC));
+  ObjCGenericsBugType.reset(new BugType(
+  GenericCheckName, "Generics", categories::CoreFoundationObjectiveC));
   }
 
   class GenericsBugVisitor : public BugReporterVisitor {
@@ -108,6 +109,7 @@ class DynamicTypePropagation:
 
   /// This value is set to true, when the Generics checker is turned on.
   DefaultBool CheckGenerics;
+  CheckerNameRef GenericCheckName;
 };
 
 bool isObjCClassType(QualType Type) {
@@ -1101,6 +1103,7 @@ PathDiagnosticPieceRef 
DynamicTypePropagation::GenericsBugVisitor::VisitNode(
 void ento::registerObjCGenericsChecker(CheckerManager ) {
   DynamicTypePropagation *checker = mgr.getChecker();
   checker->CheckGenerics = true;
+  checker->GenericCheckName = mgr.getCurrentCheckerName();
 }
 
 bool ento::shouldRegisterObjCGenericsChecker(const CheckerManager ) {

diff  --git a/clang/test/Analysis/Inputs/expected-plists/generics.m.plist 
b/clang/test/Analysis/Inputs/expected-plists/generics.m.plist
index ad01828a350e..bbe77595818d 100644
--- a/clang/test/Analysis/Inputs/expected-plists/generics.m.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/generics.m.plist
@@ -138,9 +138,9 @@
descriptionConversion from value of type 
NSMutableArrayNSString * * to incompatible type 
NSArrayNSNumber * *
categoryCore Foundation/Objective-C
typeGenerics
-   check_namecore.DynamicTypePropagation
+   check_nameosx.cocoa.ObjCGenerics

-   
issue_hash_content_of_line_in_context33d4584e2bf66b029ab9d152cc9cd8f7
+   
issue_hash_content_of_line_in_context9be4ccbeebeb5f814eb9ff5cef4907d3
   issue_context_kindfunction
   issue_contextincompatibleTypesErased
   issue_hash_function_offset2
@@ -295,9 +295,9 @@
descriptionConversion from value of type 
NSMutableArrayNSString * * to incompatible type 
NSArrayNSNumber * *
categoryCore Foundation/Objective-C
typeGenerics
-   check_namecore.DynamicTypePropagation
+   check_nameosx.cocoa.ObjCGenerics

-   
issue_hash_content_of_line_in_context6edc910aaa9dc1f2d823abc8cb75360f
+   
issue_hash_content_of_line_in_context567cd90c936f23ea70aca98b9d3af2b7
   issue_context_kindfunction
   issue_contextincompatibleTypesErased
   issue_hash_function_offset5
@@ -421,9 +421,9 @@
descriptionConversion from value of type NSNumber 
* to incompatible type NSString *
categoryCore Foundation/Objective-C
typeGenerics
-   check_namecore.DynamicTypePropagation
+   check_nameosx.cocoa.ObjCGenerics

-   
issue_hash_content_of_line_in_context73c71c858082f5d7a2258f707927da3c
+   
issue_hash_content_of_line_in_contexta6e6f9c2db7532f45c07d2c13bcf496b
   issue_context_kindfunction
   issue_contextincompatibleTypesErased
   issue_hash_function_offset8
@@ -689,9 +689,9 @@
descriptionConversion from value of type 
NSArrayNSNumber * * to incompatible type 
NSArrayNSString * *
categoryCore Foundation/Objective-C
typeGenerics
-   check_namecore.DynamicTypePropagation
+   check_nameosx.cocoa.ObjCGenerics

-   
issue_hash_content_of_line_in_context82c378fdcfcc5c0318d2f3ca46420ec1
+   
issue_hash_content_of_line_in_context73523166e7c9e436da86a96fbd7b3d90
   issue_context_kindfunction
   issue_contextcrossProceduralErasedTypes
   issue_hash_function_offset1
@@ -846,9 +846,9 @@
descriptionConversion from value of type 
NSMutableArrayNSString * * to incompatible type 
NSArrayNSNumber * *
categoryCore 

[clang] b47d1ba - [analyzer][NSOrCFError] Don't emit diagnostics under the name osx.NSOrCFErrorDerefChecker

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-20T00:05:49+02:00
New Revision: b47d1baa535abe061e6a89341e91c8b885b5b80e

URL: 
https://github.com/llvm/llvm-project/commit/b47d1baa535abe061e6a89341e91c8b885b5b80e
DIFF: 
https://github.com/llvm/llvm-project/commit/b47d1baa535abe061e6a89341e91c8b885b5b80e.diff

LOG: [analyzer][NSOrCFError] Don't emit diagnostics under the name 
osx.NSOrCFErrorDerefChecker

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
clang/test/Analysis/incorrect-checker-names.mm

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 5f68788fafcd..90c5583d8969 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -144,14 +144,14 @@ namespace {
 
 class NSErrorDerefBug : public BugType {
 public:
-  NSErrorDerefBug(const CheckerBase *Checker)
+  NSErrorDerefBug(const CheckerNameRef Checker)
   : BugType(Checker, "NSError** null dereference",
 "Coding conventions (Apple)") {}
 };
 
 class CFErrorDerefBug : public BugType {
 public:
-  CFErrorDerefBug(const CheckerBase *Checker)
+  CFErrorDerefBug(const CheckerNameRef Checker)
   : BugType(Checker, "CFErrorRef* null dereference",
 "Coding conventions (Apple)") {}
 };
@@ -166,9 +166,9 @@ class NSOrCFErrorDerefChecker
   mutable std::unique_ptr NSBT;
   mutable std::unique_ptr CFBT;
 public:
-  bool ShouldCheckNSError, ShouldCheckCFError;
-  NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr),
-  ShouldCheckNSError(0), ShouldCheckCFError(0) { }
+  DefaultBool ShouldCheckNSError, ShouldCheckCFError;
+  CheckerNameRef NSErrorName, CFErrorName;
+  NSOrCFErrorDerefChecker() : NSErrorII(nullptr), CFErrorII(nullptr) {}
 
   void checkLocation(SVal loc, bool isLoad, const Stmt *S,
  CheckerContext ) const;
@@ -276,12 +276,12 @@ void 
NSOrCFErrorDerefChecker::checkEvent(ImplicitNullDerefEvent event) const {
   BugType *bug = nullptr;
   if (isNSError) {
 if (!NSBT)
-  NSBT.reset(new NSErrorDerefBug(this));
+  NSBT.reset(new NSErrorDerefBug(NSErrorName));
 bug = NSBT.get();
   }
   else {
 if (!CFBT)
-  CFBT.reset(new CFErrorDerefBug(this));
+  CFBT.reset(new CFErrorDerefBug(CFErrorName));
 bug = CFBT.get();
   }
   BR.emitReport(
@@ -331,6 +331,7 @@ void ento::registerNSErrorChecker(CheckerManager ) {
   mgr.registerChecker();
   NSOrCFErrorDerefChecker *checker = mgr.getChecker();
   checker->ShouldCheckNSError = true;
+  checker->NSErrorName = mgr.getCurrentCheckerName();
 }
 
 bool ento::shouldRegisterNSErrorChecker(const CheckerManager ) {
@@ -341,6 +342,7 @@ void ento::registerCFErrorChecker(CheckerManager ) {
   mgr.registerChecker();
   NSOrCFErrorDerefChecker *checker = mgr.getChecker();
   checker->ShouldCheckCFError = true;
+  checker->CFErrorName = mgr.getCurrentCheckerName();
 }
 
 bool ento::shouldRegisterCFErrorChecker(const CheckerManager ) {

diff  --git a/clang/test/Analysis/incorrect-checker-names.mm 
b/clang/test/Analysis/incorrect-checker-names.mm
index f14eea9d9c63..11b6a21a14a9 100644
--- a/clang/test/Analysis/incorrect-checker-names.mm
+++ b/clang/test/Analysis/incorrect-checker-names.mm
@@ -106,9 +106,19 @@ + (id)errorWithDomain:(NSString *)domain 
code:(NSInteger)code userInfo:(NSDictio
 
 void foo(CFErrorRef* error) { // expected-warning{{Function accepting 
CFErrorRef* should have a non-void return value to indicate whether or not an 
error occurred [osx.coreFoundation.CFError]}}
   // FIXME: This shouldn't be tied to a modeling checker.
-  *error = 0;  // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.NSOrCFErrorDerefChecker]}}
+  *error = 0; // expected-warning {{Potential null dereference.  According to 
coding standards documented in CoreFoundation/CFError.h the parameter may be 
null [osx.coreFoundation.CFError]}}
 }
 
+@interface A
+- (void)myMethodWhichMayFail:(NSError **)error;
+@end
+
+@implementation A
+- (void)myMethodWhichMayFail:(NSError **)error {  // 
expected-warning {{Method accepting NSError** should have a non-void return 
value to indicate whether or not an error occurred [osx.cocoa.NSError]}}
+  *error = [NSError errorWithDomain:@"domain" code:1 userInfo:0]; // 
expected-warning {{Potential null dereference.  According to coding standards 
in 'Creating and Returning NSError Objects' the parameter may be null 
[osx.cocoa.NSError]}}
+}
+@end
+
 bool write_into_out_param_on_success(OS_RETURNS_RETAINED OSObject **obj);
 
 void use_out_param_leak() {



___
cfe-commits mailing list

[clang] e4e1080 - [analyzer][Nullability] Don't emit under the checker name NullabilityBase

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-19T17:04:06+02:00
New Revision: e4e1080a5837e4a4f38c46b9974f2b2402bb021a

URL: 
https://github.com/llvm/llvm-project/commit/e4e1080a5837e4a4f38c46b9974f2b2402bb021a
DIFF: 
https://github.com/llvm/llvm-project/commit/e4e1080a5837e4a4f38c46b9974f2b2402bb021a.diff

LOG: [analyzer][Nullability] Don't emit under the checker name NullabilityBase

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
clang/test/Analysis/incorrect-checker-names.mm

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 565d0eeef704..bc7a8a3b12a1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -82,7 +82,6 @@ class NullabilityChecker
  check::PostCall, check::PostStmt,
  check::PostObjCMessage, check::DeadSymbols,
  check::Location, check::Event> {
-  mutable std::unique_ptr BT;
 
 public:
   // If true, the checker will not diagnose nullabilility issues for calls
@@ -107,21 +106,26 @@ class NullabilityChecker
   void printState(raw_ostream , ProgramStateRef State, const char *NL,
   const char *Sep) const override;
 
-  struct NullabilityChecksFilter {
-DefaultBool CheckNullPassedToNonnull;
-DefaultBool CheckNullReturnedFromNonnull;
-DefaultBool CheckNullableDereferenced;
-DefaultBool CheckNullablePassedToNonnull;
-DefaultBool CheckNullableReturnedFromNonnull;
-
-CheckerNameRef CheckNameNullPassedToNonnull;
-CheckerNameRef CheckNameNullReturnedFromNonnull;
-CheckerNameRef CheckNameNullableDereferenced;
-CheckerNameRef CheckNameNullablePassedToNonnull;
-CheckerNameRef CheckNameNullableReturnedFromNonnull;
+  enum CheckKind {
+CK_NullPassedToNonnull,
+CK_NullReturnedFromNonnull,
+CK_NullableDereferenced,
+CK_NullablePassedToNonnull,
+CK_NullableReturnedFromNonnull,
+CK_NumCheckKinds
   };
 
-  NullabilityChecksFilter Filter;
+  DefaultBool ChecksEnabled[CK_NumCheckKinds];
+  CheckerNameRef CheckNames[CK_NumCheckKinds];
+  mutable std::unique_ptr BTs[CK_NumCheckKinds];
+
+  const std::unique_ptr (CheckKind Kind) const {
+if (!BTs[Kind])
+  BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability",
+  categories::MemoryError));
+return BTs[Kind];
+  }
+
   // When set to false no nullability information will be tracked in
   // NullabilityMap. It is possible to catch errors like passing a null pointer
   // to a callee that expects nonnull argument without the information that is
@@ -153,18 +157,16 @@ class NullabilityChecker
   ///
   /// When \p SuppressPath is set to true, no more bugs will be reported on 
this
   /// path by this checker.
-  void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error,
+  void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, CheckKind CK,
  ExplodedNode *N, const MemRegion *Region,
  CheckerContext ,
  const Stmt *ValueExpr = nullptr,
-  bool SuppressPath = false) const;
+ bool SuppressPath = false) const;
 
-  void reportBug(StringRef Msg, ErrorKind Error, ExplodedNode *N,
+  void reportBug(StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
  const MemRegion *Region, BugReporter ,
  const Stmt *ValueExpr = nullptr) const {
-if (!BT)
-  BT.reset(new BugType(this, "Nullability", categories::MemoryError));
-
+const std::unique_ptr  = getBugType(CK);
 auto R = std::make_unique(*BT, Msg, N);
 if (Region) {
   R->markInteresting(Region);
@@ -432,9 +434,10 @@ static bool checkInvariantViolation(ProgramStateRef State, 
ExplodedNode *N,
   return false;
 }
 
-void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg,
-ErrorKind Error, ExplodedNode *N, const MemRegion *Region,
-CheckerContext , const Stmt *ValueExpr, bool SuppressPath) const {
+void NullabilityChecker::reportBugIfInvariantHolds(
+StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
+const MemRegion *Region, CheckerContext , const Stmt *ValueExpr,
+bool SuppressPath) const {
   ProgramStateRef OriginalState = N->getState();
 
   if (checkInvariantViolation(OriginalState, N, C))
@@ -444,7 +447,7 @@ void 
NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg,
 N = C.addTransition(OriginalState, N);
   }
 
-  reportBug(Msg, Error, N, Region, C.getBugReporter(), ValueExpr);
+  reportBug(Msg, Error, CK, N, Region, 

[clang] 268fa40 - [analyzer] Don't print the config count in debug.ConfigDumper

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-19T16:51:14+02:00
New Revision: 268fa40daa151d3b4bff1df12b62e5dae94685d7

URL: 
https://github.com/llvm/llvm-project/commit/268fa40daa151d3b4bff1df12b62e5dae94685d7
DIFF: 
https://github.com/llvm/llvm-project/commit/268fa40daa151d3b4bff1df12b62e5dae94685d7.diff

LOG: [analyzer] Don't print the config count in debug.ConfigDumper

I think anyone who added a checker config wondered why is there a need
to test this. Its just a chore when adding a new config, so I removed
it.

To give some historic insight though, we used to not list **all**
options, but only those explicitly added to AnalyzerOptions, such as the
ones specified on the command line. However, past this change (and
arguably even before that) this line makes little sense.

There is an argument to be made against the entirety of
analyzer-config.c test file, but since this commit fixes some builtbots
and is landing without review, I wouldn't like to be too invasive.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
clang/test/Analysis/analyzer-config.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
index e93c1e9b0a6a..03b7cbd1c833 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
@@ -281,8 +281,6 @@ class ConfigDumper : public Checker< 
check::EndOfTranslationUnit > {
   llvm::errs() << Keys[I]->getKey() << " = "
<< (Keys[I]->second.empty() ? "\"\"" : Keys[I]->second)
<< '\n';
-
-llvm::errs() << "[stats]\n" << "num-entries = " << Keys.size() << '\n';
   }
 };
 }

diff  --git a/clang/test/Analysis/analyzer-config.c 
b/clang/test/Analysis/analyzer-config.c
index 3a6b4d20b1b1..85cd6738b6dc 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -107,5 +107,3 @@
 // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
-// CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 104



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


[clang] 500479d - [analyzer][DirectIvarAssignment] Turn DirectIvarAssignmentForAnnotatedFunctions into a checker option

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-19T15:41:43+02:00
New Revision: 500479dba33ae03644313e34d10c013371b5215b

URL: 
https://github.com/llvm/llvm-project/commit/500479dba33ae03644313e34d10c013371b5215b
DIFF: 
https://github.com/llvm/llvm-project/commit/500479dba33ae03644313e34d10c013371b5215b.diff

LOG: [analyzer][DirectIvarAssignment] Turn 
DirectIvarAssignmentForAnnotatedFunctions into a checker option

Since this is an alpha checker, I don't worry about backward compatibility :)

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 5cf32ac03436..cc6952b55810 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1118,13 +1118,15 @@ def MissingInvalidationMethod : 
Checker<"MissingInvalidationMethod">,
 
 def DirectIvarAssignment : Checker<"DirectIvarAssignment">,
   HelpText<"Check for direct assignments to instance variables">,
-  Documentation;
-
-def DirectIvarAssignmentForAnnotatedFunctions :
-  Checker<"DirectIvarAssignmentForAnnotatedFunctions">,
-  HelpText<"Check for direct assignments to instance variables in the methods "
-   "annotated with objc_no_direct_instance_variable_assignment">,
-  Dependencies<[DirectIvarAssignment]>,
+  CheckerOptions<[
+CmdLineOption
+  ]>,
   Documentation;
 
 } // end "alpha.osx.cocoa"

diff  --git a/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
index d09f0da3faac..df88b71ff063 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
@@ -219,19 +219,12 @@ static bool AttrFilter(const ObjCMethodDecl *M) {
 // Register the checker that checks for direct accesses in all functions,
 // except for the initialization and copy routines.
 void ento::registerDirectIvarAssignment(CheckerManager ) {
-  mgr.registerChecker();
+  auto Chk = mgr.registerChecker();
+  if (mgr.getAnalyzerOptions().getCheckerBooleanOption(Chk,
+   "AnnotatedFunctions"))
+Chk->ShouldSkipMethod = 
 }
 
 bool ento::shouldRegisterDirectIvarAssignment(const CheckerManager ) {
   return true;
 }
-
-void ento::registerDirectIvarAssignmentForAnnotatedFunctions(
-CheckerManager ) {
-  mgr.getChecker()->ShouldSkipMethod = 
-}
-
-bool ento::shouldRegisterDirectIvarAssignmentForAnnotatedFunctions(
-const CheckerManager ) 
{
-  return true;
-}

diff  --git a/clang/test/Analysis/analyzer-config.c 
b/clang/test/Analysis/analyzer-config.c
index b94f618d7492..3a6b4d20b1b1 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -8,6 +8,7 @@
 // CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
 // CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
 // CHECK-NEXT: alpha.cplusplus.STLAlgorithmModeling:AggressiveStdFindModeling 
= false
+// CHECK-NEXT: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions = false
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = ""

diff  --git 
a/clang/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m 
b/clang/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m
index 782fcecd43f9..4d3ee072ca29 100644
--- a/clang/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m
+++ b/clang/test/Analysis/objc/direct-ivar-assignment-in-annotated-functions.m
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 
-analyzer-checker=alpha.osx.cocoa.DirectIvarAssignmentForAnnotatedFunctions 
-verify -fblocks %s
+// RUN: %clang_analyze_cc1 -verify -fblocks %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=alpha.osx.cocoa.DirectIvarAssignment \
+// RUN:   -analyzer-config \
+// RUN: alpha.osx.cocoa.DirectIvarAssignment:AnnotatedFunctions=true
 
 typedef signed char BOOL;
 @protocol NSObject  - (BOOL)isEqual:(id)object; @end
@@ -60,4 +64,4 @@ - (void) someMethodNotAnnaotated: (MyClass*)In {
 _nonSynth = 0; // no-warning
   }
 
-@end
\ No newline at end of file
+  @end



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


[clang] 2e5e42d - [analyzer][MallocChecker] When modeling realloc-like functions, don't early return if the argument is symbolic

2020-05-19 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-19T13:59:29+02:00
New Revision: 2e5e42d4aeab98636346db558e89ab9b122c9dc3

URL: 
https://github.com/llvm/llvm-project/commit/2e5e42d4aeab98636346db558e89ab9b122c9dc3
DIFF: 
https://github.com/llvm/llvm-project/commit/2e5e42d4aeab98636346db558e89ab9b122c9dc3.diff

LOG: [analyzer][MallocChecker] When modeling realloc-like functions, don't 
early return if the argument is symbolic

The very essence of MallocChecker lies in 2 overload sets: the FreeMemAux
functions and the MallocMemAux functions. The former houses most of the error
checking as well (aside from leaks), such as incorrect deallocation. There, we
check whether the argument's MemSpaceRegion is the heap or unknown, and if it
isn't, we know we encountered a bug (aside from a corner case patched by
@balazske in D76830), as specified by MEM34-C.

In ReallocMemAux, which really is the combination of  FreeMemAux and
MallocMemAux, we incorrectly early returned if the memory argument of realloc is
non-symbolic. The problem is, one of the cases where this happens when we know
precisely what the region is, like an array, as demonstrated in the test file.
So, lets get rid of this false negative :^)

Side note, I dislike the warning message and the associated checker name, but
I'll address it in a later patch.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/malloc.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 2bff317873c1..d108f88776d4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2371,13 +2371,7 @@ MallocChecker::ReallocMemAux(CheckerContext , const 
CallExpr *CE,
   if (PrtIsNull && SizeIsZero)
 return State;
 
-  // Get the from and to pointer symbols as in toPtr = realloc(fromPtr, size).
   assert(!PrtIsNull);
-  SymbolRef FromPtr = arg0Val.getAsSymbol();
-  SVal RetVal = C.getSVal(CE);
-  SymbolRef ToPtr = RetVal.getAsSymbol();
-  if (!FromPtr || !ToPtr)
-return nullptr;
 
   bool IsKnownToBeAllocated = false;
 
@@ -2406,6 +2400,14 @@ MallocChecker::ReallocMemAux(CheckerContext , const 
CallExpr *CE,
 else if (!IsKnownToBeAllocated)
   Kind = OAR_DoNotTrackAfterFailure;
 
+// Get the from and to pointer symbols as in toPtr = realloc(fromPtr, 
size).
+SymbolRef FromPtr = arg0Val.getAsSymbol();
+SVal RetVal = C.getSVal(CE);
+SymbolRef ToPtr = RetVal.getAsSymbol();
+assert(FromPtr && ToPtr &&
+   "By this point, FreeMemAux and MallocMemAux should have checked "
+   "whether the argument or the return value is symbolic!");
+
 // Record the info about the reallocated symbol so that we could properly
 // process failed reallocation.
 stateRealloc = stateRealloc->set(ToPtr,

diff  --git a/clang/test/Analysis/malloc.c b/clang/test/Analysis/malloc.c
index 5288e21a2821..b7a29db274b4 100644
--- a/clang/test/Analysis/malloc.c
+++ b/clang/test/Analysis/malloc.c
@@ -1828,6 +1828,21 @@ void testCStyleListItems(struct ListInfo *list) {
   list_add(list, >li); // will free 'x'.
 }
 
+// MEM34-C. Only free memory allocated dynamically
+// Second non-compliant example.
+// 
https://wiki.sei.cmu.edu/confluence/display/c/MEM34-C.+Only+free+memory+allocated+dynamically
+enum { BUFSIZE = 256 };
+
+void MEM34_C(void) {
+  char buf[BUFSIZE];
+  char *p = (char *)realloc(buf, 2 * BUFSIZE);
+  // expected-warning@-1{{Argument to realloc() is the address of the local \
+variable 'buf', which is not memory allocated by malloc() [unix.Malloc]}}
+  if (p == NULL) {
+/* Handle error */
+  }
+}
+
 // 
 // False negatives.
 



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


[clang] 9d69072 - [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

2020-05-18 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-19T00:18:38+02:00
New Revision: 9d69072fb80755a0029a01c74892b4bf03f20f65

URL: 
https://github.com/llvm/llvm-project/commit/9d69072fb80755a0029a01c74892b4bf03f20f65
DIFF: 
https://github.com/llvm/llvm-project/commit/9d69072fb80755a0029a01c74892b4bf03f20f65.diff

LOG: [analyzer][NFC] Introduce CXXDeallocatorCall, deploy it in MallocChecker

One of the pain points in simplifying MallocCheckers interface by gradually
changing to CallEvent is that a variety of C++ allocation and deallocation
functionalities are modeled through preStmt<...> where CallEvent is unavailable,
and a single one of these callbacks can prevent a mass parameter change.

This patch introduces a new CallEvent, CXXDeallocatorCall, which happens after
preStmt, and can completely replace that callback as
demonstrated.

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

Added: 
clang/unittests/StaticAnalyzer/CallEventTest.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index bc562a4ca6f1..871875b45b1b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -67,8 +67,9 @@ enum CallEventKind {
   CE_BEG_CXX_CONSTRUCTOR_CALLS = CE_CXXConstructor,
   CE_END_CXX_CONSTRUCTOR_CALLS = CE_CXXInheritedConstructor,
   CE_CXXAllocator,
+  CE_CXXDeallocator,
   CE_BEG_FUNCTION_CALLS = CE_Function,
-  CE_END_FUNCTION_CALLS = CE_CXXAllocator,
+  CE_END_FUNCTION_CALLS = CE_CXXDeallocator,
   CE_Block,
   CE_ObjCMessage
 };
@@ -1045,6 +1046,55 @@ class CXXAllocatorCall : public AnyFunctionCall {
   }
 };
 
+/// Represents the memory deallocation call in a C++ delete-expression.
+///
+/// This is a call to "operator delete".
+// FIXME: CXXDeleteExpr isn't present for custom delete operators, or even for
+// some those that are in the standard library, like the no-throw or align_val
+// versions.
+// Some pointers:
+// http://lists.llvm.org/pipermail/cfe-dev/2020-April/065080.html
+// clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp
+// clang/unittests/StaticAnalyzer/CallEventTest.cpp
+class CXXDeallocatorCall : public AnyFunctionCall {
+  friend class CallEventManager;
+
+protected:
+  CXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef St,
+ const LocationContext *LCtx)
+  : AnyFunctionCall(E, St, LCtx) {}
+  CXXDeallocatorCall(const CXXDeallocatorCall ) = default;
+
+  void cloneTo(void *Dest) const override {
+new (Dest) CXXDeallocatorCall(*this);
+  }
+
+public:
+  virtual const CXXDeleteExpr *getOriginExpr() const {
+return cast(AnyFunctionCall::getOriginExpr());
+  }
+
+  const FunctionDecl *getDecl() const override {
+return getOriginExpr()->getOperatorDelete();
+  }
+
+  unsigned getNumArgs() const override { return getDecl()->getNumParams(); }
+
+  const Expr *getArgExpr(unsigned Index) const override {
+// CXXDeleteExpr's only have a single argument.
+return getOriginExpr()->getArgument();
+  }
+
+  Kind getKind() const override { return CE_CXXDeallocator; }
+  virtual StringRef getKindAsString() const override {
+return "CXXDeallocatorCall";
+  }
+
+  static bool classof(const CallEvent *CE) {
+return CE->getKind() == CE_CXXDeallocator;
+  }
+};
+
 /// Represents the ways an Objective-C message send can occur.
 //
 // Note to maintainers: OCM_Message should always be last, since it does not
@@ -1367,6 +1417,12 @@ class CallEventManager {
   const LocationContext *LCtx) {
 return create(E, State, LCtx);
   }
+
+  CallEventRef
+  getCXXDeallocatorCall(const CXXDeleteExpr *E, ProgramStateRef State,
+const LocationContext *LCtx) {
+return create(E, State, LCtx);
+  }
 };
 
 template 

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
index 97556ca856a0..7c5833762008 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
@@ -92,6 +92,8 @@ void DeleteWithNonVirtualDtorChecker::checkPreStmt(const 
CXXDeleteExpr *DE,
  "Logic error"));
 
   ExplodedNode *N = C.generateNonFatalErrorNode();

[clang] 2a12acd - [analyzer][StreamChecker] Don't make StreamTestChecker depend on StreamChecker for the time being

2020-05-13 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-05-13T20:05:11+02:00
New Revision: 2a12acda4c9fad4d69dce7a43e99690df357648c

URL: 
https://github.com/llvm/llvm-project/commit/2a12acda4c9fad4d69dce7a43e99690df357648c
DIFF: 
https://github.com/llvm/llvm-project/commit/2a12acda4c9fad4d69dce7a43e99690df357648c.diff

LOG: [analyzer][StreamChecker] Don't make StreamTestChecker depend on 
StreamChecker for the time being

The comment in Checkers.td explains whats going on. As StreamChecker grows,
expect a need to have smaller checkers out of it, but let that be a worry for
later.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/test/Analysis/stream-error.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 4315b0984abc..c35f8b115f16 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1430,9 +1430,14 @@ def TaintTesterChecker : Checker<"TaintTest">,
   HelpText<"Mark tainted symbols as such.">,
   Documentation;
 
+// This checker *technically* depends on SteamChecker, but we don't allow
+// dependency checkers to emit diagnostics, and a debug checker isn't worth
+// the chore needed to create a modeling portion on its own. Since this checker
+// is for development purposes only anyways, make sure that StreamChecker is
+// also enabled, at least for the time being.
 def StreamTesterChecker : Checker<"StreamTester">,
-  HelpText<"Add test functions to StreamChecker for test and debugging 
purposes.">,
-  Dependencies<[StreamChecker]>,
+  HelpText<"Add test functions to StreamChecker for test and debugging "
+   "purposes.">,
   Documentation;
 
 def ExprInspectionChecker : Checker<"ExprInspection">,

diff  --git a/clang/test/Analysis/stream-error.c 
b/clang/test/Analysis/stream-error.c
index 2bd25ca30fa3..2302f3efc2f1 100644
--- a/clang/test/Analysis/stream-error.c
+++ b/clang/test/Analysis/stream-error.c
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core 
-analyzer-checker=debug.StreamTester,debug.ExprInspection -analyzer-store 
region -verify %s
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN: -analyzer-checker=core \
+// RUN: -analyzer-checker=alpha.unix.Stream \
+// RUN: -analyzer-checker=debug.StreamTester \
+// RUN: -analyzer-checker=debug.ExprInspection
 
 #include "Inputs/system-header-simulator.h"
 



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


[clang] 023c4d4 - [analyzer][AnalysisOrderChecker] Display the CallEvent type in preCall/postCall

2020-04-09 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-04-09T16:41:07+02:00
New Revision: 023c4d400ef5acf3a7339dc8452ce552b15a9ae4

URL: 
https://github.com/llvm/llvm-project/commit/023c4d400ef5acf3a7339dc8452ce552b15a9ae4
DIFF: 
https://github.com/llvm/llvm-project/commit/023c4d400ef5acf3a7339dc8452ce552b15a9ae4.diff

LOG: [analyzer][AnalysisOrderChecker] Display the CallEvent type in 
preCall/postCall

Exactly what it says on the tin! The included testfile demonstrates why this is
important -- for C++ dynamic memory operators, we don't always recognize custom,
or even standard-specified new/delete operators as CXXAllocatorCall or
CXXDeallocatorCall.

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

Added: 
clang/test/Analysis/cxx-dynamic-memory-analysis-order.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
clang/lib/StaticAnalyzer/Core/CallEvent.cpp
clang/test/Analysis/Inputs/system-header-simulator-cxx.h
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/diagnostics/explicit-suppression.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 31e7e02c192b..4315b0984abc 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1275,6 +1275,30 @@ def AnalysisOrderChecker : Checker<"AnalysisOrder">,
   "false",
   Released,
   Hide>,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
+CmdLineOption,
 CmdLineOption,
   "false",
   Released,
   Hide>,
+CmdLineOption,
 CmdLineOptiongetKind() == CE_Function;
@@ -634,13 +638,12 @@ class BlockCall : public CallEvent {
   void getInitialStackFrameContents(const StackFrameContext *CalleeCtx,
 BindingsTy ) const override;
 
-  ArrayRef parameters() const override;
+  ArrayRef parameters() const override;
 
   Kind getKind() const override { return CE_Block; }
+  virtual StringRef getKindAsString() const override { return "BlockCall"; }
 
-  static bool classof(const CallEvent *CA) {
-return CA->getKind() == CE_Block;
-  }
+  static bool classof(const CallEvent *CA) { return CA->getKind() == CE_Block; 
}
 };
 
 /// Represents a non-static C++ member function call, no matter how
@@ -712,6 +715,7 @@ class CXXMemberCall : public CXXInstanceCall {
   RuntimeDefinition getRuntimeDefinition() const override;
 
   Kind getKind() const override { return CE_CXXMember; }
+  virtual StringRef getKindAsString() const override { return "CXXMemberCall"; 
}
 
   static bool classof(const CallEvent *CA) {
 return CA->getKind() == CE_CXXMember;
@@ -751,6 +755,9 @@ class CXXMemberOperatorCall : public CXXInstanceCall {
   const Expr *getCXXThisExpr() const override;
 
   Kind getKind() const override { return CE_CXXMemberOperator; }
+  virtual StringRef getKindAsString() const override {
+return "CXXMemberOperatorCall";
+  }
 
   static bool classof(const CallEvent *CA) {
 return CA->getKind() == CE_CXXMemberOperator;
@@ -815,6 +822,9 @@ class CXXDestructorCall : public CXXInstanceCall {
   }
 
   Kind getKind() const override { return CE_CXXDestructor; }
+  virtual StringRef getKindAsString() const override {
+return "CXXDestructorCall";
+  }
 
   static bool classof(const CallEvent *CA) {
 return CA->getKind() == CE_CXXDestructor;
@@ -887,6 +897,9 @@ class CXXConstructorCall : public AnyCXXConstructorCall {
   }
 
   Kind getKind() const override { return CE_CXXConstructor; }
+  virtual StringRef getKindAsString() const override {
+return "CXXConstructorCall";
+  }
 
   static bool classof(const CallEvent *CA) {
 return CA->getKind() == CE_CXXConstructor;
@@ -964,6 +977,9 @@ class CXXInheritedConstructorCall : public 
AnyCXXConstructorCall {
   }
 
   Kind getKind() const override { return CE_CXXInheritedConstructor; }
+  virtual StringRef getKindAsString() const override {
+return "CXXInheritedConstructorCall";
+  }
 
   static bool classof(const CallEvent *CA) {
 return CA->getKind() == CE_CXXInheritedConstructor;
@@ -1020,6 +1036,9 @@ class CXXAllocatorCall : public AnyFunctionCall {
   }
 
   Kind getKind() const override { return CE_CXXAllocator; }
+  virtual StringRef getKindAsString() const override {
+return "CXXAllocatorCall";
+  }
 
   static bool classof(const CallEvent *CE) {
 return CE->getKind() == CE_CXXAllocator;
@@ -1143,6 +1162,9 @@ class ObjCMethodCall : public CallEvent {
   ArrayRef parameters() const override;
 
   Kind getKind() const override { return CE_ObjCMessage; }
+  virtual StringRef getKindAsString() const override {
+

[clang] a2b6ece - [analyzer] Display the checker name in the text output

2020-04-09 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-04-09T16:21:45+02:00
New Revision: a2b6ece1fd4e13f2f463c5075eb0e513af47e656

URL: 
https://github.com/llvm/llvm-project/commit/a2b6ece1fd4e13f2f463c5075eb0e513af47e656
DIFF: 
https://github.com/llvm/llvm-project/commit/a2b6ece1fd4e13f2f463c5075eb0e513af47e656.diff

LOG: [analyzer] Display the checker name in the text output

Exactly what it says on the tin! There is no reason I think not to have this.

Also, I added test files for checkers that emit warning under the wrong name.

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

Added: 
clang/test/Analysis/incorrect-checker-names.cpp
clang/test/Analysis/incorrect-checker-names.mm

Modified: 
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/dispatch-once.m
clang/test/Analysis/explain-svals.c
clang/test/Analysis/explain-svals.cpp
clang/test/Analysis/explain-svals.m

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index ee67f60df948..597a65c21318 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -310,6 +310,10 @@ ANALYZER_OPTION(bool, ShouldApplyFixIts, "apply-fixits",
 "Apply the fix-it hints to the files",
 false)
 
+ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
+"Display the checker name for textual outputs",
+true)
+
 
//===--===//
 // Unsigned analyzer options.
 
//===--===//

diff  --git a/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
index 07e5685e604c..f4c7e5978e19 100644
--- a/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
@@ -35,17 +35,19 @@ namespace {
 /// diagnostics in textual format for the 'text' output type.
 class TextDiagnostics : public PathDiagnosticConsumer {
   DiagnosticsEngine 
-  LangOptions LO;
+  const LangOptions 
   const bool IncludePath = false;
   const bool ShouldEmitAsError = false;
   const bool ApplyFixIts = false;
+  const bool ShouldDisplayCheckerName = false;
 
 public:
-  TextDiagnostics(DiagnosticsEngine , LangOptions LO,
+  TextDiagnostics(DiagnosticsEngine , const LangOptions ,
   bool ShouldIncludePath, const AnalyzerOptions )
   : DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
 ShouldEmitAsError(AnOpts.AnalyzerWerror),
-ApplyFixIts(AnOpts.ShouldApplyFixIts) {}
+ApplyFixIts(AnOpts.ShouldApplyFixIts),
+ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
   ~TextDiagnostics() override {}
 
   StringRef getName() const override { return "TextDiagnostics"; }
@@ -90,9 +92,13 @@ class TextDiagnostics : public PathDiagnosticConsumer {
  E = Diags.end();
  I != E; ++I) {
   const PathDiagnostic *PD = *I;
+  std::string WarningMsg =
+  (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
+  .str();
+
   reportPiece(WarnID, PD->getLocation().asLocation(),
-  PD->getShortDescription(), PD->path.back()->getRanges(),
-  PD->path.back()->getFixits());
+  (PD->getShortDescription() + WarningMsg).str(),
+  PD->path.back()->getRanges(), PD->path.back()->getFixits());
 
   // First, add extra notes, even if paths should not be included.
   for (const auto  : PD->path) {
@@ -100,7 +106,8 @@ class TextDiagnostics : public PathDiagnosticConsumer {
   continue;
 
 reportPiece(NoteID, Piece->getLocation().asLocation(),
-Piece->getString(), Piece->getRanges(), 
Piece->getFixits());
+Piece->getString(), Piece->getRanges(),
+Piece->getFixits());
   }
 
   if (!IncludePath)
@@ -113,7 +120,8 @@ class TextDiagnostics : public PathDiagnosticConsumer {
   continue;
 
 reportPiece(NoteID, Piece->getLocation().asLocation(),
-Piece->getString(), Piece->getRanges(), 
Piece->getFixits());
+Piece->getString(), Piece->getRanges(),
+Piece->getFixits());
   }
 }
 

diff  --git a/clang/test/Analysis/analyzer-config.c 
b/clang/test/Analysis/analyzer-config.c
index 068930e6e57f..7069ee4a938d 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -52,6 +52,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
 // 

[clang] 703a1b8 - [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

2020-03-30 Thread Kirstóf Umann via cfe-commits

Author: Louis Dionne
Date: 2020-03-30T16:01:58+02:00
New Revision: 703a1b8caf09a5262a45c2179b8131922f71cf25

URL: 
https://github.com/llvm/llvm-project/commit/703a1b8caf09a5262a45c2179b8131922f71cf25
DIFF: 
https://github.com/llvm/llvm-project/commit/703a1b8caf09a5262a45c2179b8131922f71cf25.diff

LOG: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy 
CallDescriptionMap

Since its important to know whether a function frees memory (even if its a
reallocating function!), I used two CallDescriptionMaps to merge all
CallDescriptions into it. MemFunctionInfoTy no longer makes sense, it may never
have, but for now, it would be more of a distraction then anything else.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 32a500ffd102..52786bcaf072 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -62,16 +62,19 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
+#include 
 #include 
 
 using namespace clang;
 using namespace ento;
+using namespace std::placeholders;
 
 
//===--===//
 // The types of allocation we're modeling. This is used to check whether a
@@ -93,8 +96,6 @@ enum AllocationFamily {
   AF_InnerBuffer
 };
 
-struct MemFunctionInfoTy;
-
 } // end of anonymous namespace
 
 /// Print names of allocators and deallocators.
@@ -263,47 +264,6 @@ struct ReallocPair {
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
 
-//===--===//
-// Kinds of memory operations, information about resource managing functions.
-//===--===//
-
-namespace {
-
-struct MemFunctionInfoTy {
-  /// The value of the MallocChecker:Optimistic is stored in this variable.
-  ///
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
-
-  CallDescription CD_alloca{{"alloca"}, 1}, CD_win_alloca{{"_alloca"}, 1},
-  CD_malloc{{"malloc"}, 1}, CD_BSD_malloc{{"malloc"}, 3},
-  CD_free{{"free"}, 1}, CD_realloc{{"realloc"}, 2},
-  CD_calloc{{"calloc"}, 2}, CD_valloc{{"valloc"}, 1},
-  CD_reallocf{{"reallocf"}, 2}, CD_strndup{{"strndup"}, 2},
-  CD_strdup{{"strdup"}, 1}, CD_win_strdup{{"_strdup"}, 1},
-  CD_kmalloc{{"kmalloc"}, 2}, CD_if_nameindex{{"if_nameindex"}, 1},
-  CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
-  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 1},
-  CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
-  CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
-  CD_g_try_malloc0{{"g_try_malloc0"}, 1},
-  CD_g_try_realloc{{"g_try_realloc"}, 2}, CD_g_free{{"g_free"}, 1},
-  CD_g_memdup{{"g_memdup"}, 2}, CD_g_malloc_n{{"g_malloc_n"}, 2},
-  CD_g_malloc0_n{{"g_malloc0_n"}, 2}, CD_g_realloc_n{{"g_realloc_n"}, 3},
-  CD_g_try_malloc_n{{"g_try_malloc_n"}, 2},
-  CD_g_try_malloc0_n{{"g_try_malloc0_n"}, 2},
-  CD_g_try_realloc_n{{"g_try_realloc_n"}, 3};
-
-  bool isMemFunction(const CallEvent ) const;
-  bool isCMemFunction(const CallEvent ) const;
-  bool isCMemFreeFunction(const CallEvent ) const;
-  bool isCMemAllocFunction(const CallEvent ) const;
-};
-} // end of anonymous namespace
-
 /// Tells if the callee is one of the builtin new/delete operators, including
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
@@ -327,7 +287,11 @@ class MallocChecker
  check::PreStmt, check::PostStmt,
  check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  MemFunctionInfoTy MemFunctionInfo;
+  /// In pessimistic mode, the checker assumes that it does not know which
+  /// functions might free the memory.
+  /// In optimistic mode, the checker assumes that all user-defined functions
+  /// which might free a 

[clang] bda3dd0 - [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

2020-03-27 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-27T14:34:09+01:00
New Revision: bda3dd0d986b33c3a327c0ee0eb8ba43aa140699

URL: 
https://github.com/llvm/llvm-project/commit/bda3dd0d986b33c3a327c0ee0eb8ba43aa140699
DIFF: 
https://github.com/llvm/llvm-project/commit/bda3dd0d986b33c3a327c0ee0eb8ba43aa140699.diff

LOG: [analyzer][NFC] Change LangOptions to CheckerManager in the 
shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patch changes the parameter of the shouldRegister*
function to CheckerManager to be able to query the analyzer options when
deciding whether the checker should be registered.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/Analysis/plugins/CheckerOptionHandling/CheckerOptionHandling.cpp
clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
clang/lib/StaticAnalyzer/Checkers/AnalyzerStatsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ArrayBoundChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
clang/lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/BoolAssignmentChecker.cpp
clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CXXSelfAssignmentChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CastSizeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
clang/lib/StaticAnalyzer/Checkers/CheckObjCInstMethSignature.cpp
clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
clang/lib/StaticAnalyzer/Checkers/CheckSizeofPointer.cpp
clang/lib/StaticAnalyzer/Checkers/ChrootChecker.cpp
clang/lib/StaticAnalyzer/Checkers/CloneChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
clang/lib/StaticAnalyzer/Checkers/DebugContainerModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DebugIteratorModeling.cpp
clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DirectIvarAssignment.cpp
clang/lib/StaticAnalyzer/Checkers/DivZeroChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DynamicTypeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/FixedAddressChecker.cpp
clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GCDAntipatternChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
clang/lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
clang/lib/StaticAnalyzer/Checkers/IvarInvalidationChecker.cpp
clang/lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp
clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MacOSKeychainAPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocOverflowSecurityChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NSAutoreleasePoolChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp

[clang] 30a8b77 - [analyzer][MallocChecker] Fix that kfree only takes a single argument

2020-03-27 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-27T13:17:35+01:00
New Revision: 30a8b77080b98ffdd1837d2ea2e74f91e40cc867

URL: 
https://github.com/llvm/llvm-project/commit/30a8b77080b98ffdd1837d2ea2e74f91e40cc867
DIFF: 
https://github.com/llvm/llvm-project/commit/30a8b77080b98ffdd1837d2ea2e74f91e40cc867.diff

LOG: [analyzer][MallocChecker] Fix that kfree only takes a single argument

Exactly what it says on the tin!

https://www.kernel.org/doc/htmldocs/kernel-api/API-kfree.html

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/kmalloc-linux.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 23b4abc67079..0f007955abc8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -285,7 +285,7 @@ struct MemFunctionInfoTy {
   CD_strdup{{"strdup"}, 1}, CD_win_strdup{{"_strdup"}, 1},
   CD_kmalloc{{"kmalloc"}, 2}, CD_if_nameindex{{"if_nameindex"}, 1},
   CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
-  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 2},
+  CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 1},
   CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
   CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
   CD_g_try_malloc0{{"g_try_malloc0"}, 1},

diff  --git a/clang/test/Analysis/kmalloc-linux.c 
b/clang/test/Analysis/kmalloc-linux.c
index ccb6188a51b3..6b17ef2d5a79 100644
--- a/clang/test/Analysis/kmalloc-linux.c
+++ b/clang/test/Analysis/kmalloc-linux.c
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-linux %s -verify \
+// RUN:   -Wno-incompatible-library-redeclaration \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=unix.Malloc
 
 #define __GFP_ZERO 0x8000
 #define NULL ((void *)0)
@@ -6,6 +9,7 @@
 typedef __typeof(sizeof(int)) size_t;
 
 void *kmalloc(size_t, int);
+void kfree(void *);
 
 struct test {
 };
@@ -61,6 +65,8 @@ typedef unsigned long long uint64_t;
 
 struct malloc_type;
 
+// 3 parameter malloc:
+// https://www.freebsd.org/cgi/man.cgi?query=malloc=9
 void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
 
 void test_3arg_malloc(struct malloc_type *mtp) {
@@ -97,7 +103,7 @@ void test_3arg_malloc_indeterminate(struct malloc_type *mtp, 
int flags) {
   struct test **list, *t;
   int i;
 
-  list = alloc(sizeof(*list) * 10, mtp, flags);
+  list = malloc(sizeof(*list) * 10, mtp, flags);
   if (list == NULL)
 return;
 
@@ -107,3 +113,11 @@ void test_3arg_malloc_indeterminate(struct malloc_type 
*mtp, int flags) {
   }
   kfree(list);
 }
+
+void test_3arg_malloc_leak(struct malloc_type *mtp, int flags) {
+  struct test **list;
+
+  list = malloc(sizeof(*list) * 10, mtp, flags);
+  if (list == NULL)
+return;
+} // expected-warning{{Potential leak of memory pointed to by 'list'}}



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


[clang] 4dc8472 - [analyzer] Add the Preprocessor to CheckerManager

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-26T17:29:52+01:00
New Revision: 4dc8472942ce60f29de9587b9451cc3d49df7abf

URL: 
https://github.com/llvm/llvm-project/commit/4dc8472942ce60f29de9587b9451cc3d49df7abf
DIFF: 
https://github.com/llvm/llvm-project/commit/4dc8472942ce60f29de9587b9451cc3d49df7abf.diff

LOG: [analyzer] Add the Preprocessor to CheckerManager

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
clang/unittests/StaticAnalyzer/Reusables.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4635ca35736a..f34f5c239290 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -122,9 +122,10 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext *Context;
+  ASTContext *Context = nullptr;
   const LangOptions LangOpts;
-  AnalyzerOptions 
+  const AnalyzerOptions 
+  const Preprocessor *PP = nullptr;
   CheckerNameRef CurrentCheckerName;
   DiagnosticsEngine 
   std::unique_ptr Registry;
@@ -137,15 +138,16 @@ class CheckerManager {
   // dependencies look like this: Core -> Checkers -> Frontend.
 
   CheckerManager(
-  ASTContext , AnalyzerOptions ,
+  ASTContext , AnalyzerOptions , const Preprocessor ,
   ArrayRef plugins,
   ArrayRef> checkerRegistrationFns);
 
   /// Constructs a CheckerManager that ignores all non TblGen-generated
   /// checkers. Useful for unit testing, unless the checker infrastructure
   /// itself is tested.
-  CheckerManager(ASTContext , AnalyzerOptions )
-  : CheckerManager(Context, AOptions, {}, {}) {}
+  CheckerManager(ASTContext , AnalyzerOptions ,
+ const Preprocessor )
+  : CheckerManager(Context, AOptions, PP, {}, {}) {}
 
   /// Constructs a CheckerManager without requiring an AST. No checker
   /// registration will take place. Only useful for retrieving the
@@ -163,7 +165,11 @@ class CheckerManager {
   void finishedCheckerRegistration();
 
   const LangOptions () const { return LangOpts; }
-  AnalyzerOptions () const { return AOptions; }
+  const AnalyzerOptions () const { return AOptions; }
+  const Preprocessor () const {
+assert(PP);
+return *PP;
+  }
   const CheckerRegistry () const { return *Registry; }
   DiagnosticsEngine () const { return Diags; }
   ASTContext () const {

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 6f8cb1432bb1..1af93cc45363 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1485,7 +1485,7 @@ bool ento::shouldRegisterRetainCountBase(const 
LangOptions ) {
 // it should be possible to enable the NS/CF retain count checker as
 // osx.cocoa.RetainCount, and it should be possible to disable
 // osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
-static bool getOption(AnalyzerOptions ,
+static bool getOption(const AnalyzerOptions ,
   StringRef Postfix,
   StringRef Value) {
   auto I = Options.Config.find(

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 14f551403d98..562d553b8a84 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -608,7 +608,7 @@ std::string clang::ento::getVariableName(const FieldDecl 
*Field) {
 void ento::registerUninitializedObjectChecker(CheckerManager ) {
   auto Chk = Mgr.registerChecker();
 
-  AnalyzerOptions  = Mgr.getAnalyzerOptions();
+  const AnalyzerOptions  = Mgr.getAnalyzerOptions();
   UninitObjCheckerOptions  = Chk->Opts;
 
   ChOpts.IsPedantic = AnOpts.getCheckerBooleanOption(Chk, "Pedantic");

diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp 
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 6c123521e5f8..74c689730e58 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -208,7 +208,7 @@ class AnalysisConsumer : public AnalysisASTConsumer,
 
   void Initialize(ASTContext ) override {
 Ctx = 
-checkerMgr = std::make_unique(*Ctx, *Opts, 

[clang] 0766d1d - Make a windows buildbot happy

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-26T17:04:23+01:00
New Revision: 0766d1dca8685e77e8672b13f26797a5d4c8b4d7

URL: 
https://github.com/llvm/llvm-project/commit/0766d1dca8685e77e8672b13f26797a5d4c8b4d7
DIFF: 
https://github.com/llvm/llvm-project/commit/0766d1dca8685e77e8672b13f26797a5d4c8b4d7.diff

LOG: Make a windows buildbot happy

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index dde2409ed72c..73594bb8b380 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -241,8 +241,9 @@ class CheckerRegistry {
   bool IsHidden = false) {
 // Avoid MSVC's Compiler Error C2276:
 // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
-addChecker(, , FullName,
-   Desc, DocsUri, IsHidden);
+addChecker(::initializeManager,
+   ::returnTrue, FullName, Desc, DocsUri,
+   IsHidden);
   }
 
   /// Makes the checker with the full name \p fullName depends on the checker



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


[clang] 2aac0c4 - Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes"

2020-03-26 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2020-03-26T16:12:38+01:00
New Revision: 2aac0c47aed8d1eff7ab6d858173c532b881d948

URL: 
https://github.com/llvm/llvm-project/commit/2aac0c47aed8d1eff7ab6d858173c532b881d948
DIFF: 
https://github.com/llvm/llvm-project/commit/2aac0c47aed8d1eff7ab6d858173c532b881d948.diff

LOG: Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow 
CheckerManager to be constructed for non-analysis purposes"

Originally commited in rG57b8a407493c34c3680e7e1e4cb82e097f43744a, but
it broke the modules bot. This is solved by putting the contructors of
the CheckerManager class to the Frontend library.

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

Added: 
clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4454d7603b27..4635ca35736a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
@@ -121,14 +122,36 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext 
+  ASTContext *Context;
   const LangOptions LangOpts;
   AnalyzerOptions 
   CheckerNameRef CurrentCheckerName;
+  DiagnosticsEngine 
+  std::unique_ptr Registry;
 
 public:
+  // These constructors are defined in the Frontend library, because
+  // CheckerRegistry, a crucial component of the initialization is in there.
+  // CheckerRegistry cannot be moved to the Core library, because the checker
+  // registration functions are defined in the Checkers library, and the 
library
+  // dependencies look like this: Core -> Checkers -> Frontend.
+
+  CheckerManager(
+  ASTContext , AnalyzerOptions ,
+  ArrayRef plugins,
+  ArrayRef> checkerRegistrationFns);
+
+  /// Constructs a CheckerManager that ignores all non TblGen-generated
+  /// checkers. Useful for unit testing, unless the checker infrastructure
+  /// itself is tested.
   CheckerManager(ASTContext , AnalyzerOptions )
-  : Context(Context), LangOpts(Context.getLangOpts()), AOptions(AOptions) 
{}
+  : CheckerManager(Context, AOptions, {}, {}) {}
+
+  /// Constructs a CheckerManager without requiring an AST. No checker
+  /// registration will take place. Only useful for retrieving the
+  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  CheckerManager(AnalyzerOptions , const LangOptions ,
+ DiagnosticsEngine , ArrayRef plugins);
 
   ~CheckerManager();
 
@@ -141,7 +164,12 @@ class CheckerManager {
 
   const LangOptions () const { return LangOpts; }
   AnalyzerOptions () const { return AOptions; }
-  ASTContext () const { return Context; }
+  const CheckerRegistry () const { return *Registry; }
+  DiagnosticsEngine () const { return Diags; }
+  ASTContext () const {
+assert(Context);
+return *Context;
+  }
 
   /// Emits an error through a DiagnosticsEngine about an invalid user supplied
   /// checker option value.

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
index 2d24e6a9586b..bcc29a60ad70 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
@@ -55,7 +55,7 @@ class AnalysisASTConsumer : public ASTConsumer {
 std::unique_ptr
 CreateAnalysisConsumer(CompilerInstance );
 
-} // end GR namespace
+} // namespace ento
 
 } // end clang namespace
 

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
new file mode 100644
index ..a30c241e1350
--- 

[clang] 7bf871c - [analyzer][NFC] Move the text output type to its own file, move code to PathDiagnosticConsumer creator functions

2020-03-23 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-23T21:50:40+01:00
New Revision: 7bf871c39f739a7382ba89c97f50fd047f36f894

URL: 
https://github.com/llvm/llvm-project/commit/7bf871c39f739a7382ba89c97f50fd047f36f894
DIFF: 
https://github.com/llvm/llvm-project/commit/7bf871c39f739a7382ba89c97f50fd047f36f894.diff

LOG: [analyzer][NFC] Move the text output type to its own file, move code to 
PathDiagnosticConsumer creator functions

TableGen and .def files (which are meant to be used with the preprocessor) come
with obvious downsides. One of those issues is that generated switch-case
branches have to be identical. This pushes corner cases either to an outer code
block, or into the generated code.

Inspect the removed code in AnalysisConsumer::DigestAnalyzerOptions. You can see
how corner cases like a not existing output file, the analysis output type being
set to PD_NONE, or whether to complement the output with additional diagnostics
on stderr lay around the preprocessor generated code. This is a bit problematic,
as to how to deal with such errors is not in the hands of the users of this
interface (those implementing output types, like PlistDiagnostics etc).

This patch changes this by moving these corner cases into the generated code,
more specifically, into the called functions. In addition, I introduced a new
output type for convenience purposes, PD_TEXT_MINIMAL, which always existed
conceptually, but never in the actual Analyses.def file. This refactoring
allowed me to move TextDiagnostics (renamed from ClangDiagPathDiagConsumer) to
its own file, which it really deserved.

Also, those that had the misfortune to gaze upon Analyses.def will probably
enjoy the sight that a clang-format did on it.

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

Added: 
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/Analyses.def
clang/lib/StaticAnalyzer/Core/CMakeLists.txt
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/Analyses.def 
b/clang/include/clang/StaticAnalyzer/Core/Analyses.def
index 377451576148..c4e5f5be6fd7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Analyses.def
+++ b/clang/include/clang/StaticAnalyzer/Core/Analyses.def
@@ -14,41 +14,80 @@
 #define ANALYSIS_STORE(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store", 
CreateRegionStoreManager)
+ANALYSIS_STORE(RegionStore, "region", "Use region-based analyzer store",
+   CreateRegionStoreManager)
 
 #ifndef ANALYSIS_CONSTRAINTS
 #define ANALYSIS_CONSTRAINTS(NAME, CMDFLAG, DESC, CREATFN)
 #endif
 
-ANALYSIS_CONSTRAINTS(RangeConstraints, "range", "Use constraint tracking of 
concrete value ranges", CreateRangeConstraintManager)
-ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver", 
CreateZ3ConstraintManager)
+ANALYSIS_CONSTRAINTS(RangeConstraints, "range",
+ "Use constraint tracking of concrete value ranges",
+ CreateRangeConstraintManager)
+
+ANALYSIS_CONSTRAINTS(Z3Constraints, "z3", "Use Z3 contraint solver",
+ CreateZ3ConstraintManager)
 
 #ifndef ANALYSIS_DIAGNOSTICS
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
 #endif
 
-ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML", 
createHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "html-single-file", "Output analysis 
results using HTML (not allowing for multi-file bugs)", 
createHTMLSingleFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", 
createPlistDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis 
results using Plists (allowing for multi-file bugs)", 
createPlistMultiFileDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using 
HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(SARIF, "sarif", "Output analysis results in a SARIF 
file", createSarifDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", 
createTextPathDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML",
+ createHTMLDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(
+HTML_SINGLE_FILE, "html-single-file",
+"Output analysis results using HTML (not allowing for multi-file bugs)",
+createHTMLSingleFileDiagnosticConsumer)
+
+ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists",
+ 

[clang] 57b8a40 - [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-23 Thread Kirstóf Umann via cfe-commits

Author: Kristóf Umann
Date: 2020-03-23T17:09:49+01:00
New Revision: 57b8a407493c34c3680e7e1e4cb82e097f43744a

URL: 
https://github.com/llvm/llvm-project/commit/57b8a407493c34c3680e7e1e4cb82e097f43744a
DIFF: 
https://github.com/llvm/llvm-project/commit/57b8a407493c34c3680e7e1e4cb82e097f43744a.diff

LOG: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow 
CheckerManager to be constructed for non-analysis purposes

Its been a while since my CheckerRegistry related patches landed, allow me to
refresh your memory:

During compilation, TblGen turns
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td into
(build 
directory)/tools/clang/include/clang/StaticAnalyzer/Checkers/Checkers.inc.
This is a file that contains the full name of the checkers, their options, etc.

The class that is responsible for parsing this file is CheckerRegistry. The job
of this class is to establish what checkers are available for the analyzer (even
from plugins and statically linked but non-tblgen generated files!), and
calculate which ones should be turned on according to the analyzer's invocation.

CheckerManager is the class that is responsible for the construction and storage
of checkers. This process works by first creating a CheckerRegistry object, and
passing itself to CheckerRegistry::initializeManager(CheckerManager&), which
will call the checker registry functions (for example registerMallocChecker) on
it.

The big problem here is that these two classes lie in two different libraries,
so their interaction is pretty awkward. This used to be far worse, but I
refactored much of it, which made things better but nowhere near perfect.

---

This patch changes how the above mentioned two classes interact. CheckerRegistry
is mainly used by CheckerManager, and they are so intertwined, it makes a lot of
sense to turn in into a field, instead of a one-time local variable. This has
additional benefits: much of the information that CheckerRegistry conveniently
holds is no longer thrown away right after the analyzer's initialization, and
opens the possibility to pass CheckerManager in the shouldRegister* function
rather then LangOptions (D75271).

There are a few problems with this. CheckerManager isn't the only user, when we
honor help flags like -analyzer-checker-help, we only have access to a
CompilerInstance class, that is before the point of parsing the AST.
CheckerManager makes little sense without ASTContext, so I made some changes and
added new constructors to make it constructible for the use of help flags.

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

Added: 
clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4454d7603b27..6faf0f2e0afd 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -14,9 +14,11 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
@@ -40,7 +42,6 @@ class BugReporter;
 class CallEvent;
 class CheckerBase;
 class CheckerContext;
-class CheckerRegistry;
 class ExplodedGraph;
 class ExplodedNode;
 class ExplodedNodeSet;
@@ -121,14 +122,42 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext 
+  ASTContext *Context;
   const LangOptions LangOpts;
   AnalyzerOptions 
   CheckerNameRef CurrentCheckerName;
+  DiagnosticsEngine 
+  CheckerRegistry Registry;
 
 public:
+  CheckerManager(
+  ASTContext , AnalyzerOptions ,
+  ArrayRef plugins,
+  ArrayRef> checkerRegistrationFns)
+  : Context(), LangOpts(Context.getLangOpts()), AOptions(AOptions),
+Diags(Context.getDiagnostics()),
+ 

[clang] 58884eb - [analyzer][NFC] Refactor the checker registration unit test file

2020-03-09 Thread Kirstóf Umann via cfe-commits

Author: Kirstóf Umann
Date: 2020-03-09T16:38:30+01:00
New Revision: 58884eb6489880646896d72fcd723813f8a13299

URL: 
https://github.com/llvm/llvm-project/commit/58884eb6489880646896d72fcd723813f8a13299
DIFF: 
https://github.com/llvm/llvm-project/commit/58884eb6489880646896d72fcd723813f8a13299.diff

LOG: [analyzer][NFC] Refactor the checker registration unit test file

Nothing exciting to see here! The new interface allows for more fine tuning
(register but disable a checker, add custom checker registry functions, etc),
that was basically the point.

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

Added: 
clang/unittests/StaticAnalyzer/CheckerRegistration.h

Modified: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index bc258160ada4..269c226f2e1d 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -204,16 +204,14 @@ class CheckerRegistry {
 
   using PackageInfoList = llvm::SmallVector;
 
-private:
-  template  static void initializeManager(CheckerManager ) {
+  template  static void addToCheckerMgr(CheckerManager ) {
 mgr.registerChecker();
   }
 
-  template  static bool returnTrue(const LangOptions ) {
+  static bool returnTrue(const LangOptions ) {
 return true;
   }
 
-public:
   /// Adds a checker to the registry. Use this non-templated overload when your
   /// checker requires custom initialization.
   void addChecker(InitializationFunction Fn, ShouldRegisterFunction sfn,
@@ -227,9 +225,8 @@ class CheckerRegistry {
   bool IsHidden = false) {
 // Avoid MSVC's Compiler Error C2276:
 // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
-addChecker(::initializeManager,
-   ::returnTrue, FullName, Desc, DocsUri,
-   IsHidden);
+addChecker(::addToCheckerMgr,
+   ::returnTrue, FullName, Desc, DocsUri, 
IsHidden);
   }
 
   /// Makes the checker with the full name \p fullName depends on the checker

diff  --git a/clang/unittests/StaticAnalyzer/CheckerRegistration.h 
b/clang/unittests/StaticAnalyzer/CheckerRegistration.h
new file mode 100644
index ..0bbed9b7784f
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/CheckerRegistration.h
@@ -0,0 +1,81 @@
+//===- unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp 
===//
+//
+// 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
+//
+//===--===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+
+namespace clang {
+namespace ento {
+
+class DiagConsumer : public PathDiagnosticConsumer {
+  llvm::raw_ostream 
+
+public:
+  DiagConsumer(llvm::raw_ostream ) : Output(Output) {}
+  void FlushDiagnosticsImpl(std::vector ,
+FilesMade *filesMade) override {
+for (const auto *PD : Diags)
+  Output << PD->getCheckerName() << ":" << PD->getShortDescription() << 
'\n';
+  }
+
+  StringRef getName() const override { return "Test"; }
+};
+
+using AddCheckerFn = void(AnalysisASTConsumer ,
+  AnalyzerOptions );
+
+template 
+void addChecker(AnalysisASTConsumer ,
+AnalyzerOptions ) {
+  Fn1(AnalysisConsumer, AnOpts);
+  addChecker(AnalysisConsumer, AnOpts);
+}
+
+template 
+void addChecker(AnalysisASTConsumer ,
+AnalyzerOptions ) {
+  Fn1(AnalysisConsumer, AnOpts);
+}
+
+template 
+class TestAction : public ASTFrontendAction {
+  llvm::raw_ostream 
+
+public:
+  TestAction(llvm::raw_ostream ) : DiagsOutput(DiagsOutput) {}
+
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef File) override {
+std::unique_ptr AnalysisConsumer =
+CreateAnalysisConsumer(Compiler);
+AnalysisConsumer->AddDiagnosticConsumer(new DiagConsumer(DiagsOutput));
+addChecker(*AnalysisConsumer, *Compiler.getAnalyzerOpts());
+return std::move(AnalysisConsumer);
+  }
+};
+
+template 
+bool runCheckerOnCode(const std::string , std::string ) {
+  llvm::raw_string_ostream OS(Diags);
+  return