[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

Szelethus wrote:

I merget the PR as is, but I'll keep the warning message in mind, I'm open to 
changing it as we are getting closer to moving out-of-alpha.

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus closed 
https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Add some docs for LazyCompoundValue (PR #97407)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus created 
https://github.com/llvm/llvm-project/pull/97407

Yes, I basically copy-pasted some posts from discord and Artem's book, but 
these make for a rather decent docs.

From 9fed2b7dc5395f487cb91c10eb076bb87e05e9b6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Tue, 2 Jul 2024 12:58:19 +0200
Subject: [PATCH] [analyzer][NFC] Add some docs for LazyCompoundValue

Yes, I basically copy-pasted some posts from discord and Artem's book,
but these make for a rather decent docs.
---
 .../StaticAnalyzer/Core/PathSensitive/SVals.h | 39 +++
 1 file changed, 39 insertions(+)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
index 3a4b087257149..e44cda50ef21d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
@@ -326,6 +326,12 @@ class LocAsInteger : public NonLoc {
   static bool classof(SVal V) { return V.getKind() == LocAsIntegerKind; }
 };
 
+/// The simplest example of a concrete compound value is nonloc::CompoundVal,
+/// which represents a concrete r-value of an initializer-list or a string.
+/// Internally, it contains an llvm::ImmutableList of SVal's stored inside the
+/// literal.
+///
+/// Source: https://github.com/haoNoQ/clang-analyzer-guide, v0.1, section 
5.3.2.
 class CompoundVal : public NonLoc {
   friend class ento::SValBuilder;
 
@@ -346,6 +352,39 @@ class CompoundVal : public NonLoc {
   static bool classof(SVal V) { return V.getKind() == CompoundValKind; }
 };
 
+/// The simplest example of a concrete compound value is nonloc::CompoundVal,
+/// which represents a concrete r-value of an initializer-list or a string.
+/// Internally, it contains an llvm::ImmutableList of SVal's stored inside the
+/// literal.
+///
+/// However, there is another compound value used in the analyzer, which 
appears
+/// much more often during analysis, which is nonloc::LazyCompoundVal. This
+/// value is an r-value that represents a snapshot of any structure "as a 
whole"
+/// at a given moment during the analysis. Such value is already quite far from
+/// being re- ferred to as "concrete", as many fields inside it would be 
unknown
+/// or symbolic. nonloc::LazyCompoundVal operates by storing two things:
+///   * a reference to the TypedValueRegion being snapshotted (yes, it is 
always
+/// typed), and also
+///   * a copy of the whole Store object, obtained from the ProgramState in
+/// which it was created.
+///
+/// Essentially, nonloc::LazyCompoundVal is a performance optimization for the
+/// analyzer. Because Store is immutable, creating a nonloc::LazyCompoundVal is
+/// a very cheap operation. Note that the Store contains all region bindings in
+/// the program state, not only related to the region. Later, if necessary, 
such
+/// value can be unpacked -- eg. when it is assigned to another variable.
+///
+/// Source: https://github.com/haoNoQ/clang-analyzer-guide, v0.1, section 
5.3.2.
+///
+/// If you ever need to see if a LazyCompoundVal is fully or partially
+/// (un)initialized, you can iterBindings(). This is non-typechecked lookup
+/// (i.e., you cannot figure out which specific sub-region is initialized by 
the
+/// value you look at, you only get a byte offset). You can also improve
+/// iterBindings() to make it possible to restrict the iteration to a single
+/// cluster, because within the LazyCompoundVal’s Store only the cluster that
+/// corresponds to the LazyCompoundVal’s parent region is relevant.
+///
+/// Source: 
https://discourse.llvm.org/t/analyzer-for-the-undefined-value-of-array-element-the-tracking-information-is-incomplete/49372/2
 class LazyCompoundVal : public NonLoc {
   friend class ento::SValBuilder;
 

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

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From 90cedd519da8b76c686db9c3f824b6d044e16eb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/7] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index c0d3fbd0eb961..9e9f2f4fc1a0e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43..fdea2e01215b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From 90cedd519da8b76c686db9c3f824b6d044e16eb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/6] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index c0d3fbd0eb961a..9e9f2f4fc1a0e5 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43a..fdea2e01215b44 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus closed 
https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From d717b412749f10b45a9387044e97da6981f3cad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/5] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb8..05d679b659029 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43..fdea2e01215b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -393,6 +401,162 @@ ProgramStateRef 
CStringChecker::checkNonNull(CheckerContext ,
   return stateNonNull;
 }
 
+static std::optional getIndex(ProgramStateRef State,
+  const ElementRegion *ER, CharKind CK) {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SVB.getContext();
+
+  if (CK == CharKind::Regular) {
+if (ER->getValueType() != Ctx.CharTy)
+  return {};
+return ER->getIndex();
+  }
+
+  if (ER->getValueType() != Ctx.WideCharTy)
+return {};
+
+  QualType SizeTy = Ctx.getSizeType();
+  NonLoc WideSize =
+  SVB.makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+ SizeTy)
+  .castAs();
+  SVal Offset =
+  SVB.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+  if (Offset.isUnknown())
+return {};
+  return Offset.castAs();
+}
+
+// Try to get hold of the origin region (e.g. the actual array region from an
+// element region).
+static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
+  const MemRegion *MR = ER->getSuperRegion();
+  const MemRegion *Ret = MR;
+  assert(MR);
+  if (const auto *sym = MR->getAs()) {
+SymbolRef sym2 = sym->getSymbol();
+if (!sym2)
+  return nullptr;
+Ret = sym2->getOriginRegion();
+  }
+  return dyn_cast_or_null(Ret);
+}
+
+// Basically 1 -> 1st, 12 -> 12th, etc.
+static void printIdxWithOrdinalSuffix(llvm::raw_ostream , unsigned Idx) {
+  Os << Idx << llvm::getOrdinalSuffix(Idx);

Szelethus wrote:

![image](https://github.com/llvm/llvm-project/assets/23276031/0e9f0f9e-70bd-4c00-9138-0bf39b27559c)
I'm not too hot about this one. I'd keep it as-is.

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

Szelethus wrote:

> > > I did not find a similar test for `MallocChecker` but there could be one 
> > > with similar test functions.
> > 
> > 
> > I'm not sure what tests you are referring to. I did fix your other 
> > observations.
> 
> I meant another test file where the `NoStateChangeFuncVisitor` is tested (if 
> there is any case for it).

Oh, I see. The `MallocChecker` visitor is tested in 
`clang/test/Analysis/NewDeleteLeaks.cpp`. Evaluations are on the way.

https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From d717b412749f10b45a9387044e97da6981f3cad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/4] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb8..05d679b659029 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43..fdea2e01215b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

Szelethus wrote:

> The intention of the patch makes sense to me. However, I believe that the bug 
> is inside the Store. It should not say it's `Undefined` if actually an 
> existing binding overlaps (actually completely covers) the requested region. 
> So, that said, the checker does the right thing, but the Store lies to it.

I disagree. Whether a value is _initialized_ or is _well defined_ are two 
different concepts (but happen to overlap in most cases). Reading the last byte 
of an integer as a character *should* yield an `Undefined` in my view. The 
Store doesn't lie here, we are asking the wrong question (what is the value of 
this byte?), and we don't have an interface for asking the right one (is this 
byte initialized). In this sense, the checker is doing the wrong thing :)

> And especially for memcpy-like raw memory manipulating APIs, implementing 
> this element-type-wise check is really difficult. Partially because in CSA we 
> have only limited trustworthy type information for such buffers.
> 
> I'm also pragmatic with the subject, and believe in solutions today, than 
> waiting for one years to come. But I still want to ask if we could join 
> forces and implement the proposed Store model discussed here, as a counter 
> proposal for the original RFC? For instance, that would make such loads not 
> result in an Undefined value - unless it's actually uninitialized.
> 
> (Once we had that Store model, we would probably want to revert this 
> element-type-based solution outlined here.)

I think adding an "isUnitialized" interface would be better.



https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -393,6 +401,173 @@ ProgramStateRef 
CStringChecker::checkNonNull(CheckerContext ,
   return stateNonNull;
 }
 
+static std::optional getIndex(ProgramStateRef State,
+  const ElementRegion *ER, CharKind CK) {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  if (CK == CharKind::Regular) {
+if (ER->getValueType() != Ctx.CharTy)
+  return {};
+return ER->getIndex();
+  }
+
+  if (ER->getValueType() != Ctx.WideCharTy)
+return {};
+
+  QualType SizeTy = Ctx.getSizeType();
+  NonLoc WideSize =
+  SValBuilder
+  .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+  SizeTy)
+  .castAs();
+  SVal Offset =
+  SValBuilder.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+  if (Offset.isUnknown())
+return {};
+  return Offset.castAs();
+}
+
+// Try to get hold of the origin regin (e.g. the actual array region from an
+// element region).
+static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
+  const MemRegion *MR = ER->getSuperRegion();
+  const MemRegion *Ret = MR;
+  assert(MR);
+  if (const auto *sym = MR->getAs()) {
+SymbolRef sym2 = sym->getSymbol();
+if (!sym2)
+  return nullptr;
+Ret = sym2->getOriginRegion();
+  }
+  if (const auto *element = MR->getAs()) {
+Ret = element->getBaseRegion();
+  }
+  return dyn_cast_or_null(Ret);
+}
+
+// Basically 1 -> 1st, 12 -> 12th, etc.
+static void printIdxWithOrdinalSuffix(llvm::raw_ostream , unsigned Idx) {
+  Os << Idx << llvm::getOrdinalSuffix(Idx);
+}
+
+ProgramStateRef CStringChecker::checkInit(CheckerContext ,
+  ProgramStateRef State,
+  AnyArgExpr Buffer, SVal Element,
+  SVal Size) const {
+
+  // If a previous check has failed, propagate the failure.
+  if (!State)
+return nullptr;
+
+  const MemRegion *R = Element.getAsRegion();
+  if (!R)
+return State;
+
+  const auto *ER = dyn_cast(R);
+  if (!ER)
+return State;
+
+  const TypedValueRegion *Orig = getOriginRegion(ER);
+  if (!Orig)
+return State;
+
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  // FIXME: We ought to able to check objects as well. Maybe
+  // UninitializedObjectChecker could help?
+  if (!Orig->getValueType()->isArrayType())
+return State;
+
+  const QualType ElemTy = Ctx.getBaseElementType(Orig->getValueType());
+  const NonLoc Zero = SValBuilder.makeZeroArrayIndex();
+
+  SVal FirstElementVal =
+  State->getLValue(ElemTy, Zero, loc::MemRegionVal(Orig)).castAs();
+  if (!isa(FirstElementVal))
+return State;
+
+  // Ensure that we wouldn't read uninitialized value.
+  if (Filter.CheckCStringUninitializedRead &&
+  State->getSVal(FirstElementVal.castAs()).isUndef()) {
+llvm::SmallString<258> Buf;
+llvm::raw_svector_ostream OS(Buf);
+OS << "The first element of the ";
+printIdxWithOrdinalSuffix(OS, Buffer.ArgumentIndex + 1);
+OS << " argument is undefined";
+emitUninitializedReadBug(C, State, Buffer.Expression, OS.str());
+return nullptr;
+  }
+
+  // We won't check whether the entire region is fully initialized -- lets just
+  // check that the first and the last element is. So, onto checking the last
+  // element:
+  const QualType IdxTy = SValBuilder.getArrayIndexType();
+
+  NonLoc ElemSize =
+  SValBuilder
+  .makeIntVal(Ctx.getTypeSizeInChars(ElemTy).getQuantity(), IdxTy)
+  .castAs();
+
+  // FIXME: Check that the size arg to the cstring function is divisible by
+  // size of the actual element type?
+
+  // The type of the argument to the cstring function is either char or wchar,
+  // but thats not the type of the original array (or memory region).
+  // Suppose the following:
+  //   int t[5];
+  //   memcpy(dst, t, sizeof(t) / sizeof(t[0]));
+  // When checking whether t is fully initialized, we see it as char array of
+  // size sizeof(int)*5. If we check the last element as a character, we read
+  // the last byte of an integer, which will be undefined. But just because
+  // that value is undefined, it doesn't mean that the element is 
uninitialized!
+  // For this reason, we need to retrieve the actual last element with the
+  // correct type.
+
+  // Divide the size argument to the cstring function by the actual element
+  // type. This value will be size of the array, or the index to the
+  // past-the-end element.
+  std::optional Offset =
+  SValBuilder
+  .evalBinOpNN(State, clang::BO_Div, Size.castAs(), ElemSize,
+   IdxTy)
+  .getAs();
+
+  // Retrieve the index of the last element.
+  const NonLoc One = SValBuilder.makeIntVal(1, IdxTy).castAs();
+  SVal LastIdx = SValBuilder.evalBinOpNN(State, BO_Sub, *Offset, One, IdxTy);
+
+  if 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -393,6 +401,173 @@ ProgramStateRef 
CStringChecker::checkNonNull(CheckerContext ,
   return stateNonNull;
 }
 
+static std::optional getIndex(ProgramStateRef State,
+  const ElementRegion *ER, CharKind CK) {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  if (CK == CharKind::Regular) {
+if (ER->getValueType() != Ctx.CharTy)
+  return {};
+return ER->getIndex();
+  }
+
+  if (ER->getValueType() != Ctx.WideCharTy)
+return {};
+
+  QualType SizeTy = Ctx.getSizeType();
+  NonLoc WideSize =
+  SValBuilder
+  .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+  SizeTy)
+  .castAs();
+  SVal Offset =
+  SValBuilder.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+  if (Offset.isUnknown())
+return {};
+  return Offset.castAs();
+}
+
+// Try to get hold of the origin regin (e.g. the actual array region from an
+// element region).
+static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
+  const MemRegion *MR = ER->getSuperRegion();
+  const MemRegion *Ret = MR;
+  assert(MR);
+  if (const auto *sym = MR->getAs()) {
+SymbolRef sym2 = sym->getSymbol();
+if (!sym2)
+  return nullptr;
+Ret = sym2->getOriginRegion();
+  }
+  if (const auto *element = MR->getAs()) {
+Ret = element->getBaseRegion();

Szelethus wrote:

I changed `getBaseRegion()` to `getSuperRegion()`, because we're only really 
trying to peel off a simple `ElementRegeion` layer. Unfortunately, I stumbled 
upon a big in StoreManager, for which I added a new test case 
(`memcpy_array_from_matrix()`).

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -393,6 +401,173 @@ ProgramStateRef 
CStringChecker::checkNonNull(CheckerContext ,
   return stateNonNull;
 }
 
+static std::optional getIndex(ProgramStateRef State,
+  const ElementRegion *ER, CharKind CK) {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  if (CK == CharKind::Regular) {
+if (ER->getValueType() != Ctx.CharTy)
+  return {};
+return ER->getIndex();
+  }
+
+  if (ER->getValueType() != Ctx.WideCharTy)
+return {};
+
+  QualType SizeTy = Ctx.getSizeType();
+  NonLoc WideSize =
+  SValBuilder
+  .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+  SizeTy)
+  .castAs();
+  SVal Offset =
+  SValBuilder.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+  if (Offset.isUnknown())
+return {};
+  return Offset.castAs();
+}
+
+// Try to get hold of the origin regin (e.g. the actual array region from an
+// element region).
+static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
+  const MemRegion *MR = ER->getSuperRegion();
+  const MemRegion *Ret = MR;
+  assert(MR);
+  if (const auto *sym = MR->getAs()) {
+SymbolRef sym2 = sym->getSymbol();
+if (!sym2)
+  return nullptr;
+Ret = sym2->getOriginRegion();
+  }
+  if (const auto *element = MR->getAs()) {
+Ret = element->getBaseRegion();
+  }
+  return dyn_cast_or_null(Ret);
+}
+
+// Basically 1 -> 1st, 12 -> 12th, etc.
+static void printIdxWithOrdinalSuffix(llvm::raw_ostream , unsigned Idx) {
+  Os << Idx << llvm::getOrdinalSuffix(Idx);
+}
+
+ProgramStateRef CStringChecker::checkInit(CheckerContext ,
+  ProgramStateRef State,
+  AnyArgExpr Buffer, SVal Element,
+  SVal Size) const {
+
+  // If a previous check has failed, propagate the failure.
+  if (!State)
+return nullptr;
+
+  const MemRegion *R = Element.getAsRegion();
+  if (!R)
+return State;
+
+  const auto *ER = dyn_cast(R);
+  if (!ER)
+return State;

Szelethus wrote:

Very well, I changed this as suggested.

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From d717b412749f10b45a9387044e97da6981f3cad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/3] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb8..05d679b659029 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43..fdea2e01215b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/95408

From d717b412749f10b45a9387044e97da6981f3cad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH 1/2] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb8..05d679b659029 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch (getKind()) {
+#define REGION(Id, Parent) 
\
+  case Id##Kind:   
\
+return #Id;
+#include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+}
+llvm_unreachable("Unkown kind!");
+  }
+
   template const RegionTy* getAs() const;
   template 
   LLVM_ATTRIBUTE_RETURNS_NONNULL const RegionTy *castAs() const;
diff --git a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
index 238e87a712a43..fdea2e01215b4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/AST/OperationKinds.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
@@ -22,10 +23,13 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
+#include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -26,16 +50,12 @@ void top(char *dst) {
 
 void *mempcpy(void *restrict s1, const void *restrict s2, size_t n);
 
-void mempcpy14() {
+void mempcpy13() {

Szelethus wrote:

I gave sensible names to all of these test functions.

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

@@ -393,6 +401,173 @@ ProgramStateRef 
CStringChecker::checkNonNull(CheckerContext ,
   return stateNonNull;
 }
 
+static std::optional getIndex(ProgramStateRef State,
+  const ElementRegion *ER, CharKind CK) {
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  if (CK == CharKind::Regular) {
+if (ER->getValueType() != Ctx.CharTy)
+  return {};
+return ER->getIndex();
+  }
+
+  if (ER->getValueType() != Ctx.WideCharTy)
+return {};
+
+  QualType SizeTy = Ctx.getSizeType();
+  NonLoc WideSize =
+  SValBuilder
+  .makeIntVal(Ctx.getTypeSizeInChars(Ctx.WideCharTy).getQuantity(),
+  SizeTy)
+  .castAs();
+  SVal Offset =
+  SValBuilder.evalBinOpNN(State, BO_Mul, ER->getIndex(), WideSize, SizeTy);
+  if (Offset.isUnknown())
+return {};
+  return Offset.castAs();
+}
+
+// Try to get hold of the origin regin (e.g. the actual array region from an
+// element region).
+static const TypedValueRegion *getOriginRegion(const ElementRegion *ER) {
+  const MemRegion *MR = ER->getSuperRegion();
+  const MemRegion *Ret = MR;
+  assert(MR);
+  if (const auto *sym = MR->getAs()) {
+SymbolRef sym2 = sym->getSymbol();
+if (!sym2)
+  return nullptr;
+Ret = sym2->getOriginRegion();
+  }
+  if (const auto *element = MR->getAs()) {
+Ret = element->getBaseRegion();
+  }
+  return dyn_cast_or_null(Ret);
+}
+
+// Basically 1 -> 1st, 12 -> 12th, etc.
+static void printIdxWithOrdinalSuffix(llvm::raw_ostream , unsigned Idx) {
+  Os << Idx << llvm::getOrdinalSuffix(Idx);
+}
+
+ProgramStateRef CStringChecker::checkInit(CheckerContext ,
+  ProgramStateRef State,
+  AnyArgExpr Buffer, SVal Element,
+  SVal Size) const {
+
+  // If a previous check has failed, propagate the failure.
+  if (!State)
+return nullptr;
+
+  const MemRegion *R = Element.getAsRegion();
+  if (!R)
+return State;
+
+  const auto *ER = dyn_cast(R);
+  if (!ER)
+return State;
+
+  const TypedValueRegion *Orig = getOriginRegion(ER);
+  if (!Orig)
+return State;
+
+  SValBuilder  = State->getStateManager().getSValBuilder();
+  ASTContext  = SValBuilder.getContext();
+
+  // FIXME: We ought to able to check objects as well. Maybe
+  // UninitializedObjectChecker could help?
+  if (!Orig->getValueType()->isArrayType())
+return State;
+
+  const QualType ElemTy = Ctx.getBaseElementType(Orig->getValueType());
+  const NonLoc Zero = SValBuilder.makeZeroArrayIndex();
+
+  SVal FirstElementVal =
+  State->getLValue(ElemTy, Zero, loc::MemRegionVal(Orig)).castAs();
+  if (!isa(FirstElementVal))
+return State;

Szelethus wrote:

Oh ya, this is  a historical artifact. Though I wonder if the cast is a little 
too brave...

https://github.com/llvm/llvm-project/pull/95408
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

@@ -696,6 +730,69 @@ struct StreamOperationEvaluator {
 
 } // end anonymous namespace
 
+//===--===//
+// Definition of NoStreamStateChangeVisitor.
+//===--===//
+
+namespace {
+class NoStreamStateChangeVisitor final : public NoOwnershipChangeVisitor {
+protected:
+  /// Syntactically checks whether the callee is a freeing function. Since
+  /// we have no path-sensitive information on this call (we would need a
+  /// CallEvent instead of a CallExpr for that), its possible that a
+  /// freeing function was called indirectly through a function pointer,
+  /// but we are not able to tell, so this is a best effort analysis.
+  bool isFreeingCallAsWritten(const CallExpr ) const {
+const auto *StreamChk = static_cast();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}

Szelethus wrote:

Showing that a function intended to close a stream already relies on 
heuristics, and its easy to construct dumb counterexamples for it:

```c++
void absolutelyDoesntCloseItsParam(FILE *p) {
  if (coin()) {
FILE *q = fopen(...);
fclose(q); // oh, we saw an fclose, this must've been intended for the 
param...
  }
}
```
We can surely make this smarter, maybe we should, but in finite steps we get to 
the point where we start rewriting the analyzer.

Regarding the CallGraph: if we start to wander of the beaten path (meaning the 
path of execution the analyzer was on), especially if we inspect non-trivial 
functions in the graph, that will lead to a weaker heuristic in my view. 

https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

Szelethus wrote:

> I did not find a similar test for `MallocChecker` but there could be one with 
> similar test functions.

I'm not sure what tests you are referring to. I did fix your other observations.

https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/94957

From faf00d0e1286e053ba9fb457513bd8309eb541ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Fri, 7 Jun 2024 12:07:35 +0200
Subject: [PATCH 1/5] [analyzer] Add an ownership change visitor to
 StreamChecker

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 106 ++-
 clang/test/Analysis/stream-visitor.cpp| 179 ++
 2 files changed, 282 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/stream-visitor.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d4e020f7a72a0..d726ab5eaa599 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,9 @@
 //
 
//===--===//
 
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -74,6 +77,12 @@ struct StreamErrorState {
   /// Returns if the StreamErrorState is a valid object.
   operator bool() const { return NoError || FEof || FError; }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
+os << "NoError: " << NoError << ", FEof: " << FEof
+   << ", FError: " << FError;
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddBoolean(NoError);
 ID.AddBoolean(FEof);
@@ -98,6 +107,19 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
+  StringRef getKindStr() const {
+switch (State) {
+case Opened:
+  return "Opened";
+case Closed:
+  return "Closed";
+case OpenFailed:
+  return "OpenFailed";
+default:
+  llvm_unreachable("Unknown StreamState!");
+}
+  }
+
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
   StreamErrorState const ErrorState;
@@ -146,6 +168,9 @@ struct StreamState {
 return StreamState{L, OpenFailed, {}, false};
   }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
@@ -168,8 +193,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, 
StreamState)
 namespace {
 
 class StreamChecker;
-using FnCheck = std::function;
+using FnCheckTy = void(const StreamChecker *, const FnDescription *,
+   const CallEvent &, CheckerContext &);
+using FnCheck = std::function;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits::max();
@@ -183,6 +209,14 @@ struct FnDescription {
   ArgNoTy StreamArgNo;
 };
 
+LLVM_DUMP_METHOD void StreamState::dumpToStream(llvm::raw_ostream ) const {
+  os << "{Kind: " << getKindStr() << ", Last operation: " << LastOperation
+ << ", ErrorState: ";
+  ErrorState.dumpToStream(os);
+  os << ", FilePos: " << (FilePositionIndeterminate ? "Indeterminate" : "OK")
+ << '}';
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent ) {
@@ -300,6 +334,8 @@ class StreamChecker : public Checker FnDescriptions = {
   {{CDM::CLibrary, {"fopen"}, 2},
@@ -310,7 +346,7 @@ class StreamChecker : public Checker();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might change with an attempt to store stream object, not
+// only through freeing it. Check for attempted 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

@@ -696,6 +730,69 @@ struct StreamOperationEvaluator {
 
 } // end anonymous namespace
 
+//===--===//
+// Definition of NoStreamStateChangeVisitor.
+//===--===//
+
+namespace {
+class NoStreamStateChangeVisitor final : public NoOwnershipChangeVisitor {
+protected:
+  /// Syntactically checks whether the callee is a freeing function. Since
+  /// we have no path-sensitive information on this call (we would need a
+  /// CallEvent instead of a CallExpr for that), its possible that a
+  /// freeing function was called indirectly through a function pointer,
+  /// but we are not able to tell, so this is a best effort analysis.
+  bool isFreeingCallAsWritten(const CallExpr ) const {
+const auto *StreamChk = static_cast();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might change with an attempt to store stream object, not
+// only through freeing it. Check for attempted stores as well.
+return false;
+  }
+
+  virtual bool hasResourceStateChanged(ProgramStateRef CallEnterState,
+   ProgramStateRef CallExitEndState) final 
{
+return CallEnterState->get(Sym) !=
+   CallExitEndState->get(Sym);
+  }
+
+  PathDiagnosticPieceRef emitNote(const ExplodedNode *N) override {
+PathDiagnosticLocation L = PathDiagnosticLocation::create(
+N->getLocation(),
+N->getState()->getStateManager().getContext().getSourceManager());
+return std::make_shared(
+L, "Returning without freeing stream object or storing it for later "
+   "release");
+  }
+
+public:
+  NoStreamStateChangeVisitor(SymbolRef Sym, const StreamChecker *Checker)
+  : NoOwnershipChangeVisitor(Sym, Checker) {}

Szelethus wrote:

It doesn't seem possible, and definitely not worth the trouble.

https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

@@ -696,6 +730,69 @@ struct StreamOperationEvaluator {
 
 } // end anonymous namespace
 
+//===--===//
+// Definition of NoStreamStateChangeVisitor.
+//===--===//
+
+namespace {
+class NoStreamStateChangeVisitor final : public NoOwnershipChangeVisitor {
+protected:
+  /// Syntactically checks whether the callee is a freeing function. Since
+  /// we have no path-sensitive information on this call (we would need a
+  /// CallEvent instead of a CallExpr for that), its possible that a
+  /// freeing function was called indirectly through a function pointer,
+  /// but we are not able to tell, so this is a best effort analysis.
+  bool isFreeingCallAsWritten(const CallExpr ) const {
+const auto *StreamChk = static_cast();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;

Szelethus wrote:

The retionale behind not inlining this is that we could add more functions that 
could close the stream (annotations, maybe). The checker object is owned by the 
base class, so the static cast is necessary.

https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus edited 
https://github.com/llvm/llvm-project/pull/94957
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Check the correct first and last elements in cstring.UninitializedRead (PR #95408)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus created 
https://github.com/llvm/llvm-project/pull/95408

I intend to fix this checker up so that we can move it out of alpha. I made a 
bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the destination and 
source buffers are initialized: heuristically, it only checks the first and 
last element. This is fine, however, it retrieves these elements as characters, 
even if the underlaying object is not a character array. Reading the last byte 
of an integer is undefined, so the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted as 
arrays of unsigned char.". But the static analyzer right now can't check 
byte-by-byte if a memory region is _initialized_, it can only check if its a 
well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy (and 
similar functions), and retrieve the actual first and last elements according 
to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these 
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100


From d717b412749f10b45a9387044e97da6981f3cad4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Thu, 25 Apr 2024 17:31:24 +0200
Subject: [PATCH] [analyzer] Check the correct first and last elements in
 cstring.UninitializedRead

I intend to fix this checker up so that we can move it out of alpha. I
made a bunch of analyses, and found many similar false positives:

```c++
int t[] = {1,2,3};
memcpy(dst, t, sizeof(t) / sizeof(t[0])); // warn
```

The problem here is the way CStringChecker checks whether the
destination and source buffers are initialized: heuristically, it only
checks the first and last element. This is fine, however, it retrieves
these elements as characters, even if the underlaying object is not a
character array. Reading the last byte of an integer is undefined, so
the checker emits a bug here.

A quick search tells you the rationale: "Both objects are reinterpreted
as arrays of unsigned char.". But the static analyzer right now can't
check byte-by-byte if a memory region is _initialized_, it can only
check if its a well-defined character or not.

In this patch, I pry the original array out of the arguments to memcpy
(and similar functions), and retrieve the actual first and last elements
according to the array's actual element type.

Currently, my improvements reduced the number of reports to 29 on these
projects: memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_upper_bound_patched=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Before my patch, there were 87.

https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?detection-status=New=Reopened=Unresolved=on=%2acstring_uninit_baseline=%2acstring_uninit_upper_bounds_patched=New=alpha.unix.cstring.UninitializedRead=100

Change-Id: I0ef16beed735201af857abb6ed02de764af2e78a
---
 .../Core/PathSensitive/MemRegion.h|  14 ++
 .../Checkers/CStringChecker.cpp   | 226 +++---
 clang/test/Analysis/bstring_UninitRead.c  |  66 -
 3 files changed, 258 insertions(+), 48 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
index 151d3e57c1cb8..05d679b659029 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -34,6 +34,7 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -99,6 +100,8 @@ class MemRegion : public llvm::FoldingSetNode {
 #define REGION(Id, Parent) Id ## Kind,
 #define REGION_RANGE(Id, First, Last) BEGIN_##Id = First, END_##Id = Last,
 #include "clang/StaticAnalyzer/Core/PathSensitive/Regions.def"
+#undef REGION
+#undef REGION_RANGE
   };
 
 private:
@@ -171,6 +174,17 @@ class MemRegion : public llvm::FoldingSetNode {
 
   Kind getKind() const { return kind; }
 
+  StringRef getKindStr() const {
+switch 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/94957

From faf00d0e1286e053ba9fb457513bd8309eb541ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Fri, 7 Jun 2024 12:07:35 +0200
Subject: [PATCH 1/4] [analyzer] Add an ownership change visitor to
 StreamChecker

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 106 ++-
 clang/test/Analysis/stream-visitor.cpp| 179 ++
 2 files changed, 282 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/stream-visitor.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d4e020f7a72a0..d726ab5eaa599 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,9 @@
 //
 
//===--===//
 
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -74,6 +77,12 @@ struct StreamErrorState {
   /// Returns if the StreamErrorState is a valid object.
   operator bool() const { return NoError || FEof || FError; }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
+os << "NoError: " << NoError << ", FEof: " << FEof
+   << ", FError: " << FError;
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddBoolean(NoError);
 ID.AddBoolean(FEof);
@@ -98,6 +107,19 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
+  StringRef getKindStr() const {
+switch (State) {
+case Opened:
+  return "Opened";
+case Closed:
+  return "Closed";
+case OpenFailed:
+  return "OpenFailed";
+default:
+  llvm_unreachable("Unknown StreamState!");
+}
+  }
+
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
   StreamErrorState const ErrorState;
@@ -146,6 +168,9 @@ struct StreamState {
 return StreamState{L, OpenFailed, {}, false};
   }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
@@ -168,8 +193,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, 
StreamState)
 namespace {
 
 class StreamChecker;
-using FnCheck = std::function;
+using FnCheckTy = void(const StreamChecker *, const FnDescription *,
+   const CallEvent &, CheckerContext &);
+using FnCheck = std::function;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits::max();
@@ -183,6 +209,14 @@ struct FnDescription {
   ArgNoTy StreamArgNo;
 };
 
+LLVM_DUMP_METHOD void StreamState::dumpToStream(llvm::raw_ostream ) const {
+  os << "{Kind: " << getKindStr() << ", Last operation: " << LastOperation
+ << ", ErrorState: ";
+  ErrorState.dumpToStream(os);
+  os << ", FilePos: " << (FilePositionIndeterminate ? "Indeterminate" : "OK")
+ << '}';
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent ) {
@@ -300,6 +334,8 @@ class StreamChecker : public Checker FnDescriptions = {
   {{CDM::CLibrary, {"fopen"}, 2},
@@ -310,7 +346,7 @@ class StreamChecker : public Checker();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might change with an attempt to store stream object, not
+// only through freeing it. Check for attempted 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/94957

From faf00d0e1286e053ba9fb457513bd8309eb541ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Fri, 7 Jun 2024 12:07:35 +0200
Subject: [PATCH 1/3] [analyzer] Add an ownership change visitor to
 StreamChecker

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 106 ++-
 clang/test/Analysis/stream-visitor.cpp| 179 ++
 2 files changed, 282 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/stream-visitor.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d4e020f7a72a0..d726ab5eaa599 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,9 @@
 //
 
//===--===//
 
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -74,6 +77,12 @@ struct StreamErrorState {
   /// Returns if the StreamErrorState is a valid object.
   operator bool() const { return NoError || FEof || FError; }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
+os << "NoError: " << NoError << ", FEof: " << FEof
+   << ", FError: " << FError;
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddBoolean(NoError);
 ID.AddBoolean(FEof);
@@ -98,6 +107,19 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
+  StringRef getKindStr() const {
+switch (State) {
+case Opened:
+  return "Opened";
+case Closed:
+  return "Closed";
+case OpenFailed:
+  return "OpenFailed";
+default:
+  llvm_unreachable("Unknown StreamState!");
+}
+  }
+
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
   StreamErrorState const ErrorState;
@@ -146,6 +168,9 @@ struct StreamState {
 return StreamState{L, OpenFailed, {}, false};
   }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
@@ -168,8 +193,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, 
StreamState)
 namespace {
 
 class StreamChecker;
-using FnCheck = std::function;
+using FnCheckTy = void(const StreamChecker *, const FnDescription *,
+   const CallEvent &, CheckerContext &);
+using FnCheck = std::function;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits::max();
@@ -183,6 +209,14 @@ struct FnDescription {
   ArgNoTy StreamArgNo;
 };
 
+LLVM_DUMP_METHOD void StreamState::dumpToStream(llvm::raw_ostream ) const {
+  os << "{Kind: " << getKindStr() << ", Last operation: " << LastOperation
+ << ", ErrorState: ";
+  ErrorState.dumpToStream(os);
+  os << ", FilePos: " << (FilePositionIndeterminate ? "Indeterminate" : "OK")
+ << '}';
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent ) {
@@ -300,6 +334,8 @@ class StreamChecker : public Checker FnDescriptions = {
   {{CDM::CLibrary, {"fopen"}, 2},
@@ -310,7 +346,7 @@ class StreamChecker : public Checker();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might change with an attempt to store stream object, not
+// only through freeing it. Check for attempted 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/94957

From faf00d0e1286e053ba9fb457513bd8309eb541ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Fri, 7 Jun 2024 12:07:35 +0200
Subject: [PATCH 1/2] [analyzer] Add an ownership change visitor to
 StreamChecker

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 106 ++-
 clang/test/Analysis/stream-visitor.cpp| 179 ++
 2 files changed, 282 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/stream-visitor.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d4e020f7a72a0..d726ab5eaa599 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,9 @@
 //
 
//===--===//
 
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -74,6 +77,12 @@ struct StreamErrorState {
   /// Returns if the StreamErrorState is a valid object.
   operator bool() const { return NoError || FEof || FError; }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
+os << "NoError: " << NoError << ", FEof: " << FEof
+   << ", FError: " << FError;
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddBoolean(NoError);
 ID.AddBoolean(FEof);
@@ -98,6 +107,19 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
+  StringRef getKindStr() const {
+switch (State) {
+case Opened:
+  return "Opened";
+case Closed:
+  return "Closed";
+case OpenFailed:
+  return "OpenFailed";
+default:
+  llvm_unreachable("Unknown StreamState!");
+}
+  }
+
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
   StreamErrorState const ErrorState;
@@ -146,6 +168,9 @@ struct StreamState {
 return StreamState{L, OpenFailed, {}, false};
   }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
@@ -168,8 +193,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, 
StreamState)
 namespace {
 
 class StreamChecker;
-using FnCheck = std::function;
+using FnCheckTy = void(const StreamChecker *, const FnDescription *,
+   const CallEvent &, CheckerContext &);
+using FnCheck = std::function;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits::max();
@@ -183,6 +209,14 @@ struct FnDescription {
   ArgNoTy StreamArgNo;
 };
 
+LLVM_DUMP_METHOD void StreamState::dumpToStream(llvm::raw_ostream ) const {
+  os << "{Kind: " << getKindStr() << ", Last operation: " << LastOperation
+ << ", ErrorState: ";
+  ErrorState.dumpToStream(os);
+  os << ", FilePos: " << (FilePositionIndeterminate ? "Indeterminate" : "OK")
+ << '}';
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent ) {
@@ -300,6 +334,8 @@ class StreamChecker : public Checker FnDescriptions = {
   {{CDM::CLibrary, {"fopen"}, 2},
@@ -310,7 +346,7 @@ class StreamChecker : public Checker();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+
+auto Matches =
+match(findAll(callExpr().bind("call")), *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might change with an attempt to store stream object, not
+// only through freeing it. Check for attempted 

[clang] [analyzer] Add an ownership change visitor to StreamChecker (PR #94957)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus created 
https://github.com/llvm/llvm-project/pull/94957

This is very similar to https://reviews.llvm.org/D105553, in fact, I barely 
made any changes from MallocChecker's ownership visitor to this one.

The new visitor emits a diagnostic note for function where a change in stream 
ownership was expected (for example, it had a fclose() call), but the ownership 
remained unchanged. This is similar to messages regarding ordinary values 
("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946

From faf00d0e1286e053ba9fb457513bd8309eb541ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Fri, 7 Jun 2024 12:07:35 +0200
Subject: [PATCH] [analyzer] Add an ownership change visitor to StreamChecker

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

Change-Id: I7621c178dd35713d860d27bfc644fb56a42b0946
---
 .../StaticAnalyzer/Checkers/StreamChecker.cpp | 106 ++-
 clang/test/Analysis/stream-visitor.cpp| 179 ++
 2 files changed, 282 insertions(+), 3 deletions(-)
 create mode 100644 clang/test/Analysis/stream-visitor.cpp

diff --git a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
index d4e020f7a72a0..d726ab5eaa599 100644
--- a/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,9 @@
 //
 
//===--===//
 
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -74,6 +77,12 @@ struct StreamErrorState {
   /// Returns if the StreamErrorState is a valid object.
   operator bool() const { return NoError || FEof || FError; }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
+os << "NoError: " << NoError << ", FEof: " << FEof
+   << ", FError: " << FError;
+  }
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddBoolean(NoError);
 ID.AddBoolean(FEof);
@@ -98,6 +107,19 @@ struct StreamState {
 OpenFailed /// The last open operation has failed.
   } State;
 
+  StringRef getKindStr() const {
+switch (State) {
+case Opened:
+  return "Opened";
+case Closed:
+  return "Closed";
+case OpenFailed:
+  return "OpenFailed";
+default:
+  llvm_unreachable("Unknown StreamState!");
+}
+  }
+
   /// State of the error flags.
   /// Ignored in non-opened stream state but must be NoError.
   StreamErrorState const ErrorState;
@@ -146,6 +168,9 @@ struct StreamState {
 return StreamState{L, OpenFailed, {}, false};
   }
 
+  LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const;
+
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddPointer(LastOperation);
 ID.AddInteger(State);
@@ -168,8 +193,9 @@ REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, 
StreamState)
 namespace {
 
 class StreamChecker;
-using FnCheck = std::function;
+using FnCheckTy = void(const StreamChecker *, const FnDescription *,
+   const CallEvent &, CheckerContext &);
+using FnCheck = std::function;
 
 using ArgNoTy = unsigned int;
 static const ArgNoTy ArgNone = std::numeric_limits::max();
@@ -183,6 +209,14 @@ struct FnDescription {
   ArgNoTy StreamArgNo;
 };
 
+LLVM_DUMP_METHOD void StreamState::dumpToStream(llvm::raw_ostream ) const {
+  os << "{Kind: " << getKindStr() << ", Last operation: " << LastOperation
+ << ", ErrorState: ";
+  ErrorState.dumpToStream(os);
+  os << ", FilePos: " << (FilePositionIndeterminate ? "Indeterminate" : "OK")
+ << '}';
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent ) {
@@ -300,6 +334,8 @@ class StreamChecker : public Checker FnDescriptions = {
   {{CDM::CLibrary, {"fopen"}, 2},
@@ -310,7 +346,7 @@ class StreamChecker : public Checker();
+if (StreamChk->FCloseDesc.matchesAsWritten(Call))
+  return true;
+
+return false;
+  }
+
+  bool doesFnIntendToHandleOwnership(const Decl *Callee,
+ ASTContext ) override {
+using 

[clang] [analyzer][NFC] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

@@ -0,0 +1,116 @@
+//===--*- C++ 
-*--//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "NoOwnershipChangeVisitor.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "llvm/ADT/SetOperations.h"
+
+using namespace clang;
+using namespace ento;
+using OwnerSet = NoOwnershipChangeVisitor::OwnerSet;
+
+// Collect which entities point to the allocated memory, and could be
+// responsible for deallocating it.
+class OwnershipBindingsHandler : public StoreManager::BindingsHandler {

Szelethus wrote:

Good catch, I'll fix that in a followup patch.

https://github.com/llvm/llvm-project/pull/94357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus closed 
https://github.com/llvm/llvm-project/pull/94357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus updated 
https://github.com/llvm/llvm-project/pull/94357

From b6beb7098bb8e5148fe0467dc976506ff6691f15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Tue, 4 Jun 2024 16:15:42 +0200
Subject: [PATCH 1/2] [analyzer] Factor out NoOwnershipChangeVisitor

In preparation for adding essentially the same visitor to StreamChecker,
this patch factors this visitor out to a common header.

Its true that for the time being NoOwnershipChangeVisitor and
NoStateChangeVisitor play a very similar role, so it'd be reasonable
desire to merge these. My argument against that is that if we check for
a property change that is not a resource this distinction could still be
useful.

Change-Id: I99d73ccd93a18dd145bbbc83afadbb432dd42b90
---
 .../StaticAnalyzer/Checkers/CMakeLists.txt|   1 +
 .../StaticAnalyzer/Checkers/MallocChecker.cpp | 147 +++---
 .../Checkers/NoOwnershipChangeVisitor.cpp | 116 ++
 .../Checkers/NoOwnershipChangeVisitor.h   |  78 ++
 4 files changed, 214 insertions(+), 128 deletions(-)
 create mode 100644 
clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
 create mode 100644 clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h

diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4443ffd092938..2520119bffcdf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -78,6 +78,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   NoReturnFunctionChecker.cpp
   NonNullParamChecker.cpp
   NonnullGlobalConstantsChecker.cpp
+  NoOwnershipChangeVisitor.cpp
   NullabilityChecker.cpp
   NumberObjectConversionChecker.cpp
   ObjCAtSyncChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 79ab05f2c7866..41dd9cf3e6831 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -46,6 +46,7 @@
 
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
+#include "NoOwnershipChangeVisitor.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
@@ -78,13 +79,11 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
-#include 
 #include 
 #include 
 #include 
@@ -401,7 +400,7 @@ class MallocChecker
   bool isFreeingCall(const CallEvent ) const;
   static bool isFreeingOwnershipAttrCall(const FunctionDecl *Func);
 
-  friend class NoOwnershipChangeVisitor;
+  friend class NoMemOwnershipChangeVisitor;
 
   CallDescriptionMap AllocatingMemFnMap{
   {{{"alloca"}, 1}, ::checkAlloca},
@@ -730,60 +729,8 @@ class MallocChecker
 
//===--===//
 
 namespace {
-class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
-  // The symbol whose (lack of) ownership change we are interested in.
-  SymbolRef Sym;
-  const MallocChecker 
-  using OwnerSet = llvm::SmallPtrSet;
-
-  // Collect which entities point to the allocated memory, and could be
-  // responsible for deallocating it.
-  class OwnershipBindingsHandler : public StoreManager::BindingsHandler {
-SymbolRef Sym;
-OwnerSet 
-
-  public:
-OwnershipBindingsHandler(SymbolRef Sym, OwnerSet )
-: Sym(Sym), Owners(Owners) {}
-
-bool HandleBinding(StoreManager , Store Store, const MemRegion 
*Region,
-   SVal Val) override {
-  if (Val.getAsSymbol() == Sym)
-Owners.insert(Region);
-  return true;
-}
-
-LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
-LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
-  out << "Owners: {\n";
-  for (const MemRegion *Owner : Owners) {
-out << "  ";
-Owner->dumpToStream(out);
-out << ",\n";
-  }
-  out << "}\n";
-}
-  };
-
+class NoMemOwnershipChangeVisitor final : public NoOwnershipChangeVisitor {
 protected:
-  OwnerSet getOwnersAtNode(const ExplodedNode *N) {
-OwnerSet Ret;
-
-ProgramStateRef State = N->getState();
-OwnershipBindingsHandler Handler{Sym, Ret};
-State->getStateManager().getStoreManager().iterBindings(State->getStore(),
-Handler);
-return Ret;
-  }
-
-  LLVM_DUMP_METHOD static std::string
-  getFunctionName(const ExplodedNode *CallEnterN) {
-if (const CallExpr *CE = llvm::dyn_cast_or_null(
-CallEnterN->getLocationAs()->getCallExpr()))
-  if (const FunctionDecl *FD = 

[clang] [analyzer][NFC] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus edited 
https://github.com/llvm/llvm-project/pull/94357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

Szelethus wrote:

Yes, it is, sorry about that :) 

https://github.com/llvm/llvm-project/pull/94357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Factor out NoOwnershipChangeVisitor (PR #94357)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus created 
https://github.com/llvm/llvm-project/pull/94357

In preparation for adding essentially the same visitor to StreamChecker, this 
patch factors this visitor out to a common header.

I'll be the first to admit that the interface of these classes are not 
terrific, but it rather tightly held back by its main technical debt, which is 
NoStoreFuncVisitor, the main descendant of NoStateChangeVisitor.

Change-Id: I99d73ccd93a18dd145bbbc83afadbb432dd42b90

From b6beb7098bb8e5148fe0467dc976506ff6691f15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krist=C3=B3f=20Umann?= 
Date: Tue, 4 Jun 2024 16:15:42 +0200
Subject: [PATCH] [analyzer] Factor out NoOwnershipChangeVisitor

In preparation for adding essentially the same visitor to StreamChecker,
this patch factors this visitor out to a common header.

Its true that for the time being NoOwnershipChangeVisitor and
NoStateChangeVisitor play a very similar role, so it'd be reasonable
desire to merge these. My argument against that is that if we check for
a property change that is not a resource this distinction could still be
useful.

Change-Id: I99d73ccd93a18dd145bbbc83afadbb432dd42b90
---
 .../StaticAnalyzer/Checkers/CMakeLists.txt|   1 +
 .../StaticAnalyzer/Checkers/MallocChecker.cpp | 147 +++---
 .../Checkers/NoOwnershipChangeVisitor.cpp | 116 ++
 .../Checkers/NoOwnershipChangeVisitor.h   |  78 ++
 4 files changed, 214 insertions(+), 128 deletions(-)
 create mode 100644 
clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.cpp
 create mode 100644 clang/lib/StaticAnalyzer/Checkers/NoOwnershipChangeVisitor.h

diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt 
b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4443ffd092938..2520119bffcdf 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -78,6 +78,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   NoReturnFunctionChecker.cpp
   NonNullParamChecker.cpp
   NonnullGlobalConstantsChecker.cpp
+  NoOwnershipChangeVisitor.cpp
   NullabilityChecker.cpp
   NumberObjectConversionChecker.cpp
   ObjCAtSyncChecker.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 79ab05f2c7866..41dd9cf3e6831 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -46,6 +46,7 @@
 
 #include "AllocationState.h"
 #include "InterCheckerAPI.h"
+#include "NoOwnershipChangeVisitor.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
@@ -78,13 +79,11 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SetOperations.h"
-#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
-#include 
 #include 
 #include 
 #include 
@@ -401,7 +400,7 @@ class MallocChecker
   bool isFreeingCall(const CallEvent ) const;
   static bool isFreeingOwnershipAttrCall(const FunctionDecl *Func);
 
-  friend class NoOwnershipChangeVisitor;
+  friend class NoMemOwnershipChangeVisitor;
 
   CallDescriptionMap AllocatingMemFnMap{
   {{{"alloca"}, 1}, ::checkAlloca},
@@ -730,60 +729,8 @@ class MallocChecker
 
//===--===//
 
 namespace {
-class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
-  // The symbol whose (lack of) ownership change we are interested in.
-  SymbolRef Sym;
-  const MallocChecker 
-  using OwnerSet = llvm::SmallPtrSet;
-
-  // Collect which entities point to the allocated memory, and could be
-  // responsible for deallocating it.
-  class OwnershipBindingsHandler : public StoreManager::BindingsHandler {
-SymbolRef Sym;
-OwnerSet 
-
-  public:
-OwnershipBindingsHandler(SymbolRef Sym, OwnerSet )
-: Sym(Sym), Owners(Owners) {}
-
-bool HandleBinding(StoreManager , Store Store, const MemRegion 
*Region,
-   SVal Val) override {
-  if (Val.getAsSymbol() == Sym)
-Owners.insert(Region);
-  return true;
-}
-
-LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
-LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ) const {
-  out << "Owners: {\n";
-  for (const MemRegion *Owner : Owners) {
-out << "  ";
-Owner->dumpToStream(out);
-out << ",\n";
-  }
-  out << "}\n";
-}
-  };
-
+class NoMemOwnershipChangeVisitor final : public NoOwnershipChangeVisitor {
 protected:
-  OwnerSet getOwnersAtNode(const ExplodedNode *N) {
-OwnerSet Ret;
-
-ProgramStateRef State = N->getState();
-OwnershipBindingsHandler Handler{Sym, Ret};
-

[clang] [analyzer] Use CDM::CLibrary instead of isGlobalCFunction() (PR #88267)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus approved this pull request.

Seems like a very straightforward followup to the existing patches.

https://github.com/llvm/llvm-project/pull/88267
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" again (PR #85791)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus approved this pull request.

Lets hope it works fine this time around :)

https://github.com/llvm/llvm-project/pull/85791
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [llvm] Reapply "[analyzer] Accept C library functions from the `std` namespace" (#84926) (PR #84963)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/84963
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus edited 
https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)

[Kristóf Umann via cfe-commits]

@@ -41,12 +41,8 @@ class CallDescription {
 ///  - We also accept calls where the number of arguments or parameters is
 ///greater than the specified value.
 /// For the exact heuristics, see CheckerContext::isCLibraryFunction().
-/// Note that functions whose declaration context is not a TU (e.g.
-/// methods, functions in namespaces) are not accepted as C library
-/// functions.
-/// FIXME: If I understand it correctly, this discards calls where C++ code
-/// refers a C library function through the namespace `std::` via headers
-/// like .
+/// (This mode only matches functions that are declared either directly
+/// within a TU or in the namespace `std`.)

Szelethus wrote:

Might as well go the extra mile and mention an example, like `std::malloc`.

https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)

[Kristóf Umann via cfe-commits]

@@ -87,9 +87,11 @@ bool CheckerContext::isCLibraryFunction(const FunctionDecl 
*FD,
   if (!II)
 return false;
 
-  // Look through 'extern "C"' and anything similar invented in the future.
-  // If this function is not in TU directly, it is not a C library function.
-  if (!FD->getDeclContext()->getRedeclContext()->isTranslationUnit())
+  // C library functions are either declared directly within a TU (the common
+  // case) or they are accessed through the namespace `std` (when they are used
+  // in C++ via headers like ).

Szelethus wrote:

Same here.

https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Accept C library functions from the `std` namespace (PR #84469)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus commented:

Makes perfect sense to me. Can you add a testcase for `std::malloc` or 
something similar?

https://github.com/llvm/llvm-project/pull/84469
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Add std::variant checker (PR #66481)

[Kristóf Umann via cfe-commits]

https://github.com/Szelethus edited 
https://github.com/llvm/llvm-project/pull/66481
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a504ddc - [analyzer] Initialize regions returned by CXXNew to undefined

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-10-26T17:22:12+02:00
New Revision: a504ddc8bf9d5c406ea88b84b8495d7aae200d4c

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

LOG: [analyzer] Initialize regions returned by CXXNew to undefined

Discourse mail:
https://discourse.llvm.org/t/analyzer-why-do-we-suck-at-modeling-c-dynamic-memory/65667

malloc() returns a piece of uninitialized dynamic memory. We were (almost)
always able to model this behaviour. Its C++ counterpart, operator new is a
lot more complex, because it allows for initialization, the most complicated of 
which is the usage of constructors.

We gradually became better in modeling constructors, but for some reason, most
likely for reasons lost in history, we never actually modeled the case when the
memory returned by operator new was just simply uninitialized. This patch
(attempts) to fix this tiny little error.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/cxx-member-initializer-const-field.cpp
clang/test/Analysis/new-ctor-conservative.cpp
clang/test/Analysis/new-ctor-recursive.cpp
clang/test/Analysis/new.cpp
clang/test/Analysis/placement-new.cpp
clang/test/Analysis/reinterpret-cast.cpp
clang/test/Analysis/uninit-const.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 476afc598ac6c..38c8896b10ba7 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -10,15 +10,16 @@
 //
 
//===--===//
 
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/Analysis/ConstructionContext.h"
 #include "clang/AST/DeclCXX.h"
-#include "clang/AST/StmtCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/AST/StmtCXX.h"
+#include "clang/Analysis/ConstructionContext.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 
 using namespace clang;
 using namespace ento;
@@ -953,6 +954,11 @@ void ExprEngine::VisitCXXNewAllocatorCall(const CXXNewExpr 
*CNE,
 // skip it for now.
 ProgramStateRef State = I->getState();
 SVal RetVal = State->getSVal(CNE, LCtx);
+// [basic.stc.dynamic.allocation] (on the return value of an allocation
+// function):
+// "The order, contiguity, and initial value of storage allocated by
+// successive calls to an allocation function are unspecified."
+State = State->bindDefaultInitial(RetVal, UndefinedVal{}, LCtx);
 
 // If this allocation function is not declared as non-throwing, failures
 // /must/ be signalled by exceptions, and thus the return value will never

diff  --git a/clang/test/Analysis/NewDelete-checker-test.cpp 
b/clang/test/Analysis/NewDelete-checker-test.cpp
index 5a06c8327f717..1100b49e84d3a 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -385,7 +385,11 @@ class DerefClass{
 public:
   int *x;
   DerefClass() {}
-  ~DerefClass() {*x = 1;}
+  ~DerefClass() {
+int i = 0;
+x = 
+*x = 1;
+  }
 };
 
 void testDoubleDeleteClassInstance() {

diff  --git a/clang/test/Analysis/cxx-member-initializer-const-field.cpp 
b/clang/test/Analysis/cxx-member-initializer-const-field.cpp
index f0abbddbc4441..98076124d82b7 100644
--- a/clang/test/Analysis/cxx-member-initializer-const-field.cpp
+++ b/clang/test/Analysis/cxx-member-initializer-const-field.cpp
@@ -10,7 +10,7 @@ struct WithConstructor {
   WithConstructor(int *x) : ptr(x) {}
 
   static auto compliant() {
-WithConstructor c(new int);
+WithConstructor c(new int{});
 return *(c.ptr); // no warning
   }
 
@@ -28,7 +28,7 @@ struct RegularAggregate {
   int *const ptr = nullptr;
 
   static int compliant() {
-RegularAggregate c{new int};
+RegularAggregate c{new int{}};
 return *(c.ptr); // no warning
   }
 

diff  --git a/clang/test/Analysis/new-ctor-conservative.cpp 
b/clang/test/Analysis/new-ctor-conservative.cpp
index bcc33fb8f8970..1fcb6708e43e1 100644
--- a/clang/test/Analysis/new-ctor-conservative.cpp
+++ b/clang/test/Analysis/new-ctor-conservative.cpp
@@ -14,9 +14,12 @@ void checkConstructorInlining() {
   clang_analyzer_eval(s->x == 1); // expected-warning{{TRUE}}
 }
 
-void checkNewPOD() {
+void 

[clang] fd8e576 - [analyzer] Don't track function calls as control dependencies

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-04-08T10:16:58+02:00
New Revision: fd8e5762f86f0a602ec08eea5c4c86927faba6dc

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

LOG: [analyzer] Don't track function calls as control dependencies

I recently evaluated ~150 of bug reports on open source projects relating to my
GSoC'19 project, which was about tracking control dependencies that were
relevant to a bug report.

Here is what I found: when the condition is a function call, the extra notes
were almost always unimportant, and often times intrusive:

void f(int *x) {
  x = nullptr;
  if (alwaysTrue()) // We don't need a whole lot of explanation
// here, the function name is good enough.
*x = 5;
}
It almost always boiled down to a few "Returning null pointer, which 
participates
in a condition later", or similar notes. I struggled to find a single case
where the notes revealed anything interesting or some previously hidden
correlation, which is kind of the point of condition tracking.

This patch checks whether the condition is a function call, and if so, bails
out.

The argument against the patch is the popular feedback we hear from some of our
users, namely that they can never have too much information. I was specifically
fishing for examples that display best that my contribution did more good than
harm, so admittedly I set the bar high, and one can argue that there can be
non-trivial trickery inside functions, and function names may not be that
descriptive.

My argument for the patch is all those reports that got longer without any
notable improvement in the report intelligibility. I think the few exceptional
cases where this patch would remove notable information are an acceptable
sacrifice in favor of more reports being leaner.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index e13387fb1fc8e..11e5ff8c579a1 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -82,6 +82,10 @@ static const Expr *peelOffPointerArithmetic(const 
BinaryOperator *B) {
   return nullptr;
 }
 
+/// \return A subexpression of @c Ex which represents the
+/// expression-of-interest.
+static const Expr *peelOffOuterExpr(const Expr *Ex, const ExplodedNode *N);
+
 /// Given that expression S represents a pointer that would be dereferenced,
 /// try to find a sub-expression from which the pointer came from.
 /// This is used for tracking down origins of a null or undefined value:
@@ -1919,11 +1923,33 @@ TrackControlDependencyCondBRVisitor::VisitNode(const 
ExplodedNode *N,
   return nullptr;
 
 if (const Expr *Condition = NB->getLastCondition()) {
+
+  // If we can't retrieve a sensible condition, just bail out.
+  const Expr *InnerExpr = peelOffOuterExpr(Condition, N);
+  if (!InnerExpr)
+return nullptr;
+
+  // If the condition was a function call, we likely won't gain much from
+  // tracking it either. Evidence suggests that it will mostly trigger in
+  // scenarios like this:
+  //
+  //   void f(int *x) {
+  // x = nullptr;
+  // if (alwaysTrue()) // We don't need a whole lot of explanation
+  //   // here, the function name is good enough.
+  //   *x = 5;
+  //   }
+  //
+  // Its easy to create a counterexample where this heuristic would make us
+  // lose valuable information, but we've never really seen one in 
practice.
+  if (isa(InnerExpr))
+return nullptr;
+
   // Keeping track of the already tracked conditions on a visitor level
   // isn't sufficient, because a new visitor is created for each tracked
   // expression, hence the BugReport level set.
   if (BR.addTrackedCondition(N)) {
-getParentTracker().track(Condition, N,
+getParentTracker().track(InnerExpr, N,
  {bugreporter::TrackingKind::Condition,
   /*EnableNullFPSuppression=*/false});
 return constructDebugPieceForTrackedCondition(Condition, N, BRC);
@@ -1938,10 +1964,8 @@ TrackControlDependencyCondBRVisitor::VisitNode(const 
ExplodedNode *N,
 // Implementation of trackExpressionValue.
 
//===--===//
 
-/// \return A subexpression of @c Ex which represents the
-/// expression-of-interest.
-static const Expr 

[clang] d832078 - [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-03-03T11:27:56+01:00
New Revision: d832078904c6e1d26648236b9f724f699dafb201

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

LOG: [analyzer] Improve NoOwnershipChangeVisitor's understanding of deallocators

The problem with leak bug reports is that the most interesting event in the code
is likely the one that did not happen -- lack of ownership change and lack of
deallocation, which is often present within the same function that the analyzer
inlined anyway, but not on the path of execution on which the bug occured. We
struggle to understand that a function was responsible for freeing the memory,
but failed.

D105819 added a new visitor to improve memory leak bug reports. In addition to
inspecting the ExplodedNodes of the bug pat, the visitor tries to guess whether
the function was supposed to free memory, but failed to. Initially (in D108753),
this was done by checking whether a CXXDeleteExpr is present in the function. If
so, we assume that the function was at least party responsible, and prevent the
analyzer from pruning bug report notes in it. This patch improves this heuristic
by recognizing all deallocator functions that MallocChecker itself recognizes,
by reusing MallocChecker::isFreeingCall.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 63194d69d6363..238022d1253b9 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -398,6 +398,9 @@ class MallocChecker
   };
 
   bool isFreeingCall(const CallEvent ) const;
+  static bool isFreeingOwnershipAttrCall(const FunctionDecl *Func);
+
+  friend class NoOwnershipChangeVisitor;
 
   CallDescriptionMap AllocatingMemFnMap{
   {{"alloca", 1}, ::checkAlloca},
@@ -742,7 +745,9 @@ class MallocChecker
 
 namespace {
 class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
+  // The symbol whose (lack of) ownership change we are interested in.
   SymbolRef Sym;
+  const MallocChecker 
   using OwnerSet = llvm::SmallPtrSet;
 
   // Collect which entities point to the allocated memory, and could be
@@ -794,6 +799,29 @@ class NoOwnershipChangeVisitor final : public 
NoStateChangeFuncVisitor {
 return "";
   }
 
+  /// Syntactically checks whether the callee is a deallocating function. Since
+  /// we have no path-sensitive information on this call (we would need a
+  /// CallEvent instead of a CallExpr for that), its possible that a
+  /// deallocation function was called indirectly through a function pointer,
+  /// but we are not able to tell, so this is a best effort analysis.
+  /// See namespace `memory_passed_to_fn_call_free_through_fn_ptr` in
+  /// clang/test/Analysis/NewDeleteLeaks.cpp.
+  bool isFreeingCallAsWritten(const CallExpr ) const {
+if (Checker.FreeingMemFnMap.lookupAsWritten(Call) ||
+Checker.ReallocatingMemFnMap.lookupAsWritten(Call))
+  return true;
+
+if (const auto *Func =
+llvm::dyn_cast_or_null(Call.getCalleeDecl()))
+  return MallocChecker::isFreeingOwnershipAttrCall(Func);
+
+return false;
+  }
+
+  /// Heuristically guess whether the callee intended to free memory. This is
+  /// done syntactically, because we are trying to argue about alternative
+  /// paths of execution, and as a consequence we don't have path-sensitive
+  /// information.
   bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext ) {
 using namespace clang::ast_matchers;
 const FunctionDecl *FD = dyn_cast(Callee);
@@ -807,13 +835,21 @@ class NoOwnershipChangeVisitor final : public 
NoStateChangeFuncVisitor {
 if (!FD || !FD->hasBody())
   return false;
 
-// TODO: Operator delete is hardly the only deallocator -- Can we reuse
-// isFreeingCall() or something thats already here?
-auto Deallocations = match(
-stmt(hasDescendant(cxxDeleteExpr().bind("delete"))
- ), *FD->getBody(), ACtx);
-// TODO: Ownership my change with an attempt to store the allocated memory.
-return !Deallocations.empty();
+auto Matches = match(findAll(stmt(anyOf(cxxDeleteExpr().bind("delete"),
+callExpr().bind("call",
+ *FD->getBody(), ACtx);
+for (BoundNodes Match : Matches) {
+  if (Match.getNodeAs("delete"))
+return true;
+
+  if (const auto *Call = Match.getNodeAs("call"))
+if (isFreeingCallAsWritten(*Call))
+  return true;
+}
+// TODO: Ownership might 

[clang] 32ac21d - [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-03-01T17:13:04+01:00
New Revision: 32ac21d04909da0d50d3b24100d5d9ab30b29a95

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

LOG: [NFC][analyzer] Allow CallDescriptions to be matched with CallExprs

Since CallDescriptions can only be matched against CallEvents that are created
during symbolic execution, it was not possible to use it in syntactic-only
contexts. For example, even though InnerPointerChecker can check with its set of
CallDescriptions whether a function call is interested during analysis, its
unable to check without hassle whether a non-analyzer piece of code also calls
such a function.

The patch adds the ability to use CallDescriptions in syntactic contexts as
well. While we already have that in Signature, we still want to leverage the
ability to use dynamic information when we have it (function pointers, for
example). This could be done with Signature as well (StdLibraryFunctionsChecker
does it), but it makes it even less of a drop-in replacement.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
clang/lib/StaticAnalyzer/Core/CallDescription.cpp
clang/unittests/StaticAnalyzer/CallDescriptionTest.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
index cd972b7837d00..e552b833b6f25 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
@@ -108,13 +108,56 @@ class CallDescription {
 return CD1.matches(Call);
   }
 
-  /// \copydoc clang::ento::matchesAny(const CallEvent &, const 
CallDescription &)
+  /// \copydoc clang::ento::CallDescription::matchesAny(const CallEvent &, 
const CallDescription &)
   template 
   friend bool matchesAny(const CallEvent , const CallDescription ,
  const Ts &...CDs) {
 return CD1.matches(Call) || matchesAny(Call, CDs...);
   }
   /// @}
+
+  /// @name Matching CallDescriptions against a CallExpr
+  /// @{
+
+  /// Returns true if the CallExpr is a call to a function that matches the
+  /// CallDescription.
+  ///
+  /// When available, always prefer matching with a CallEvent! This function
+  /// exists only when that is not available, for example, when _only_
+  /// syntactic check is done on a piece of code.
+  ///
+  /// Also, StdLibraryFunctionsChecker::Signature is likely a better candicade
+  /// for syntactic only matching if you are writing a new checker. This is
+  /// handy if a CallDescriptionMap is already there.
+  ///
+  /// The function is imprecise because CallEvent may know path sensitive
+  /// information, such as the precise argument count (see comments for
+  /// CallEvent::getNumArgs), the called function if it was called through a
+  /// function pointer, and other information not available syntactically.
+  bool matchesAsWritten(const CallExpr ) const;
+
+  /// Returns true whether the CallExpr matches on any of the CallDescriptions
+  /// supplied.
+  ///
+  /// \note This function is not intended to be used to match Obj-C method
+  /// calls.
+  friend bool matchesAnyAsWritten(const CallExpr ,
+  const CallDescription ) {
+return CD1.matchesAsWritten(CE);
+  }
+
+  /// \copydoc clang::ento::CallDescription::matchesAnyAsWritten(const 
CallExpr &, const CallDescription &)
+  template 
+  friend bool matchesAnyAsWritten(const CallExpr ,
+  const CallDescription ,
+  const Ts &...CDs) {
+return CD1.matchesAsWritten(CE) || matchesAnyAsWritten(CE, CDs...);
+  }
+  /// @}
+
+private:
+  bool matchesImpl(const FunctionDecl *Callee, size_t ArgCount,
+   size_t ParamCount) const;
 };
 
 /// An immutable map from CallDescriptions to arbitrary data. Provides a 
unified
@@ -156,6 +199,28 @@ template  class CallDescriptionMap {
 
 return nullptr;
   }
+
+  /// When available, always prefer lookup with a CallEvent! This function
+  /// exists only when that is not available, for example, when _only_
+  /// syntactic check is done on a piece of code.
+  ///
+  /// Also, StdLibraryFunctionsChecker::Signature is likely a better candicade
+  /// for syntactic only matching if you are writing a new checker. This is
+  /// handy if a CallDescriptionMap is already there.
+  ///
+  /// The function is imprecise because CallEvent may know path sensitive
+  /// information, such as the precise argument count (see comments for
+  /// CallEvent::getNumArgs), the called function if it was called through a
+  

[clang] 5048a58 - [analyzer] Don't crash if the analyzer-constraint is set to Z3, but llvm is not built with it

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-02-25T17:51:37+01:00
New Revision: 5048a58a6792ee7432b749a48c937cc9b6a9dc93

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

LOG: [analyzer] Don't crash if the analyzer-constraint is set to Z3, but llvm 
is not built with it

Exactly what it says on the tin! We had a nasty crash with the following 
incovation:

$ clang --analyze -Xclang -analyzer-constraints=z3 test.c
fatal error: error in backend: LLVM was not compiled with Z3 support, rebuild 
with -DLLVM_ENABLE_Z3_SOLVER=ON
...  ...

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

Added: 
clang/test/Analysis/missing-z3-nocrash.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/lit.cfg.py

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b608b8ec50682..1ac5bf844efe6 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -438,6 +438,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_not_built_with_z3 : Error<
+  "analyzer constraint manager 'z3' is only available if LLVM was built with "
+  "-DLLVM_ENABLE_Z3_SOLVER=ON">;
 
 def warn_drv_needs_hvx : Warning<
   "%0 requires HVX, use -mhvx/-mhvx= to enable it">,

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 1014147451091..5d7c999b0143a 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -915,6 +915,11 @@ static bool ParseAnalyzerArgs(AnalyzerOptions , 
ArgList ,
   Diags.Report(diag::err_drv_invalid_value)
 << A->getAsString(Args) << Name;
 } else {
+#ifndef LLVM_WITH_Z3
+  if (Value == AnalysisConstraints::Z3ConstraintsModel) {
+Diags.Report(diag::err_analyzer_not_built_with_z3);
+  }
+#endif // LLVM_WITH_Z3
   Opts.AnalysisConstraintsOpt = Value;
 }
   }

diff  --git a/clang/test/Analysis/missing-z3-nocrash.c 
b/clang/test/Analysis/missing-z3-nocrash.c
new file mode 100644
index 0..698430fffe1a1
--- /dev/null
+++ b/clang/test/Analysis/missing-z3-nocrash.c
@@ -0,0 +1,5 @@
+// RUN: not %clang_analyze_cc1 -analyzer-constraints=z3 %s 2>&1 | FileCheck %s
+// REQUIRES: no-z3
+
+// CHECK: error: analyzer constraint manager 'z3' is only available if LLVM
+// CHECK: was built with -DLLVM_ENABLE_Z3_SOLVER=ON

diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index a8efe6c593e98..31425e4009fe2 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -100,6 +100,8 @@ def have_host_jit_support():
 
 if config.clang_staticanalyzer_z3:
 config.available_features.add('z3')
+else:
+config.available_features.add('no-z3')
 
 check_analyzer_fixit_path = os.path.join(
 config.test_source_root, "Analysis", "check-analyzer-fixit.py")



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

[clang] 3ad35ba - [Templight] Don't display empty strings for names of unnamed template parameters

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2022-01-24T16:37:11+01:00
New Revision: 3ad35ba4dea5240dd58476f0c85f0fe096d6c7ce

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

LOG: [Templight] Don't display empty strings for names of unnamed template 
parameters

Patch originally by oktal3000: 
https://github.com/mikael-s-persson/templight/pull/40

When a template parameter is unnamed, the name of -templight-dump might return
an empty string. This is fine, they are unnamed after all, but it might be more
user friendly to at least describe what entity is unnamed.

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

Added: 
clang/test/Templight/templight-empty-entries-fix.cpp

Modified: 
clang/lib/Frontend/FrontendActions.cpp

Removed: 




diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index 5b77c3e01aace..ad2e6039477f8 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -8,9 +8,10 @@
 
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/FileManager.h"
-#include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -23,6 +24,7 @@
 #include "clang/Sema/TemplateInstCallback.h"
 #include "clang/Serialization/ASTReader.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -480,25 +482,92 @@ class DefaultTemplateInstCallback : public 
TemplateInstantiationCallback {
 Out << "---" << YAML << "\n";
   }
 
+  static void printEntryName(const Sema , const Decl *Entity,
+ llvm::raw_string_ostream ) {
+auto *NamedTemplate = cast(Entity);
+
+PrintingPolicy Policy = TheSema.Context.getPrintingPolicy();
+// FIXME: Also ask for FullyQualifiedNames?
+Policy.SuppressDefaultTemplateArgs = false;
+NamedTemplate->getNameForDiagnostic(OS, Policy, true);
+
+if (!OS.str().empty())
+  return;
+
+Decl *Ctx = Decl::castFromDeclContext(NamedTemplate->getDeclContext());
+NamedDecl *NamedCtx = dyn_cast_or_null(Ctx);
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const auto *R = dyn_cast(Decl)) {
+if (R->isLambda()) {
+  OS << "lambda at ";
+  Decl->getLocation().print(OS, TheSema.getSourceManager());
+  return;
+}
+  }
+  OS << "unnamed " << Decl->getKindName();
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed function parameter " << Decl->getFunctionScopeIndex()
+ << " ";
+  if (Decl->getFunctionScopeDepth() > 0)
+OS << "(at depth " << Decl->getFunctionScopeDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  if (const Type *Ty = Decl->getTypeForDecl()) {
+if (const auto *TTPT = dyn_cast_or_null(Ty)) {
+  OS << "unnamed template type parameter " << TTPT->getIndex() << " ";
+  if (TTPT->getDepth() > 0)
+OS << "(at depth " << TTPT->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+  }
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed template non-type parameter " << Decl->getIndex() << " ";
+  if (Decl->getDepth() > 0)
+OS << "(at depth " << Decl->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+if (const auto *Decl = dyn_cast(NamedTemplate)) {
+  OS << "unnamed template template parameter " << Decl->getIndex() << " ";
+  if (Decl->getDepth() > 0)
+OS << "(at depth " << Decl->getDepth() << ") ";
+  OS << "of ";
+  NamedCtx->getNameForDiagnostic(OS, TheSema.getLangOpts(), true);
+  return;
+}
+
+llvm_unreachable("Failed to retrieve a name for this entry!");
+OS << "unnamed identifier";
+  }
+
   template 
   static TemplightEntry getTemplightEntry(const Sema ,
   const CodeSynthesisContext ) {
 TemplightEntry Entry;
 Entry.Kind = toString(Inst.Kind);
 Entry.Event = BeginInstantiation ? "Begin" : "End";
-if (auto *NamedTemplate = dyn_cast_or_null(Inst.Entity)) {
-  llvm::raw_string_ostream 

[clang] 8cc2de6 - [analyzer][docs] Fix the incorrect structure of the checker docs

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-11-09T15:26:00+01:00
New Revision: 8cc2de667ec2526b055e971f46f4b3731107546c

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

LOG: [analyzer][docs] Fix the incorrect structure of the checker docs

The alpha.security.cert section came right after alpha.security, making it look
like checkers like alpha.security.MmapWriteExec belonged to that package.

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 62eeb16d10dfa..80cf3bc7c3132 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2064,90 +2064,6 @@ Warns against using one vs. many plural pattern in code 
when generating localize
 alpha.security
 ^^
 
-
-alpha.security.cert
-^^^
-
-SEI CERT checkers which tries to find errors based on their `C coding rules 
`_.
-
-.. _alpha-security-cert-pos-checkers:
-
-alpha.security.cert.pos
-^^^
-
-SEI CERT checkers of `POSIX C coding rules 
`__.
-
-.. _alpha-security-cert-pos-34c:
-
-alpha.security.cert.pos.34c
-"""
-Finds calls to the ``putenv`` function which pass a pointer to an automatic 
variable as the argument.
-
-.. code-block:: c
-
-  int func(const char *var) {
-char env[1024];
-int retval = snprintf(env, sizeof(env),"TEST=%s", var);
-if (retval < 0 || (size_t)retval >= sizeof(env)) {
-/* Handle error */
-}
-
-return putenv(env); // putenv function should not be called with auto 
variables
-  }
-
-alpha.security.cert.env
-^^^
-
-SEI CERT checkers of `POSIX C coding rules 
`__.
-
-.. _alpha-security-cert-env-InvalidPtr:
-
-alpha.security.cert.env.InvalidPtr
-""
-
-Corresponds to SEI CERT Rules ENV31-C and ENV34-C.
-
-ENV31-C:
-Rule is about the possible problem with `main` function's third argument, 
environment pointer,
-"envp". When enviornment array is modified using some modification function
-such as putenv, setenv or others, It may happen that memory is reallocated,
-however "envp" is not updated to reflect the changes and points to old memory
-region.
-
-ENV34-C:
-Some functions return a pointer to a statically allocated buffer.
-Consequently, subsequent call of these functions will invalidate previous
-pointer. These functions include: getenv, localeconv, asctime, setlocale, 
strerror
-
-.. code-block:: c
-
-  int main(int argc, const char *argv[], const char *envp[]) {
-if (setenv("MY_NEW_VAR", "new_value", 1) != 0) {
-  // setenv call may invalidate 'envp'
-  /* Handle error */
-}
-if (envp != NULL) {
-  for (size_t i = 0; envp[i] != NULL; ++i) {
-puts(envp[i]);
-// envp may no longer point to the current environment
-// this program has unanticipated behavior, since envp
-// does not reflect changes made by setenv function.
-  }
-}
-return 0;
-  }
-
-  void previous_call_invalidation() {
-char *p, *pp;
-
-p = getenv("VAR");
-pp = getenv("VAR2");
-// subsequent call to 'getenv' invalidated previous one
-
-*p;
-// dereferencing invalid pointer
-  }
-
 .. _alpha-security-ArrayBound:
 
 alpha.security.ArrayBound (C)
@@ -2299,6 +2215,95 @@ Check for an out-of-bound pointer being returned to 
callers.
return x; // warn: undefined or garbage returned
  }
 
+
+alpha.security.cert
+^^^
+
+SEI CERT checkers which tries to find errors based on their `C coding rules 
`_.
+
+.. _alpha-security-cert-pos-checkers:
+
+alpha.security.cert.pos
+^^^
+
+SEI CERT checkers of `POSIX C coding rules 
`_.
+
+.. _alpha-security-cert-pos-34c:
+
+alpha.security.cert.pos.34c
+"""
+Finds calls to the ``putenv`` function which pass a pointer to an automatic 
variable as the argument.
+
+.. code-block:: c
+
+  int func(const char *var) {
+char env[1024];
+int retval = snprintf(env, sizeof(env),"TEST=%s", var);
+if (retval < 0 || (size_t)retval >= sizeof(env)) {
+/* Handle error */
+}
+
+return putenv(env); // putenv function should not be called with auto 
variables
+  }
+
+alpha.security.cert.env
+^^^
+
+SEI CERT checkers of `Environment C coding rules 
`_.
+
+.. 

[clang] fb4d590 - Fix a unittest file after D108695 when Z3 is enabled

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-14T16:11:11+02:00
New Revision: fb4d590a622f4031900516360c07ee6ace01c5e6

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

LOG: Fix a unittest file after D108695 when Z3 is enabled

Added: 


Modified: 
clang/unittests/StaticAnalyzer/CheckerRegistration.h
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp

Removed: 




diff  --git a/clang/unittests/StaticAnalyzer/CheckerRegistration.h 
b/clang/unittests/StaticAnalyzer/CheckerRegistration.h
index de804236229ad..bccdb94d29383 100644
--- a/clang/unittests/StaticAnalyzer/CheckerRegistration.h
+++ b/clang/unittests/StaticAnalyzer/CheckerRegistration.h
@@ -20,11 +20,27 @@
 namespace clang {
 namespace ento {
 
-class DiagConsumer : public PathDiagnosticConsumer {
+class OnlyWarningsDiagConsumer : public PathDiagnosticConsumer {
   llvm::raw_ostream 
 
 public:
-  DiagConsumer(llvm::raw_ostream ) : Output(Output) {}
+  OnlyWarningsDiagConsumer(llvm::raw_ostream ) : Output(Output) {}
+  void FlushDiagnosticsImpl(std::vector ,
+FilesMade *filesMade) override {
+for (const auto *PD : Diags) {
+  Output << PD->getCheckerName() << ": ";
+  Output << PD->getShortDescription() << '\n';
+}
+  }
+
+  StringRef getName() const override { return "Test"; }
+};
+
+class PathDiagConsumer : public PathDiagnosticConsumer {
+  llvm::raw_ostream 
+
+public:
+  PathDiagConsumer(llvm::raw_ostream ) : Output(Output) {}
   void FlushDiagnosticsImpl(std::vector ,
 FilesMade *filesMade) override {
 for (const auto *PD : Diags) {
@@ -65,18 +81,24 @@ void addChecker(AnalysisASTConsumer ,
   Fn1(AnalysisConsumer, AnOpts);
 }
 
-template 
-class TestAction : public ASTFrontendAction {
+template  class TestAction : public ASTFrontendAction {
   llvm::raw_ostream 
+  bool OnlyEmitWarnings;
 
 public:
-  TestAction(llvm::raw_ostream ) : DiagsOutput(DiagsOutput) {}
+  TestAction(llvm::raw_ostream , bool OnlyEmitWarnings)
+  : DiagsOutput(DiagsOutput), OnlyEmitWarnings(OnlyEmitWarnings) {}
 
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
  StringRef File) override {
 std::unique_ptr AnalysisConsumer =
 CreateAnalysisConsumer(Compiler);
-AnalysisConsumer->AddDiagnosticConsumer(new DiagConsumer(DiagsOutput));
+if (OnlyEmitWarnings)
+  AnalysisConsumer->AddDiagnosticConsumer(
+  new OnlyWarningsDiagConsumer(DiagsOutput));
+else
+  AnalysisConsumer->AddDiagnosticConsumer(
+  new PathDiagConsumer(DiagsOutput));
 addChecker(*AnalysisConsumer, *Compiler.getAnalyzerOpts());
 return std::move(AnalysisConsumer);
   }
@@ -92,15 +114,16 @@ inline SmallString<80> getCurrentTestNameAsFileName() {
 }
 
 template 
-bool runCheckerOnCode(const std::string , std::string ) {
+bool runCheckerOnCode(const std::string , std::string ,
+  bool OnlyEmitWarnings = false) {
   const SmallVectorImpl  = getCurrentTestNameAsFileName();
   llvm::raw_string_ostream OS(Diags);
-  return tooling::runToolOnCode(std::make_unique>(OS), Code,
-FileName);
+  return tooling::runToolOnCode(
+  std::make_unique>(OS, OnlyEmitWarnings), Code,
+  FileName);
 }
 
-template 
-bool runCheckerOnCode(const std::string ) {
+template  bool runCheckerOnCode(const std::string ) {
   std::string Diags;
   return runCheckerOnCode(Code, Diags);
 }
@@ -108,11 +131,13 @@ bool runCheckerOnCode(const std::string ) {
 template 
 bool runCheckerOnCodeWithArgs(const std::string ,
   const std::vector ,
-  std::string ) {
+  std::string ,
+  bool OnlyEmitWarnings = false) {
   const SmallVectorImpl  = getCurrentTestNameAsFileName();
   llvm::raw_string_ostream OS(Diags);
   return tooling::runToolOnCodeWithArgs(
-  std::make_unique>(OS), Code, Args, FileName);
+  std::make_unique>(OS, OnlyEmitWarnings), Code, Args,
+  FileName);
 }
 
 template 

diff  --git 
a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp 
b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
index beaaebdd36cf9..28dad31f54f3e 100644
--- a/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
+++ b/clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
@@ -126,7 +126,7 @@ TEST_F(FalsePositiveRefutationBRVisitorTestBase, 
UnSatInTheMiddleNoReport) {
 
   std::string Diags;
   EXPECT_TRUE(runCheckerOnCodeWithArgs(
-  Code, LazyAssumeAndCrossCheckArgs, Diags));
+  Code, LazyAssumeAndCrossCheckArgs, Diags, /*OnlyEmitWarnings=*/ true));
   

[clang] 9d359f6 - [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only when a function "intents", but doesn't change ownership, enable by default

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-13T15:01:20+02:00
New Revision: 9d359f6c738632c6973e9f5328b10bf39b3df55a

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

LOG: [analyzer] MallocChecker: Add notes from NoOwnershipChangeVisitor only 
when a function "intents", but doesn't change ownership, enable by default

D105819 Added NoOwnershipChangeVisitor, but it is only registered when an
off-by-default, hidden checker option was enabled. The reason behind this was
that it grossly overestimated the set of functions that really needed a note:

std::string getTrainName(const Train *T) {
  return T->name;
} // note: Retuning without changing the ownership of or deallocating memory
// Umm... I mean duh? Nor would I expect this function to do anything like 
that...

void foo() {
  Train *T = new Train("Land Plane");
  print(getTrainName(T)); // note: calling getTrainName / returning from 
getTrainName
} // warn: Memory leak

This patch adds a heuristic that guesses that any function that has an explicit
operator delete call could have be responsible for deallocating the memory that
ended up leaking. This is wy too conservative (see the TODOs in the new
function), but it safer to err on the side of too little than too much, and
would allow us to enable the option by default *now*, and add refinements
one-by-one.

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/NewDeleteLeaks.cpp
clang/test/Analysis/analyzer-config.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 125ef859d1ebb..86513a3e541be 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -493,8 +493,8 @@ def DynamicMemoryModeling: Checker<"DynamicMemoryModeling">,
   "that neither deallocated it, or have taken responsibility "
   "of the ownership are noted, similarly to "
   "NoStoreFuncVisitor.",
-  "false",
-  InAlpha,
+  "true",
+  Released,
   Hide>
   ]>,
   Dependencies<[CStringModeling]>,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index d97e8c33ce254..9899298b348a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -52,6 +52,8 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/ProgramPoint.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
@@ -791,9 +793,28 @@ class NoOwnershipChangeVisitor final : public 
NoStateChangeFuncVisitor {
 return "";
   }
 
+  bool doesFnIntendToHandleOwnership(const Decl *Callee, ASTContext ) {
+using namespace clang::ast_matchers;
+const FunctionDecl *FD = dyn_cast(Callee);
+if (!FD)
+  return false;
+// TODO: Operator delete is hardly the only deallocator -- Can we reuse
+// isFreeingCall() or something thats already here?
+auto Deallocations = match(
+stmt(hasDescendant(cxxDeleteExpr().bind("delete"))
+ ), *FD->getBody(), ACtx);
+// TODO: Ownership my change with an attempt to store the allocated memory.
+return !Deallocations.empty();
+  }
+
   virtual bool
   wasModifiedInFunction(const ExplodedNode *CallEnterN,
 const ExplodedNode *CallExitEndN) override {
+if (!doesFnIntendToHandleOwnership(
+CallExitEndN->getFirstPred()->getLocationContext()->getDecl(),
+CallExitEndN->getState()->getAnalysisManager().getASTContext()))
+  return true;
+
 if (CallEnterN->getState()->get(Sym) !=
 CallExitEndN->getState()->get(Sym))
   return true;

diff  --git a/clang/test/Analysis/NewDeleteLeaks.cpp 
b/clang/test/Analysis/NewDeleteLeaks.cpp
index 28040d9d0d36b..57c7e57c98e32 100644
--- a/clang/test/Analysis/NewDeleteLeaks.cpp
+++ b/clang/test/Analysis/NewDeleteLeaks.cpp
@@ -20,15 +20,16 @@
 
 bool coin();
 
+// TODO: AST analysis of sink would reveal that it doesn't intent to free the
+// allocated memory, but in this instance, its also the only function with
+// the ability to do so, we should see a note here.
 namespace memory_allocated_in_fn_call {
 
 void sink(int *P) {
-} // ownership-note {{Returning without deallocating memory or storing the 
pointer for later 

[clang] 0213d7e - [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-13T13:50:01+02:00
New Revision: 0213d7ec0c501414d12020737fdc47e47e4392d9

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

LOG: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire 
function calls, rather than each ExplodedNode in it

Fix a compilation error due to a missing 'template' keyword.

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

Added: 
clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/CallEventTest.cpp
clang/unittests/StaticAnalyzer/CheckerRegistration.h
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 139b0dcd51704..c42521376af92 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -633,6 +633,9 @@ class CXXConstructorCall;
 /// Descendants can define what a "state change is", like a change of value
 /// to a memory region, liveness, etc. For function calls where the state did
 /// not change as defined, a custom note may be constructed.
+///
+/// For a minimal example, check out
+/// clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp.
 class NoStateChangeFuncVisitor : public BugReporterVisitor {
 private:
   /// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
@@ -643,6 +646,8 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// many times (going up the path for each node and checking whether the
   /// region was written into) we instead lazily compute the stack frames
   /// along the path.
+  // TODO: Can't we just use a map instead? This is likely not as cheap as it
+  // makes the code 
diff icult to read.
   llvm::SmallPtrSet FramesModifying;
   llvm::SmallPtrSet FramesModifyingCalculated;
 
@@ -651,6 +656,8 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// The calculation is cached in FramesModifying.
   bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
 
+  void markFrameAsModifying(const StackFrameContext *SCtx);
+
   /// Write to \c FramesModifying all stack frames along the path in the 
current
   /// stack frame which modifies the state.
   void findModifyingFrames(const ExplodedNode *const CallExitBeginN);
@@ -658,11 +665,37 @@ class NoStateChangeFuncVisitor : public 
BugReporterVisitor {
 protected:
   bugreporter::TrackingKind TKind;
 
-  /// \return Whether the state was modified from the current node, \CurrN, to
-  /// the end of the stack fram, at \p CallExitBeginN.
-  virtual bool
-  wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
-const ExplodedNode *CallExitBeginN) = 0;
+  /// \return Whether the state was modified from the current node, \p CurrN, 
to
+  /// the end of the stack frame, at \p CallExitBeginN. \p CurrN and
+  /// \p CallExitBeginN are always in the same stack frame.
+  /// Clients should override this callback when a state change is important
+  /// not only on the entire function call, but inside of it as well.
+  /// Example: we may want to leave a note about the lack of locking/unlocking
+  /// on a particular mutex, but not if inside the function its state was
+  /// changed, but also restored. wasModifiedInFunction() wouldn't know of this
+  /// change.
+  virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
+ const ExplodedNode *CallExitBeginN) {
+return false;
+  }
+
+  /// \return Whether the state was modified in the inlined function call in
+  /// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame
+  /// retrieved from a CallEnterN and CallExitEndN is the *caller's* stack
+  /// frame! The inlined function's stack should be retrieved from either the
+  /// immediate successor to \p CallEnterN or immediate predecessor to
+  /// \p CallExitEndN.
+  /// Clients should override this function if a state changes local to the
+  /// inlined function are not interesting, only the change occuring as a
+  /// result of it.
+  /// Example: we want to leave a not about a leaked resource object not being
+  /// deallocated / its ownership changed inside a function, and we don't care
+  

[clang] a375bfb - [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-03T13:50:18+02:00
New Revision: a375bfb5b729e0f3ca8d5e001f423fa89e74de87

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

LOG: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire 
function calls, rather than each ExplodedNode in it

D105553 added NoStateChangeFuncVisitor, an abstract class to aid in creating
notes such as "Returning without writing to 'x'", or "Returning without changing
the ownership status of allocated memory". Its clients need to define, among
other things, what a change of state is.

For code like this:

f() {
  g();
}

foo() {
  f();
  h();
}

We'd have a path in the ExplodedGraph that looks like this:

 --  -->
/  \
 --- >---  --->
/\  /\
 -- -->

When we're interested in whether f neglected to change some property,
NoStateChangeFuncVisitor asks these questions:

   ÷×~
--  -->
   ß   /  \$

[clang] 3891b45 - Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it"

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-02T17:19:49+02:00
New Revision: 3891b45a06f9192b7185d03269717cd60dfdea13

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

LOG: Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to 
check entire function calls, rather than each ExplodedNode in it"

This reverts commit 7d0e62bfb773c68d2bc8831fddcc8536f4613190.

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/CallEventTest.cpp
clang/unittests/StaticAnalyzer/CheckerRegistration.h
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 
clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp



diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index c42521376af92..139b0dcd51704 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -633,9 +633,6 @@ class CXXConstructorCall;
 /// Descendants can define what a "state change is", like a change of value
 /// to a memory region, liveness, etc. For function calls where the state did
 /// not change as defined, a custom note may be constructed.
-///
-/// For a minimal example, check out
-/// clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp.
 class NoStateChangeFuncVisitor : public BugReporterVisitor {
 private:
   /// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
@@ -646,8 +643,6 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// many times (going up the path for each node and checking whether the
   /// region was written into) we instead lazily compute the stack frames
   /// along the path.
-  // TODO: Can't we just use a map instead? This is likely not as cheap as it
-  // makes the code 
diff icult to read.
   llvm::SmallPtrSet FramesModifying;
   llvm::SmallPtrSet FramesModifyingCalculated;
 
@@ -656,8 +651,6 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// The calculation is cached in FramesModifying.
   bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
 
-  void markFrameAsModifying(const StackFrameContext *SCtx);
-
   /// Write to \c FramesModifying all stack frames along the path in the 
current
   /// stack frame which modifies the state.
   void findModifyingFrames(const ExplodedNode *const CallExitBeginN);
@@ -665,37 +658,11 @@ class NoStateChangeFuncVisitor : public 
BugReporterVisitor {
 protected:
   bugreporter::TrackingKind TKind;
 
-  /// \return Whether the state was modified from the current node, \p CurrN, 
to
-  /// the end of the stack frame, at \p CallExitBeginN. \p CurrN and
-  /// \p CallExitBeginN are always in the same stack frame.
-  /// Clients should override this callback when a state change is important
-  /// not only on the entire function call, but inside of it as well.
-  /// Example: we may want to leave a note about the lack of locking/unlocking
-  /// on a particular mutex, but not if inside the function its state was
-  /// changed, but also restored. wasModifiedInFunction() wouldn't know of this
-  /// change.
-  virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
- const ExplodedNode *CallExitBeginN) {
-return false;
-  }
-
-  /// \return Whether the state was modified in the inlined function call in
-  /// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame
-  /// retrieved from a CallEnterN and CallExitEndN is the *caller's* stack
-  /// frame! The inlined function's stack should be retrieved from either the
-  /// immediate successor to \p CallEnterN or immediate predecessor to
-  /// \p CallExitEndN.
-  /// Clients should override this function if a state changes local to the
-  /// inlined function are not interesting, only the change occuring as a
-  /// result of it.
-  /// Example: we want to leave a not about a leaked resource object not being
-  /// deallocated / its ownership changed inside a function, and we don't care
-  /// if it was assigned to a local variable (its change in ownership is
-  /// inconsequential).
-  virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN,
- const ExplodedNode *CallExitEndN) {
-return false;
-  }
+  /// \return Whether the state was modified from the current 

[clang] 7d0e62b - [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-09-02T16:56:32+02:00
New Revision: 7d0e62bfb773c68d2bc8831fddcc8536f4613190

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

LOG: [analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire 
function calls, rather than each ExplodedNode in it

D105553 added NoStateChangeFuncVisitor, an abstract class to aid in creating
notes such as "Returning without writing to 'x'", or "Returning without changing
the ownership status of allocated memory". Its clients need to define, among
other things, what a change of state is.

For code like this:

f() {
  g();
}

foo() {
  f();
  h();
}

We'd have a path in the ExplodedGraph that looks like this:

 --  -->
/  \
 --- >---  --->
/\  /\
 -- -->

When we're interested in whether f neglected to change some property,
NoStateChangeFuncVisitor asks these questions:

   ÷×~
--  -->
   ß   /  \$

[clang] 2d3668c - [analyzer] MallocChecker: Add a visitor to leave a note on functions that could have, but did not change ownership on leaked memory

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-08-16T16:19:00+02:00
New Revision: 2d3668c997faac1f64cd3b8eb336af989069d135

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

LOG: [analyzer] MallocChecker: Add a visitor to leave a note on functions that 
could have, but did not change ownership on leaked memory

This is a rather common feedback we get from out leak checkers: bug reports are
really short, and are contain barely any usable information on what the analyzer
did to conclude that a leak actually happened.

This happens because of our bug report minimizing effort. We construct bug
reports by inspecting the ExplodedNodes that lead to the error from the bottom
up (from the error node all the way to the root of the exploded graph), and mark
entities that were the cause of a bug, or have interacted with it as
interesting. In order to make the bug report a bit less verbose, whenever we
find an entire function call (from CallEnter to CallExitEnd) that didn't talk
about any interesting entity, we prune it (click here for more info on bug
report generation). Even if the event to highlight is exactly this lack of
interaction with interesting entities.

D105553 generalized the visitor that creates notes for these cases. This patch
adds a new kind of NoStateChangeVisitor that leaves notes in functions that
took a piece of dynamically allocated memory that later leaked as parameter,
and didn't change its ownership status.

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

Added: 
clang/test/Analysis/NewDeleteLeaks.cpp

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

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index 444b00d73f0b7..125ef859d1ebb 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -485,7 +485,17 @@ def DynamicMemoryModeling: 
Checker<"DynamicMemoryModeling">,
   "allocating and deallocating functions are annotated with "
   "ownership_holds, ownership_takes and ownership_returns.",
   "false",
-  InAlpha>
+  InAlpha>,
+CmdLineOption
   ]>,
   Dependencies<[CStringModeling]>,
   Documentation,

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index a6470da09c458..7db4066653cbd 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -48,6 +48,7 @@
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ParentMap.h"
@@ -64,12 +65,15 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.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/SVals.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/StoreRef.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SetOperations.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Compiler.h"
@@ -298,6 +302,8 @@ class MallocChecker
   /// which might free a pointer are annotated.
   DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
+  DefaultBool ShouldRegisterNoOwnershipChangeVisitor;
+
   /// Many checkers are essentially built into this one, so enabling them will
   /// make MallocChecker perform additional modeling and reporting.
   enum CheckKind {
@@ -722,11 +728,146 @@ class MallocChecker
   bool isArgZERO_SIZE_PTR(ProgramStateRef State, CheckerContext ,
   SVal ArgVal) const;
 };
+} // end anonymous namespace
+
+//===--===//
+// Definition of NoOwnershipChangeVisitor.
+//===--===//
+
+namespace {
+class NoOwnershipChangeVisitor final : public NoStateChangeFuncVisitor {
+  SymbolRef Sym;
+  using OwnerSet = llvm::SmallPtrSet;
+
+  // Collect which entities point 

[clang] c019142 - [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-08-16T15:03:22+02:00
New Revision: c019142a89b477cd247434c1d8f571662d26e19d

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

LOG: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract 
NoStateChangeVisitor class

Preceding discussion on cfe-dev: 
https://lists.llvm.org/pipermail/cfe-dev/2021-June/068450.html

NoStoreFuncVisitor is a rather unique visitor. As VisitNode is invoked on most
other visitors, they are looking for the point where something changed -- change
on a value, some checker-specific GDM trait, a new constraint.
NoStoreFuncVisitor, however, looks specifically for functions that *didn't*
write to a MemRegion of interesting. Quoting from its comments:

/// Put a diagnostic on return statement of all inlined functions
/// for which  the region of interest \p RegionOfInterest was passed into,
/// but not written inside, and it has caused an undefined read or a null
/// pointer dereference outside.

It so happens that there are a number of other similar properties that are
worth checking. For instance, if some memory leaks, it might be interesting why
a function didn't take ownership of said memory:

void sink(int *P) {} // no notes

void f() {
  sink(new int(5)); // note: Memory is allocated
// Well hold on, sink() was supposed to deal with
// that, this must be a false positive...
} // warning: Potential memory leak [cplusplus.NewDeleteLeaks]

In here, the entity of interest isn't a MemRegion, but a symbol. The property
that changed here isn't a change of value, but rather liveness and GDM traits
managed by MalloChecker.

This patch moves some of the logic of NoStoreFuncVisitor to a new abstract
class, NoStateChangeFuncVisitor. This is mostly calculating and caching the
stack frames in which the entity of interest wasn't changed.

Descendants of this interface have to define 3 things:

* What constitutes as a change to an entity (this is done by overriding
wasModifiedBeforeCallExit)
* What the diagnostic message should be (this is done by overriding
maybeEmitNoteFor.*)
* What constitutes as the entity of interest being passed into the function 
(this
is also done by overriding maybeEmitNoteFor.*)

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

Added: 


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

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 24cae12af24a1..139b0dcd51704 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/StringRef.h"
 #include 
 #include 
@@ -622,6 +623,84 @@ class TagVisitor : public BugReporterVisitor {
PathSensitiveBugReport ) override;
 };
 
+class ObjCMethodCall;
+class CXXConstructorCall;
+
+/// Put a diagnostic on return statement (or on } in its absence) of all 
inlined
+/// functions for which some property remained unchanged.
+/// Resulting diagnostics may read such as "Returning without writing to X".
+///
+/// Descendants can define what a "state change is", like a change of value
+/// to a memory region, liveness, etc. For function calls where the state did
+/// not change as defined, a custom note may be constructed.
+class NoStateChangeFuncVisitor : public BugReporterVisitor {
+private:
+  /// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
+  /// This visitor generates a note only if a function does *not* change the
+  /// state that way. This information is not immediately available
+  /// by looking at the node associated with the exit from the function
+  /// (usually the return statement). To avoid recomputing the same information
+  /// many times (going up the path for each node and checking whether the
+  /// region was written into) we instead lazily compute the stack frames
+  /// along the path.
+  llvm::SmallPtrSet FramesModifying;
+  llvm::SmallPtrSet FramesModifyingCalculated;
+
+  /// Check and lazily calculate whether the state is modified in the stack
+  /// frame to which \p CallExitBeginN belongs.
+  /// The calculation is cached in FramesModifying.
+  bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
+
+  /// Write to \c FramesModifying all stack frames along the path in the 
current
+  /// stack frame which modifies 

[clang] 027c5a6 - [analyzer][NFC] Make test/Analysis/self-assign.cpp readable

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2021-08-13T16:14:54+02:00
New Revision: 027c5a6adcb3a41c29533680a650fae7262f2b31

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

LOG: [analyzer][NFC] Make test/Analysis/self-assign.cpp readable

Added: 


Modified: 
clang/test/Analysis/self-assign.cpp

Removed: 




diff  --git a/clang/test/Analysis/self-assign.cpp 
b/clang/test/Analysis/self-assign.cpp
index ca28c534f1e25..7d3ea99b005de 100644
--- a/clang/test/Analysis/self-assign.cpp
+++ b/clang/test/Analysis/self-assign.cpp
@@ -1,4 +1,9 @@
-// RUN: %clang_analyze_cc1 -std=c++11 
-analyzer-checker=core,cplusplus,unix.Malloc,debug.ExprInspection 
-analyzer-config eagerly-assume=false %s -verify -analyzer-output=text
+// RUN: %clang_analyze_cc1 -std=c++11 %s -verify -analyzer-output=text \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false
 
 extern "C" char *strdup(const char* s);
 extern "C" void free(void* ptr);
@@ -28,18 +33,31 @@ StringUsed::~StringUsed() {
   free(str);
 }
 
-StringUsed& StringUsed::operator=(const StringUsed ) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} 
expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUsed ::operator=(const StringUsed ) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs == *this}}
+  // expected-note@-3{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
-  str = strdup(rhs.str); // expected-warning{{Use of memory after it is 
freed}}  expected-note{{Use of memory after it is freed}}
-// expected-note@-1{{Memory is allocated}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ // expected-note@-1{{Use of memory after it is freed}}
+ // expected-note@-2{{Memory is allocated}}
   return *this;
 }
 
-StringUsed& StringUsed::operator=(StringUsed &) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUsed ::operator=(StringUsed &) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   str = rhs.str;
-  rhs.str = nullptr; // expected-warning{{Potential memory leak}} 
expected-note{{Potential memory leak}}
+  rhs.str = nullptr; // expected-warning{{Potential memory leak}}
+ // expected-note@-1{{Potential memory leak}}
   return *this;
 }
 
@@ -63,15 +81,27 @@ StringUnused::~StringUnused() {
   free(str);
 }
 
-StringUnused& StringUnused::operator=(const StringUnused ) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs == *this}} 
expected-note{{Assuming rhs != *this}}
-  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}} 
expected-warning{{UNKNOWN}} expected-note{{TRUE}} expected-note{{UNKNOWN}}
+StringUnused ::operator=(const StringUnused ) {
+  // expected-note@-1{{Assuming rhs == *this}}
+  // expected-note@-2{{Assuming rhs == *this}}
+  // expected-note@-3{{Assuming rhs != *this}}
+  clang_analyzer_eval(*this == rhs); // expected-warning{{TRUE}}
+ // expected-warning@-1{{UNKNOWN}}
+ // expected-note@-2{{TRUE}}
+ // expected-note@-3{{UNKNOWN}}
   free(str); // expected-note{{Memory is released}}
-  str = strdup(rhs.str); // expected-warning{{Use of memory after it is 
freed}}  expected-note{{Use of memory after it is freed}}
+  str = strdup(rhs.str); // expected-warning{{Use of memory after it is freed}}
+ // expected-note@-1{{Use of memory after it is freed}}
   return *this;
 }
 
-StringUnused& StringUnused::operator=(StringUnused &) { // 
expected-note{{Assuming rhs == *this}} expected-note{{Assuming rhs != *this}}
-  

[clang] 9cca5c1 - [analyzer] Make checker silencing work for non-pathsensitive bug reports

[Kristóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2021-06-17T10:27:34+02:00
New Revision: 9cca5c1391d637b5500ada646cf136ddb38254a3

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

LOG: [analyzer] Make checker silencing work for non-pathsensitive bug reports

D66572 separated BugReport and BugReporter into basic and path sensitive
versions. As a result, checker silencing, which worked deep in the path
sensitive report generation facilities became specific to it. DeadStoresChecker,
for instance, despite being in the static analyzer, emits non-pathsensitive
reports, and was impossible to silence.

This patch moves the corresponding code before the call to the virtual function
generateDiagnosticForConsumerMap (which is overriden by the specific kinds of
bug reporters). Although we see bug reporting as relatively lightweight compared
to the analysis, this will get rid of several steps we used to throw away.

Quoting from D65379:

At a very high level, this consists of 3 steps:

For all BugReports in the same BugReportEquivClass, collect all their error
nodes in a set. With that set, create a new, trimmed ExplodedGraph whose leafs
are all error nodes.
Until a valid report is found, construct a bug path, which is yet another
ExplodedGraph, that is linear from a given error node to the root of the graph.
Run all visitors on the constructed bug path. If in this process the report got
invalidated, start over from step 2.
Checker silencing used to kick in after all of these. Now it does before any of
them :^)

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

Change-Id: Ice42939304516f2bebd05a1ea19878b89c96a25d

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

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

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



diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp 
b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 61734808d1ba..6ace986b4e0d 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1989,13 +1989,6 @@ PathDiagnosticBuilder::generate(const 
PathDiagnosticConsumer *PDC) const {
   const SourceManager  = getSourceManager();
   const AnalyzerOptions  = getAnalyzerOptions();
 
-  // 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 (R->getBugType().getCheckerName().startswith(CheckerOrPackage))
-  return nullptr;
-  }
-
   if (!PDC->shouldGenerateDiagnostics())
 return generateEmptyDiagnosticForReport(R, getSourceManager());
 
@@ -3040,6 +3033,14 @@ void BugReporter::FlushReport(BugReportEquivClass& EQ) {
   if (!report)
 return;
 
+  // See whether we need to silence the checker/package.
+  for (const std::string  :
+   getAnalyzerOptions().SilencedCheckersAndPackages) {
+if (report->getBugType().getCheckerName().startswith(
+CheckerOrPackage))
+  return;
+  }
+
   ArrayRef Consumers = getPathDiagnosticConsumers();
   std::unique_ptr Diagnostics =
   generateDiagnosticForConsumerMap(report, Consumers, bugReports);

diff  --git a/clang/test/Analysis/silence-checkers-malloc.cpp 
b/clang/test/Analysis/silence-checkers.cpp
similarity index 81%
rename from clang/test/Analysis/silence-checkers-malloc.cpp
rename to clang/test/Analysis/silence-checkers.cpp
index 2f6a9dd2d5b8..a2f1e60a4522 100644
--- a/clang/test/Analysis/silence-checkers-malloc.cpp
+++ b/clang/test/Analysis/silence-checkers.cpp
@@ -11,6 +11,12 @@
 // RUN:   -analyzer-checker=cplusplus.NewDelete\
 // RUN:   -analyzer-config silence-checkers="unix"
 
+// RUN: %clang_analyze_cc1 -verify="deadstore-silenced" %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling \
+// RUN:   -analyzer-checker=deadcode \
+// RUN:   -analyzer-config silence-checkers="deadcode.DeadStores"
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof(sizeof(int)) size_t;
@@ -38,3 +44,17 @@ void test_delete_ZERO_SIZE_PTR() {
   delete Ptr; // no-silence-warning{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
   // unix-silenced-warning@-1{{Argument to 'delete' is a constant 
address (16), which is not memory allocated by 'new' [cplusplus.NewDelete]}}
 }
+
+// deadstore-silenced-no-diagnostics
+
+int foo() {
+  int x = 42;
+  return x;
+}
+
+void g() {
+  int y;
+  y = 7;
+  int x = foo();
+  y = 10;
+}



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

[clang] dd1d548 - [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-09-15T17:43:02+02:00
New Revision: dd1d5488e47d0a89217dfd22a726c3d3ad2b4984

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

LOG: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a 
thing doesn't exist

The summary and very short discussion in D82122 summarizes whats happening here.

In short, liveness talks about variables, or expressions, anything that
has a value. Well, statements just simply don't have a one.

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

Added: 


Modified: 
clang/docs/analyzer/developer-docs/DebugChecks.rst
clang/include/clang/Analysis/Analyses/LiveVariables.h
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
clang/lib/Analysis/LiveVariables.cpp
clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
clang/lib/StaticAnalyzer/Core/Environment.cpp
clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
clang/test/Analysis/live-stmts.cpp
clang/test/Analysis/live-stmts.mm

Removed: 




diff  --git a/clang/docs/analyzer/developer-docs/DebugChecks.rst 
b/clang/docs/analyzer/developer-docs/DebugChecks.rst
index 48b584a46307..45985a1dfd79 100644
--- a/clang/docs/analyzer/developer-docs/DebugChecks.rst
+++ b/clang/docs/analyzer/developer-docs/DebugChecks.rst
@@ -30,7 +30,7 @@ using a 'dot' format viewer (such as Graphviz on macOS) 
instead.
 - debug.DumpLiveVars: Show the results of live variable analysis for each
   top-level function being analyzed.
 
-- debug.DumpLiveStmts: Show the results of live statement analysis for each
+- debug.DumpLiveExprs: Show the results of live expression analysis for each
   top-level function being analyzed.
 
 - debug.ViewExplodedGraph: Show the Exploded Graphs generated for the

diff  --git a/clang/include/clang/Analysis/Analyses/LiveVariables.h 
b/clang/include/clang/Analysis/Analyses/LiveVariables.h
index 2e7dd5d81678..8a3dd0c35e64 100644
--- a/clang/include/clang/Analysis/Analyses/LiveVariables.h
+++ b/clang/include/clang/Analysis/Analyses/LiveVariables.h
@@ -30,22 +30,22 @@ class LiveVariables : public ManagedAnalysis {
   class LivenessValues {
   public:
 
-llvm::ImmutableSet liveStmts;
+llvm::ImmutableSet liveExprs;
 llvm::ImmutableSet liveDecls;
 llvm::ImmutableSet liveBindings;
 
 bool equals(const LivenessValues ) const;
 
 LivenessValues()
-  : liveStmts(nullptr), liveDecls(nullptr), liveBindings(nullptr) {}
+  : liveExprs(nullptr), liveDecls(nullptr), liveBindings(nullptr) {}
 
-LivenessValues(llvm::ImmutableSet LiveStmts,
+LivenessValues(llvm::ImmutableSet liveExprs,
llvm::ImmutableSet LiveDecls,
llvm::ImmutableSet LiveBindings)
-: liveStmts(LiveStmts), liveDecls(LiveDecls),
+: liveExprs(liveExprs), liveDecls(LiveDecls),
   liveBindings(LiveBindings) {}
 
-bool isLive(const Stmt *S) const;
+bool isLive(const Expr *E) const;
 bool isLive(const VarDecl *D) const;
 
 friend class LiveVariables;
@@ -83,17 +83,17 @@ class LiveVariables : public ManagedAnalysis {
   ///  only returns liveness information for block-level expressions.
   bool isLive(const Stmt *S, const VarDecl *D);
 
-  /// Returns true the block-level expression "value" is live
+  /// Returns true the block-level expression value is live
   ///  before the given block-level expression (see runOnAllBlocks).
-  bool isLive(const Stmt *Loc, const Stmt *StmtVal);
+  bool isLive(const Stmt *Loc, const Expr *Val);
 
   /// Print to stderr the variable liveness information associated with
   /// each basic block.
   void dumpBlockLiveness(const SourceManager );
 
-  /// Print to stderr the statement liveness information associated with
+  /// Print to stderr the expression liveness information associated with
   /// each basic block.
-  void dumpStmtLiveness(const SourceManager );
+  void dumpExprLiveness(const SourceManager );
 
   void runOnAllBlocks(Observer );
 

diff  --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td 
b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index cbc048ba74c4..3540fe5fe55c 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -1478,8 +1478,8 @@ def LiveVariablesDumper : Checker<"DumpLiveVars">,
   HelpText<"Print results of live variable analysis">,
   Documentation;
 
-def LiveStatementsDumper : Checker<"DumpLiveStmts">,
-  HelpText<"Print results of live statement analysis">,
+def LiveExpressionsDumper : Checker<"DumpLiveExprs">,
+  HelpText<"Print results of live expression analysis">,
   Documentation;
 
 def CFGViewer : Checker<"ViewCFG">,

[clang] 7c6f5b7 - [analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-09-15T16:55:44+02:00
New Revision: 7c6f5b7fbf5a9eee7f3ef9192c354d1536a8f1c6

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

LOG: [analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

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

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
clang/docs/analyzer/user-docs/CrossTranslationUnit.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index 7a294f916bcf..9fb6782cf5a5 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -1491,6 +1491,23 @@ Warn about assigning non-{0,1} values to boolean 
variables.
 alpha.core
 ^^
 
+.. _alpha-core-C11Lock:
+
+alpha.core.C11Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of ``mtx_t`` mutexes.
+
+.. code-block:: cpp
+
+ mtx_t mtx1;
+
+ void bad1(void)
+ {
+   mtx_lock();
+   mtx_lock(); // warn: This lock has already been acquired
+ }
+
 .. _alpha-core-CallAndMessageUnInitRefArg:
 
 alpha.core.CallAndMessageUnInitRefArg (C,C++, ObjC)
@@ -1868,6 +1885,26 @@ Check for dereference of null smart pointers.
*P; // warn: dereference of a default constructed smart unique_ptr
  }
 
+alpha.fuchsia
+^
+
+.. _alpha-fuchsia-lock:
+
+alpha.fuchsia.Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of fuchsia mutexes.
+
+.. code-block:: cpp
+
+ spin_lock_t mtx1;
+
+ void bad1(void)
+ {
+   spin_lock();
+   spin_lock();   // warn: This lock has already been acquired
+ }
+
 alpha.llvm
 ^^
 

diff  --git a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst 
b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
index 36be82f209ef..0606185f39e6 100644
--- a/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ b/clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -201,6 +201,8 @@ Example usage of scan-build-py:
   ^C
   $
 
+.. _ctu-on-demand:
+
 On-demand analysis
 __
 The analysis produces the necessary AST structure of external TUs during 
analysis. This requires the



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

Re: [clang] a8503b8 - [NFC] Remove unused static function

[Kristóf Umann via cfe-commits]

Yup, unless you this breaks something, I'd really prefer to keep it.

On Sat, 12 Sep 2020, 03:24 David Blaikie,  wrote:

> LLVM_DUMP_METHOD is meant to be used for annotating functions that might
> be useful to execute from a debugger to dump data structures, etc - so it's
> expected that they'd be unused. Do you find that this function is not
> useful to use from a debugger/similar situation? (or perhaps because the
> function was "static" it was hitting a compiler warning/error & not
> actually being emitted into the resulting binary for use by an interactive
> debugger?)
>
> +Kristof for comments/thoughts
>
> On Fri, Sep 11, 2020 at 4:50 PM Vitaly Buka via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> Author: Vitaly Buka
>> Date: 2020-09-11T16:50:30-07:00
>> New Revision: a8503b87f739776cc9d5738f69aa0990db952340
>>
>> URL:
>> https://github.com/llvm/llvm-project/commit/a8503b87f739776cc9d5738f69aa0990db952340
>> DIFF:
>> https://github.com/llvm/llvm-project/commit/a8503b87f739776cc9d5738f69aa0990db952340.diff
>>
>> LOG: [NFC] Remove unused static function
>>
>> Added:
>>
>>
>> Modified:
>> clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>>
>> Removed:
>>
>>
>>
>>
>> 
>> diff  --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> index 441dcad42444..ce4addd2f945 100644
>> --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
>> @@ -834,11 +834,6 @@ LLVM_DUMP_METHOD static void
>> dumpArgTokensToStream(llvm::raw_ostream ,
>> const Preprocessor
>> ,
>> const ArgTokensTy
>> );
>>
>> -LLVM_DUMP_METHOD static void dumpArgTokens(const Preprocessor ,
>> -   const ArgTokensTy ) {
>> -  dumpArgTokensToStream(llvm::errs(), PP, Toks);
>> -}
>> -
>>  namespace {
>>  /// Maps unexpanded macro parameters to expanded arguments. A macro
>> argument may
>>  /// need to expanded further when it is nested inside another macro.
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b9bca88 - [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it with a GDM trait

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-09-11T15:58:48+02:00
New Revision: b9bca883c970d36f408db80df21838c713c326db

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

LOG: [analyzer][NFC] Don't bind values to ObjCForCollectionStmt, replace it 
with a GDM trait

Based on the discussion in D82598#2171312. Thanks @NoQ!

D82598 is titled "Get rid of statement liveness, because such a thing doesn't
exist", and indeed, expressions express a value, non-expression statements
don't.

if (a && get() || []{ return true; }())
~~ has a value
~ has a value
~~ has a value
   has a value
~~~ doesn't have a value

That is simple enough, so it would only make sense if we only assigned symbolic
values to expressions in the static analyzer. Yet the interface checkers can
access presents, among other strange things, the following two methods:

ProgramState::BindExpr(const Stmt *S, const LocationContext *LCtx, SVal V,
   bool Invalidate=true)
ProgramState::getSVal(const Stmt *S, const LocationContext *LCtx)

So, what gives? Turns out, we make an exception for ReturnStmt (which we'll
leave for another time) and ObjCForCollectionStmt. For any other loops, in order
to know whether we should analyze another iteration, among other things, we
evaluate it's condition. Which is a problem for ObjCForCollectionStmt, because
it simply doesn't have one (CXXForRangeStmt has an implicit one!). In its
absence, we assigned the actual statement with a concrete 1 or 0 to indicate
whether there are any more iterations left. However, this is wildly incorrect,
its just simply not true that the for statement has a value of 1 or 0, we can't
calculate its liveness because that doesn't make any sense either, so this patch
turns it into a GDM trait.

Fixing this allows us to reinstate the assert removed in
https://reviews.llvm.org/rG032b78a0762bee129f33e4255ada6d374aa70c71.

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

Added: 
clang/test/Analysis/objc-live-crash.mm

Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
clang/lib/StaticAnalyzer/Checkers/UndefBranchChecker.cpp
clang/lib/StaticAnalyzer/Core/Environment.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
clang/lib/StaticAnalyzer/Core/SymbolManager.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index cdfe986355c5..582a56cbee1e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -869,6 +869,23 @@ class ExprEngine {
   void handleConstructor(const Expr *E, ExplodedNode *Pred,
  ExplodedNodeSet );
 
+public:
+  /// Note whether this loop has any more iteratios to model. These methods are
+  /// essentially an interface for a GDM trait. Further reading in
+  /// ExprEngine::VisitObjCForCollectionStmt().
+  LLVM_NODISCARD static ProgramStateRef
+  setWhetherHasMoreIteration(ProgramStateRef State,
+ const ObjCForCollectionStmt *O,
+ const LocationContext *LC, bool HasMoreIteraton);
+
+  LLVM_NODISCARD static ProgramStateRef
+  removeIterationState(ProgramStateRef State, const ObjCForCollectionStmt *O,
+   const LocationContext *LC);
+
+  LLVM_NODISCARD static bool hasMoreIteration(ProgramStateRef State,
+  const ObjCForCollectionStmt *O,
+  const LocationContext *LC);
+private:
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
   /// has CXXConstructExpr as its initializer, the object would be considered

diff  --git a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp 
b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
index 918c6e361381..a86a410ebcbc 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
@@ -978,8 +978,7 @@ void ObjCLoopChecker::checkPostStmt(const 
ObjCForCollectionStmt *FCS,
   ProgramStateRef State = C.getState();
 
   // Check if this is the branch for the end of the loop.
-  SVal CollectionSentinel = C.getSVal(FCS);
-  if (CollectionSentinel.isZeroConstant()) {
+  if (!ExprEngine::hasMoreIteration(State, FCS, 

[clang] 7527898 - [analyzer][MacroExpansion][NFC] Fix a missing test output check

[Kristóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-09-11T13:48:26+02:00
New Revision: 7527898fef47da929e70c81100a0248c2f445762

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

LOG: [analyzer][MacroExpansion][NFC] Fix a missing test output check

Added: 


Modified: 

clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
clang/test/Analysis/plist-macros-with-expansion.cpp

Removed: 




diff  --git 
a/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
 
b/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
index 2988f8504fcf..499119c81d25 100644
--- 
a/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ 
b/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -5645,12 +5645,12 @@
 start
  
   
-   line459
+   line462
col33
file0
   
   
-   line459
+   line462
col33
file0
   
@@ -5658,12 +5658,12 @@
 end
  
   
-   line459
+   line462
col37
file0
   
   
-   line459
+   line462
col39
file0
   
@@ -5675,7 +5675,7 @@
  kindevent
  location
  
-  line459
+  line462
   col37
   file0
  
@@ -5683,12 +5683,12 @@
  

 
- line459
+ line462
  col37
  file0
 
 
- line459
+ line462
  col41
  file0
 
@@ -5704,7 +5704,7 @@
  kindevent
  location
  
-  line458
+  line461
   col1
   file0
  
@@ -5718,7 +5718,7 @@
  kindevent
  location
  
-  line458
+  line461
   col1
   file0
  
@@ -5726,12 +5726,12 @@
  

 
- line458
+ line461
  col1
  file0
 
 
- line458
+ line461
  col16
  file0
 
@@ -5747,7 +5747,7 @@
  kindevent
  location
  
-  line459
+  line462
   col37
   file0
  
@@ -5755,12 +5755,12 @@
  

 
- line459
+ line462
  col37
  file0
 
 
- line459
+ line462
  col41
  file0
 
@@ -5780,12 +5780,12 @@
 start
  
   
-   line459
+   line462
col37
file0
   
   
-   line459
+   line462
col39
file0
   
@@ -5793,12 +5793,12 @@
 end
  
   
-   line459
+   line462
col35
file0
   
   
-   line459
+   line462
col35
file0
   
@@ -5810,7 +5810,7 @@
  kindevent
  location
  
-  line459
+  line462
   col35
   file0
  
@@ -5818,12 +5818,12 @@
  

 
- line459
+ line462
  col33
  file0
 
 
- line459
+ line462
  col41
  file0
 
@@ -5841,7 +5841,7 @@
 
  location
  
-  line458
+  line461
   col1
   file0
  
@@ -5860,7 +5860,7 @@
   issue_hash_function_offset0
   location
   
-   line459
+   line462
col35
file0
   
@@ -5868,8 +5868,8 @@
   
0

-458
-459
+461
+462

   
   
@@ -5884,12 +5884,12 @@
 start
  
   
-   line468
+   line471
col33
file0
   
   
-   line468
+   line471
col33
file0
   
@@ -5897,12 +5897,12 @@
 end
  
   
-   line468
+   line471
col37
file0
   
   
-   line468
+   line471
col39
file0
   
@@ -5914,7 +5914,7 @@
  kindevent
  location
  
-  line468
+  line471
   col37
   file0
  
@@ -5922,12 +5922,12 @@
  

 
- line468
+ line471
  col37
  file0
 
 
- line468
+ line471
  col41
  file0
 
@@ -5943,7 +5943,7 @@
  kindevent
  location
  
-  line467
+  line470
   col1
   file0
  
@@ -5957,7 +5957,7 @@
  kindevent
  location
  
-  line467
+  line470
   col1
   file0
  
@@ -5965,12 +5965,12 @@
  

 
- line467
+ line470
  

[clang] 26d9a94 - [analyzer][MacroExpansion][NFC] Fix incorrectly calling parameters arguments

[Kristóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-09-11T13:33:02+02:00
New Revision: 26d9a94681056f88bd3e892f8113093268fa0907

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

LOG: [analyzer][MacroExpansion][NFC] Fix incorrectly calling parameters 
arguments

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index c4b66da676aa..87c9b8479463 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -825,25 +825,26 @@ void PlistDiagnostics::FlushDiagnosticsImpl(
 
 namespace {
 
-using ExpArgTokensTy = llvm::SmallVector;
+using ArgTokensTy = llvm::SmallVector;
 
 } // end of anonymous namespace
 
-LLVM_DUMP_METHOD static void
-dumpExpArgTokensToStream(llvm::raw_ostream , const Preprocessor ,
- const ExpArgTokensTy );
+LLVM_DUMP_METHOD static void dumpArgTokensToStream(llvm::raw_ostream ,
+   const Preprocessor ,
+   const ArgTokensTy );
 
-LLVM_DUMP_METHOD static void dumpExpArgTokens(const Preprocessor ,
-  const ExpArgTokensTy ) {
-  dumpExpArgTokensToStream(llvm::errs(), PP, Toks);
+LLVM_DUMP_METHOD static void dumpArgTokens(const Preprocessor ,
+   const ArgTokensTy ) {
+  dumpArgTokensToStream(llvm::errs(), PP, Toks);
 }
 
 namespace {
-/// Maps unexpanded macro arguments to expanded arguments. A macro argument may
+/// Maps unexpanded macro parameters to expanded arguments. A macro argument 
may
 /// need to expanded further when it is nested inside another macro.
-class MacroArgMap : public std::map {
+class MacroParamMap : public std::map {
 public:
-  void expandFromPrevMacro(const MacroArgMap );
+  void expandFromPrevMacro(const MacroParamMap );
+
   LLVM_DUMP_METHOD void dump(const Preprocessor ) const {
 dumpToStream(llvm::errs(), PP);
   }
@@ -852,13 +853,13 @@ class MacroArgMap : public std::map {
  const Preprocessor ) const;
 };
 
-struct MacroNameAndArgs {
+struct MacroExpansionInfo {
   std::string Name;
   const MacroInfo *MI = nullptr;
-  MacroArgMap Args;
+  MacroParamMap ParamMap;
 
-  MacroNameAndArgs(std::string N, const MacroInfo *MI, MacroArgMap M)
-: Name(std::move(N)), MI(MI), Args(std::move(M)) {}
+  MacroExpansionInfo(std::string N, const MacroInfo *MI, MacroParamMap M)
+  : Name(std::move(N)), MI(MI), ParamMap(std::move(M)) {}
 };
 
 class TokenPrinter {
@@ -896,7 +897,7 @@ class TokenPrinter {
 ///
 /// As we expand the last line, we'll immediately replace PRINT(str) with
 /// print(x). The information that both 'str' and 'x' refers to the same string
-/// is an information we have to forward, hence the argument \p PrevArgs.
+/// is an information we have to forward, hence the argument \p PrevParamMap.
 ///
 /// To avoid infinite recursion we maintain the already processed tokens in
 /// a set. This is carried as a parameter through the recursive calls. The set
@@ -906,13 +907,11 @@ class TokenPrinter {
 /// #define f(y) x
 /// #define x f(x)
 static std::string getMacroNameAndPrintExpansion(
-TokenPrinter ,
-SourceLocation MacroLoc,
-const Preprocessor ,
-const MacroArgMap ,
+TokenPrinter , SourceLocation MacroLoc, const Preprocessor ,
+const MacroParamMap ,
 llvm::SmallPtrSet );
 
-/// Retrieves the name of the macro and what it's arguments expand into
+/// Retrieves the name of the macro and what it's parameters expand into
 /// at \p ExpanLoc.
 ///
 /// For example, for the following macro expansion:
@@ -934,8 +933,8 @@ static std::string getMacroNameAndPrintExpansion(
 /// When \p ExpanLoc references "SET_TO_NULL(a)" within the definition of
 /// "NOT_SUSPICOUS", the macro name "SET_TO_NULL" and the MacroArgMap map
 /// { (x, a) } will be returned.
-static MacroNameAndArgs getMacroNameAndArgs(SourceLocation ExpanLoc,
-const Preprocessor );
+static MacroExpansionInfo getMacroExpansionInfo(SourceLocation ExpanLoc,
+const Preprocessor );
 
 /// Retrieves the ')' token that matches '(' \p It points to.
 static MacroInfo::tokens_iterator getMatchingRParen(
@@ -969,21 +968,20 @@ getExpandedMacro(SourceLocation MacroLoc, const 
Preprocessor ,
   llvm::SmallPtrSet AlreadyProcessedTokens;
 
   std::string MacroName = getMacroNameAndPrintExpansion(
-  Printer, MacroLoc, *PPToUse, MacroArgMap{}, AlreadyProcessedTokens);
+  Printer, MacroLoc, *PPToUse, MacroParamMap{}, 

[clang] 1c08da3 - [analyzer][MacroExpansion] Add a few dumps functions

[Kristóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-09-11T13:29:14+02:00
New Revision: 1c08da38676d15600b5c707cf7522eb4273a5347

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

LOG: [analyzer][MacroExpansion] Add a few dumps functions

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index ed62778623a8..c4b66da676aa 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -825,13 +825,31 @@ void PlistDiagnostics::FlushDiagnosticsImpl(
 
 namespace {
 
-using ExpArgTokens = llvm::SmallVector;
+using ExpArgTokensTy = llvm::SmallVector;
 
+} // end of anonymous namespace
+
+LLVM_DUMP_METHOD static void
+dumpExpArgTokensToStream(llvm::raw_ostream , const Preprocessor ,
+ const ExpArgTokensTy );
+
+LLVM_DUMP_METHOD static void dumpExpArgTokens(const Preprocessor ,
+  const ExpArgTokensTy ) {
+  dumpExpArgTokensToStream(llvm::errs(), PP, Toks);
+}
+
+namespace {
 /// Maps unexpanded macro arguments to expanded arguments. A macro argument may
 /// need to expanded further when it is nested inside another macro.
-class MacroArgMap : public std::map {
+class MacroArgMap : public std::map {
 public:
   void expandFromPrevMacro(const MacroArgMap );
+  LLVM_DUMP_METHOD void dump(const Preprocessor ) const {
+dumpToStream(llvm::errs(), PP);
+  }
+
+  LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream ,
+ const Preprocessor ) const;
 };
 
 struct MacroNameAndArgs {
@@ -1225,7 +1243,7 @@ static const MacroInfo *getMacroInfoForLocation(const 
Preprocessor ,
 void MacroArgMap::expandFromPrevMacro(const MacroArgMap ) {
 
   for (value_type  : *this) {
-ExpArgTokens  = Pair.second;
+ExpArgTokensTy  = Pair.second;
 
 // For each token in the expanded macro argument.
 auto It = CurrExpArgTokens.begin();
@@ -1244,7 +1262,7 @@ void MacroArgMap::expandFromPrevMacro(const MacroArgMap 
) {
 continue;
   }
 
-  const ExpArgTokens  = Super.at(II);
+  const ExpArgTokensTy  = Super.at(II);
 
   It = CurrExpArgTokens.insert(
   It, SuperExpArgTokens.begin(), SuperExpArgTokens.end());
@@ -1254,6 +1272,23 @@ void MacroArgMap::expandFromPrevMacro(const MacroArgMap 
) {
   }
 }
 
+void MacroArgMap::dumpToStream(llvm::raw_ostream ,
+   const Preprocessor ) const {
+  for (const std::pair Pair : *this) {
+Out << Pair.first->getName() << " -> ";
+dumpExpArgTokensToStream(Out, PP, Pair.second);
+Out << '\n';
+  }
+}
+
+static void dumpExpArgTokensToStream(llvm::raw_ostream ,
+ const Preprocessor ,
+ const ExpArgTokensTy ) {
+  TokenPrinter Printer(Out, PP);
+  for (Token Tok : Toks)
+Printer.printToken(Tok);
+}
+
 void TokenPrinter::printToken(const Token ) {
   // If this is the first token to be printed, don't print space.
   if (PrevTok.isNot(tok::unknown)) {



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

Re: [clang] e4b3fc1 - Get rid of -Wunused warnings in release build, NFC.

[Kristóf Umann via cfe-commits]

Apologies for the inconvenience, though I wonder why I haven't gotten
builtbot mails :/

On Tue, 16 Jun 2020 at 09:54, Haojian Wu  wrote:

> On Mon, 15 Jun 2020 at 18:29, David Blaikie  wrote:
>
>> I don't think the comment's adding value here - it should be fairly
>> clear from the context that the whole loop only exists for some
>> assertions.
>>
>> Also: Could you remove the "(void)" casts now that the whole thing's
>> wrapped in NDEBUG?
>>
>
> oops, I think this is coincident, there were two patches at the same time
> to fix the issue.
> Cleaned it up in  e00dcf61a2f4397c0525db7133503b80d8ecf5ac.
>
>
>> (an alternative way of phrasing this that doesn't require the #ifdef
>> would be using nested llvm::all_of calls, but that would mean the
>> assertion firing wouldn't point to a particular mismatched pair, just
>> that overall the data structures were mismatched - if you think that
>> this isn't especially likely/wouldn't be super hard to debug with
>> that, maybe the assert would be more compact/tidier?
>>
>> assert(llvm::all_of(Dependencies, [&](const auto ) {
>>   return llvm::all_of(WeakDependencies, [&](const auto ) {
>> return WeakDepPair != DepPair && WeakDepPair.first !=
>> DepPair.second && WeakDepPair.second != DepPair.second;
>>   }
>> }) && "...");
>>
>
> I'm not the author of this code, defer the decision to the author Kirstóf,
> dkszelet...@gmail.com.
>
>

That is a great suggestion, but I find the individual asserts (with a
message!) a better solution to guide the developer should an error be made
in the TableGen file.


> On Fri, Jun 12, 2020 at 6:45 AM Haojian Wu via cfe-commits
>>  wrote:
>> >
>> >
>> > Author: Haojian Wu
>> > Date: 2020-06-12T15:42:29+02:00
>> > New Revision: e4b3fc18d33199e2081d300f14687d81be48b6a0
>> >
>> > URL:
>> https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0
>> > DIFF:
>> https://github.com/llvm/llvm-project/commit/e4b3fc18d33199e2081d300f14687d81be48b6a0.diff
>> >
>> > LOG: Get rid of -Wunused warnings in release build, NFC.
>> >
>> > Added:
>> >
>> >
>> > Modified:
>> > clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
>> >
>> > Removed:
>> >
>> >
>> >
>> >
>> 
>> > diff  --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
>> b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
>> > index c2ca9c12b025..4a7e0d91ea23 100644
>> > --- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
>> > +++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
>> > @@ -285,6 +285,7 @@ CheckerRegistry::CheckerRegistry(
>> >resolveDependencies();
>> >resolveDependencies();
>> >
>> > +#ifndef NDEBUG // avoid -Wunused warnings in release build.
>> >for (auto  : Dependencies) {
>> >  for (auto  : WeakDependencies) {
>> >// Some assertions to enforce that strong dependencies are
>> relations in
>> > @@ -298,6 +299,7 @@ CheckerRegistry::CheckerRegistry(
>> >   "A strong dependency mustn't be a weak dependency as
>> well!");
>> >  }
>> >}
>> > +#endif
>> >
>> >resolveCheckerAndPackageOptions();
>> >
>> >
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Kristóf Umann via cfe-commits]

Yup, I'm working on it already, thanks.

On Thu, 21 May 2020 at 01:36, Nico Weber  wrote:

> This breaks tests: http://45.33.8.238/linux/18215/step_7.txt
>
> On Wed, May 20, 2020 at 7:05 PM Kirstóf Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>> 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
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e551333 - [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to CallDescription

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-02-25T15:43:33+01:00
New Revision: e5513336aee4a9b10cb98f234145aeb4763fdd69

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

LOG: [analyzer][MallocChecker][NFC] Change the use of IdentifierInfo* to 
CallDescription

Exactly what it says on the tin! I decided not to merge this with the patch that
changes all these to a CallDescriptionMap object, so the patch is that much more
trivial.

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

Added: 
clang/test/Analysis/malloc-annotations.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Analysis/kmalloc-linux.c

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
index fc1cc9138826..5b9bbb197e82 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -258,6 +258,13 @@ class CallEvent {
   /// calls.
   bool isCalled(const CallDescription ) const;
 
+  /// Returns true whether the CallEvent is any of the CallDescriptions 
supplied
+  /// as a parameter.
+  template 
+  bool isCalled(const FirstCallDesc , const CallDescs &... Rest) const {
+return isCalled(First) || isCalled(Rest...);
+  }
+
   /// Returns a source range for the entire call, suitable for
   /// outputting in diagnostics.
   virtual SourceRange getSourceRange() const {

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index fd3ce6cf3b9b..23b4abc67079 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -268,8 +268,6 @@ REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, 
ReallocPair)
 
 namespace {
 
-enum class MemoryOperationKind { MOK_Allocate, MOK_Free, MOK_Any };
-
 struct MemFunctionInfoTy {
   /// The value of the MallocChecker:Optimistic is stored in this variable.
   ///
@@ -279,44 +277,41 @@ struct MemFunctionInfoTy {
   /// which might free a pointer are annotated.
   DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
-  // TODO: Change these to CallDescription, and get rid of lazy initialization.
-  mutable IdentifierInfo *II_alloca = nullptr, *II_win_alloca = nullptr,
- *II_malloc = nullptr, *II_free = nullptr,
- *II_realloc = nullptr, *II_calloc = nullptr,
- *II_valloc = nullptr, *II_reallocf = nullptr,
- *II_strndup = nullptr, *II_strdup = nullptr,
- *II_win_strdup = nullptr, *II_kmalloc = nullptr,
- *II_if_nameindex = nullptr,
- *II_if_freenameindex = nullptr, *II_wcsdup = nullptr,
- *II_win_wcsdup = nullptr, *II_g_malloc = nullptr,
- *II_g_malloc0 = nullptr, *II_g_realloc = nullptr,
- *II_g_try_malloc = nullptr,
- *II_g_try_malloc0 = nullptr,
- *II_g_try_realloc = nullptr, *II_g_free = nullptr,
- *II_g_memdup = nullptr, *II_g_malloc_n = nullptr,
- *II_g_malloc0_n = nullptr, *II_g_realloc_n = nullptr,
- *II_g_try_malloc_n = nullptr,
- *II_g_try_malloc0_n = nullptr, *II_kfree = nullptr,
- *II_g_try_realloc_n = nullptr;
-
-  void initIdentifierInfo(ASTContext ) const;
-
-  ///@{
-  /// Check if this is one of the functions which can allocate/reallocate
-  /// memory pointed to by one of its arguments.
-  bool isMemFunction(const FunctionDecl *FD, ASTContext ) const;
-  bool isCMemFunction(const FunctionDecl *FD, ASTContext ,
-  AllocationFamily Family,
-  MemoryOperationKind MemKind) const;
-
-  /// Tells if the callee is one of the builtin new/delete operators, including
-  /// placement operators and other standard overloads.
-  bool isStandardNewDelete(const FunctionDecl *FD, ASTContext ) const;
-  ///@}
+  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}, 

[clang] 9fd7ce7 - [analyzer][MallocChecker][NFC] Communicate the allocation family to auxiliary functions with parameters

[Kristóf Umann via cfe-commits]

Author: Kristóf Umann
Date: 2020-02-25T11:17:32+01:00
New Revision: 9fd7ce7f4449619bc85ab4d2643e656836a2d5e2

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

LOG: [analyzer][MallocChecker][NFC] Communicate the allocation family to 
auxiliary functions with parameters

The following series of refactoring patches aim to fix the horrible mess that 
MallocChecker.cpp is.

I genuinely hate this file. It goes completely against how most of the checkers
are implemented, its by far the biggest headache regarding checker dependencies,
checker options, or anything you can imagine. On top of all that, its just bad
code. Its seriously everything that you shouldn't do in C++, or any other
language really. Bad variable/class names, in/out parameters... Apologies, rant
over.

So: there are a variety of memory manipulating function this checker models. One
aspect of these functions is their AllocationFamily, which we use to distinguish
between allocation kinds, like using free() on an object allocated by operator
new. However, since we always know which function we're actually modeling, in
fact we know it compile time, there is no need to use tricks to retrieve this
information out of thin air n+1 function calls down the line. This patch changes
many methods of MallocChecker to take a non-optional AllocationFamily template
parameter (which also makes stack dumps a bit nicer!), and removes some no
longer needed auxiliary functions.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h 
b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
index 9642588d6a41..99731d6044a0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
+++ b/clang/lib/StaticAnalyzer/Checkers/InterCheckerAPI.h
@@ -11,13 +11,19 @@
 
 #ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
 #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_INTERCHECKERAPI_H
-namespace clang {
-class CheckerManager;
 
+// FIXME: This file goes against how a checker should be implemented either in
+// a single file, or be exposed in a header file. Let's try to get rid of it!
+
+namespace clang {
 namespace ento {
 
+class CheckerManager;
+
 /// Register the part of MallocChecker connected to InnerPointerChecker.
 void registerInnerPointerCheckerAux(CheckerManager );
 
-}}
+} // namespace ento
+} // namespace clang
+
 #endif /* INTERCHECKERAPI_H_ */

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 1d365bb727fd..fd3ce6cf3b9b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -44,13 +44,14 @@
 //
 
//===--===//
 
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "AllocationState.h"
 #include "InterCheckerAPI.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -64,7 +65,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
-#include "AllocationState.h"
+#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 
@@ -72,7 +73,10 @@ using namespace clang;
 using namespace ento;
 
 
//===--===//
-// The types of allocation we're modeling.
+// The types of allocation we're modeling. This is used to check whether a
+// dynamically allocated object is deallocated with the correct function, like
+// not using operator delete on an object created by malloc(), or alloca 
regions
+// aren't ever deallocated manually.
 
//===--===//
 
 namespace {
@@ -92,22 +96,14 @@ struct MemFunctionInfoTy;
 
 } // end of anonymous namespace
 
-/// Determine family of a deallocation expression.
-static AllocationFamily
-getAllocationFamily(const MemFunctionInfoTy , CheckerContext 
,
-const Stmt *S);
-
 /// Print names of allocators and deallocators.
 ///
 /// \returns true on success.
-static bool printAllocDeallocName(raw_ostream , CheckerContext ,
-  const Expr *E);
+static bool 

Re: [clang-tools-extra] r369763 - [clang-tidy] Possibility of displaying duplicate warnings

[Kristóf Umann via cfe-commits]

Apologies for not picking this up, I just recently changed my commit email.
I can see that it was fixed in the meanwhile.

On Sat, 24 Aug 2019 at 01:45, Galina Kistanova via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hello Kristof,
>
> This commit broke test to few builders:
>
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/53703
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast
>
> . . .
> Failing Tests (1):
> Clang Tools :: clang-tidy/duplicate-reports.cpp
>
> Please have a look ASAP?
>
> Thanks
>
> Galina
>
> On Fri, Aug 23, 2019 at 7:56 AM Kristof Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: szelethus
>> Date: Fri Aug 23 07:57:27 2019
>> New Revision: 369763
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=369763=rev
>> Log:
>> [clang-tidy] Possibility of displaying duplicate warnings
>>
>> Summary: In case a checker is registered multiple times as an alias, the
>> emitted warnings are uniqued by the report message. However, it is random
>> which checker name is included in the warning. When processing the output
>> of clang-tidy this behavior caused some problems. In this commit the
>> uniquing key contains the checker name too.
>>
>> Reviewers: alexfh, xazax.hun, Szelethus, aaron.ballman, lebedev.ri,
>> JonasToth, gribozavr
>>
>> Reviewed By: alexfh
>>
>> Subscribers: dkrupp, whisperity, rnkovacs, mgrang, cfe-commits
>>
>> Patch by Tibor Brunner!
>>
>> Tags: #clang
>>
>> Differential Revision: https://reviews.llvm.org/D65065
>>
>> Added:
>> clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=369763=369762=369763=diff
>>
>> ==
>> --- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>> (original)
>> +++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
>> Fri Aug 23 07:57:27 2019
>> @@ -742,8 +742,9 @@ struct LessClangTidyError {
>>  const tooling::DiagnosticMessage  = LHS.Message;
>>  const tooling::DiagnosticMessage  = RHS.Message;
>>
>> -return std::tie(M1.FilePath, M1.FileOffset, M1.Message) <
>> -   std::tie(M2.FilePath, M2.FileOffset, M2.Message);
>> +return
>> +  std::tie(M1.FilePath, M1.FileOffset, LHS.DiagnosticName,
>> M1.Message) <
>> +  std::tie(M2.FilePath, M2.FileOffset, RHS.DiagnosticName,
>> M2.Message);
>>}
>>  };
>>  struct EqualClangTidyError {
>>
>> Added: clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp?rev=369763=auto
>>
>> ==
>> --- clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp (added)
>> +++ clang-tools-extra/trunk/test/clang-tidy/duplicate-reports.cpp Fri Aug
>> 23 07:57:27 2019
>> @@ -0,0 +1,15 @@
>> +// RUN: %check_clang_tidy %s cert-err09-cpp,cert-err61-cpp %t
>> +
>> +void alwaysThrows() {
>> +  int ex = 42;
>> +  // CHECK-MESSAGES: warning: throw expression should throw anonymous
>> temporary values instead [cert-err09-cpp]
>> +  // CHECK-MESSAGES: warning: throw expression should throw anonymous
>> temporary values instead [cert-err61-cpp]
>> +  throw ex;
>> +}
>> +
>> +void doTheJob() {
>> +  try {
>> +alwaysThrows();
>> +  } catch (int&) {
>> +  }
>> +}
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r369616 - [analyzer] Enable control dependency condition tracking by default

[Kristóf Umann via cfe-commits]

Totally possible, thanks for letting me know! There should be plenty of
room for caching, because I do calculate control dependencies in an excess
for the same CFG, and the retrieval of a basic block from an ExplodedNode
is anything but efficient, though I honestly didnt expect a performance hit
that drastic (and havent experienced it either).

I'll roll out some fixes during the weekend. If the problem persists
after that, I'd be happy to dig deeper.


On Fri, 23 Aug 2019, 20:33 Alexander Kornienko,  wrote:

> I suspect that this patch makes analysis much slower in certain cases. For
> example, the clang/test/Analysis/pr37802.cpp test has become ~5 times
> slower in some configurations in our environment. This happened somewhere
> between r369520 and r369679, and your series of patches seems most
> suspicious :). Is it expected? Can it be improved?
>
> On Thu, Aug 22, 2019 at 5:07 AM Kristof Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: szelethus
>> Date: Wed Aug 21 20:08:48 2019
>> New Revision: 369616
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=369616=rev
>> Log:
>> [analyzer] Enable control dependency condition tracking by default
>>
>> This patch concludes my GSoC'19 project by enabling track-conditions by
>> default.
>>
>> Differential Revision: https://reviews.llvm.org/D66381
>>
>> Modified:
>> cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> cfe/trunk/test/Analysis/analyzer-config.c
>> cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> cfe/trunk/test/Analysis/return-value-guaranteed.cpp
>> cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
>>
>> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
>> (original)
>> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def Wed
>> Aug 21 20:08:48 2019
>> @@ -294,7 +294,7 @@ ANALYZER_OPTION(bool, DisplayCTUProgress
>>  ANALYZER_OPTION(bool, ShouldTrackConditions, "track-conditions",
>>  "Whether to track conditions that are a control
>> dependency of "
>>  "an already tracked variable.",
>> -false)
>> +true)
>>
>>  ANALYZER_OPTION(bool, ShouldTrackConditionsDebug,
>> "track-conditions-debug",
>>  "Whether to place an event at each tracked condition.",
>>
>> Modified: cfe/trunk/test/Analysis/analyzer-config.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/analyzer-config.c (original)
>> +++ cfe/trunk/test/Analysis/analyzer-config.c Wed Aug 21 20:08:48 2019
>> @@ -87,7 +87,7 @@
>>  // CHECK-NEXT: suppress-c++-stdlib = true
>>  // CHECK-NEXT: suppress-inlined-defensive-checks = true
>>  // CHECK-NEXT: suppress-null-return-paths = true
>> -// CHECK-NEXT: track-conditions = false
>> +// CHECK-NEXT: track-conditions = true
>>  // CHECK-NEXT: track-conditions-debug = false
>>  // CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
>>  // CHECK-NEXT: unroll-loops = false
>>
>> Modified: cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m
>> (original)
>> +++ cfe/trunk/test/Analysis/diagnostics/no-store-func-path-notes.m Wed
>> Aug 21 20:08:48 2019
>> @@ -16,6 +16,7 @@ extern int coin();
>>  return 0;
>>}
>>return 1; // expected-note{{Returning without writing to '*var'}}
>> +  // expected-note@-1{{Returning the value 1, which participates in a
>> condition later}}
>>  }
>>  @end
>>
>>
>> Modified: cfe/trunk/test/Analysis/return-value-guaranteed.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/return-value-guaranteed.cpp?rev=369616=369615=369616=diff
>>
>> ==
>> --- cfe/trunk/test/Analysis/return-value-guaranteed.cpp (original)
>> +++ cfe/trunk/test/Analysis/return-value-guaranteed.cpp Wed Aug 21
>> 20:08:48 2019
>> @@ -24,6 +24,7 @@ bool parseFoo(Foo ) {
>>// class-note@-1 {{The value 0 is assigned to 'F.Field'}}
>>return !MCAsmParser::Error();
>>// class-note@-1 {{'MCAsmParser::Error' returns true}}
>> +  // class-note@-2 {{Returning zero, which participates in a condition
>> later}}
>>  }
>>
>>  bool parseFile() {
>> @@ -57,6 +58,7 @@ namespace 

Re: r365030 - Make a buildbot using a buggy gcc happy

[Kristóf Umann via cfe-commits]

Thank you so much! Sorry for the inconvencience, I'll be that much more
careful next time :)

On Mon, 8 Jul 2019, 21:42 Richard Smith,  wrote:

> I'll commit the change below once my testing finishes :)
>
> On Mon, 8 Jul 2019 at 12:40, Kristóf Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Noted, thanks! Gabor, could you please fix this?
>>
>> On Mon, 8 Jul 2019, 21:37 Richard Smith,  wrote:
>>
>>> This is in any case the wrong fix. The code *is* wrong, for the reason
>>> this compiler is reporting.
>>>
>>> The correct fix is to declare the explicit specializations in the header
>>> file:
>>>
>>> template <> void CFGDominatorTreeImpl::anchor();
>>> template <> void CFGDominatorTreeImpl::anchor();
>>>
>>> Clang will tell you to do this under -Wundefined-func-template (which we
>>> haven't turned on by default because people get this wrong too often...).
>>>
>>> On Mon, 8 Jul 2019 at 12:29, JF Bastien via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
>>>> Kristof,
>>>>
>>>> It looks like your fix didn’t address all the bots:
>>>>
>>>> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/lib/Analysis/Dominators.cpp:14:48:
>>>> error: explicit specialization of 'anchor' after instantiation void
>>>> CFGDominatorTreeImpl::anchor() {} ^
>>>> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/include/clang/Analysis/Analyses/Dominators.h:225:3:
>>>> note: implicit instantiation first required here
>>>> ControlDependencyCalculator(CFG *cfg) ^
>>>>
>>>> Can you please address the issue?
>>>>
>>>> http://green.lab.llvm.org/green/job/clang-stage2-coverage-R/4153/consoleFull
>>>>
>>>> Thanks,
>>>>
>>>> JF
>>>>
>>>>
>>>> On Jul 3, 2019, at 5:06 AM, Kristof Umann via cfe-commits <
>>>> cfe-commits@lists.llvm.org> wrote:
>>>>
>>>> Author: szelethus
>>>> Date: Wed Jul  3 05:06:10 2019
>>>> New Revision: 365030
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=365030=rev
>>>> Log:
>>>> Make a buildbot using a buggy gcc happy
>>>>
>>>> When specializing a template in a namespace, it has to be in a namespace
>>>> block, else gcc will get confused. Hopefully this fixes the issue.
>>>>
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
>>>>
>>>> Modified:
>>>>cfe/trunk/lib/Analysis/Dominators.cpp
>>>>
>>>> Modified: cfe/trunk/lib/Analysis/Dominators.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Dominators.cpp?rev=365030=365029=365030=diff
>>>>
>>>> ==
>>>> --- cfe/trunk/lib/Analysis/Dominators.cpp (original)
>>>> +++ cfe/trunk/lib/Analysis/Dominators.cpp Wed Jul  3 05:06:10 2019
>>>> @@ -8,10 +8,12 @@
>>>>
>>>> #include "clang/Analysis/Analyses/Dominators.h"
>>>>
>>>> -using namespace clang;
>>>> +namespace clang {
>>>>
>>>> template <>
>>>> -void clang::CFGDominatorTreeImpl::anchor() {}
>>>> +void CFGDominatorTreeImpl::anchor() {}
>>>>
>>>> template <>
>>>> -void clang::CFGDominatorTreeImpl::anchor() {}
>>>> +void CFGDominatorTreeImpl::anchor() {}
>>>> +
>>>> +} // end of namespace clang
>>>>
>>>>
>>>> ___
>>>> cfe-commits mailing list
>>>> cfe-commits@lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>>
>>>> ___
>>>> cfe-commits mailing list
>>>> cfe-commits@lists.llvm.org
>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r365030 - Make a buildbot using a buggy gcc happy

[Kristóf Umann via cfe-commits]

Noted, thanks! Gabor, could you please fix this?

On Mon, 8 Jul 2019, 21:37 Richard Smith,  wrote:

> This is in any case the wrong fix. The code *is* wrong, for the reason
> this compiler is reporting.
>
> The correct fix is to declare the explicit specializations in the header
> file:
>
> template <> void CFGDominatorTreeImpl::anchor();
> template <> void CFGDominatorTreeImpl::anchor();
>
> Clang will tell you to do this under -Wundefined-func-template (which we
> haven't turned on by default because people get this wrong too often...).
>
> On Mon, 8 Jul 2019 at 12:29, JF Bastien via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Kristof,
>>
>> It looks like your fix didn’t address all the bots:
>>
>> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/lib/Analysis/Dominators.cpp:14:48:
>> error: explicit specialization of 'anchor' after instantiation void
>> CFGDominatorTreeImpl::anchor() {} ^
>> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/include/clang/Analysis/Analyses/Dominators.h:225:3:
>> note: implicit instantiation first required here
>> ControlDependencyCalculator(CFG *cfg) ^
>>
>> Can you please address the issue?
>>
>> http://green.lab.llvm.org/green/job/clang-stage2-coverage-R/4153/consoleFull
>>
>> Thanks,
>>
>> JF
>>
>>
>> On Jul 3, 2019, at 5:06 AM, Kristof Umann via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>> Author: szelethus
>> Date: Wed Jul  3 05:06:10 2019
>> New Revision: 365030
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=365030=rev
>> Log:
>> Make a buildbot using a buggy gcc happy
>>
>> When specializing a template in a namespace, it has to be in a namespace
>> block, else gcc will get confused. Hopefully this fixes the issue.
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
>>
>> Modified:
>>cfe/trunk/lib/Analysis/Dominators.cpp
>>
>> Modified: cfe/trunk/lib/Analysis/Dominators.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Dominators.cpp?rev=365030=365029=365030=diff
>>
>> ==
>> --- cfe/trunk/lib/Analysis/Dominators.cpp (original)
>> +++ cfe/trunk/lib/Analysis/Dominators.cpp Wed Jul  3 05:06:10 2019
>> @@ -8,10 +8,12 @@
>>
>> #include "clang/Analysis/Analyses/Dominators.h"
>>
>> -using namespace clang;
>> +namespace clang {
>>
>> template <>
>> -void clang::CFGDominatorTreeImpl::anchor() {}
>> +void CFGDominatorTreeImpl::anchor() {}
>>
>> template <>
>> -void clang::CFGDominatorTreeImpl::anchor() {}
>> +void CFGDominatorTreeImpl::anchor() {}
>> +
>> +} // end of namespace clang
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r365030 - Make a buildbot using a buggy gcc happy

[Kristóf Umann via cfe-commits]

Hi!

Unfortunately, I'm out of town for the week -- I'll see whether I can ask
someone in the office to help out with this.

On Mon, 8 Jul 2019, 21:29 JF Bastien,  wrote:

> Kristof,
>
> It looks like your fix didn’t address all the bots:
>
> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/lib/Analysis/Dominators.cpp:14:48:
> error: explicit specialization of 'anchor' after instantiation void
> CFGDominatorTreeImpl::anchor() {} ^
> /Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/include/clang/Analysis/Analyses/Dominators.h:225:3:
> note: implicit instantiation first required here
> ControlDependencyCalculator(CFG *cfg) ^
>
> Can you please address the issue?
>
> http://green.lab.llvm.org/green/job/clang-stage2-coverage-R/4153/consoleFull
>
> Thanks,
>
> JF
>
>
> On Jul 3, 2019, at 5:06 AM, Kristof Umann via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
> Author: szelethus
> Date: Wed Jul  3 05:06:10 2019
> New Revision: 365030
>
> URL: http://llvm.org/viewvc/llvm-project?rev=365030=rev
> Log:
> Make a buildbot using a buggy gcc happy
>
> When specializing a template in a namespace, it has to be in a namespace
> block, else gcc will get confused. Hopefully this fixes the issue.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56480
>
> Modified:
>cfe/trunk/lib/Analysis/Dominators.cpp
>
> Modified: cfe/trunk/lib/Analysis/Dominators.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Dominators.cpp?rev=365030=365029=365030=diff
>
> ==
> --- cfe/trunk/lib/Analysis/Dominators.cpp (original)
> +++ cfe/trunk/lib/Analysis/Dominators.cpp Wed Jul  3 05:06:10 2019
> @@ -8,10 +8,12 @@
>
> #include "clang/Analysis/Analyses/Dominators.h"
>
> -using namespace clang;
> +namespace clang {
>
> template <>
> -void clang::CFGDominatorTreeImpl::anchor() {}
> +void CFGDominatorTreeImpl::anchor() {}
>
> template <>
> -void clang::CFGDominatorTreeImpl::anchor() {}
> +void CFGDominatorTreeImpl::anchor() {}
> +
> +} // end of namespace clang
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits