[PATCH] D47155: [analyzer] Reduce simplifySVal complexity threshold further.

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Hello! Thank you for addressing this problem. Are these kinds of symbols common 
in real code? For me it seems very artificial. However, I agree with George, it 
would be better to have this value as an analyzer option with a default value 
(of 20).


Repository:
  rC Clang

https://reviews.llvm.org/D47155



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Can we continue the discussion here, please? We should involve Devin and/or 
George as well if we cannot agree ourselves.


https://reviews.llvm.org/D35110



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' 
throws
+  implicit_int_thrower();

lebedev.ri wrote:
> baloghadamsoftware wrote:
> > dberris wrote:
> > > How deep does this go? Say we have a call to a function that's extern 
> > > which doesn't have 'noexcept' nor a dynamic exception specifier -- do we 
> > > assume that the call to an extern function may throw? Does that warn? 
> > > What does the warning look like? Should it warn? How about when you call 
> > > a function through a function pointer?
> > > 
> > > The documentation should cover these cases and/or more explicitly say in 
> > > the warning that an exception may throw in a noexcept function (rather 
> > > than just "function <...> throws").
> > We take the most conservative way here. We assume that a function throws if 
> > and only if its body has a throw statement or its header has the (old) 
> > throw() exception specification. We do not follow function pointers.
> While i realize it may have more noise, it may be nice to have a more 
> pedantic mode (guarded by an option?).
> E.g. `noexcept` propagation, much like `const` on methods propagation.
> Or at least a test that shows that it does not happen, unless i simply did 
> not notice it :)
This could be an enhancement in the future, yes.


https://reviews.llvm.org/D33537



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 148804.
baloghadamsoftware added a comment.

I still disagree, but I want the review to continue so I did the requested 
modifications.


https://reviews.llvm.org/D35110

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.h
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
   clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
 }
 
-//---
-// False positives
-//---
-
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
   clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,66 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+void assert_in_range(int x) {
+  assert(x <= ((int)INT_MAX / 4));
+  assert(x >= -(((int)INT_MAX) / 4));
+}
+
+void assert_in_range_2(int m, int n) {
+  assert_in_range(m);
+  assert_in_range(n);
+}
+
+void equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m != n)
+return;
+  clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m == n)
+return;
+  clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m < n)
+return;
+  clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m <= n)
+return;
+  clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m > n)
+return;
+  clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m >= n)
+return;
+  clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangedConstraintManager.h
===
--- lib/StaticAnalyzer/Core/RangedConstraintManager.h
+++ lib/StaticAnalyzer/Core/RangedConstraintManager.h
@@ -115,6 +115,8 @@
   RangeSet Intersect(BasicValueFactory &BV, Factory &F, llvm::APSInt Lower,
  llvm::APSInt Upper) const;
 
+  RangeSet Negate(BasicValueFactory &BV, Factory &F) const;
+
   void print(raw_ostream &os) const;
 
   bool operator==(const RangeSet &other) const {
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -174,6 +174,28 @@
   return newRanges;
 }
 
+// Turn all [A, B] ranges to [-B, -A]. Turn minimal 

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 116662.
baloghadamsoftware added a comment.

If both sides have concrete integers, they must be in range [min/4..max/4], if 
only one, it must be in range [min/2..max/2].


https://reviews.llvm.org/D35109

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,163 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+void clang_analyzer_printState();
+
+int f();
+
+void compare_different_symbol() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 0}}
+}
+
+void compare_different_symbol_plus_left_int() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_minus_left_int() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_different_symbol_plus_right_int() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 2}}
+}
+
+void compare_different_symbol_minus_right_int() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_same_symbol() {
+  int x = f(), y = x;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{1 S32b}}
+}
+
+void compare_same_symbol_plus_left_int() {
+  int x = f(), y = x;
+  ++x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_left_int() {
+  int x = f(), y = x;
+  --x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_plus_right_int() {
+  int x = f(), y = x+1;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_right_int() {
+  int x = f(), y = x-1;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

It seems that Artem's suggestion is not enough (or I misunderstood it). So two 
options remain: either we drop this and revert to the local solution in the 
Iterator Checkers or we extend the type when rearranging the comparison. Which 
way to go?


https://reviews.llvm.org/D35109



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I think that the acceptable false positive rate of a compiler warning is much 
lower than that of a Tidy check. So I understand that the fronted patch will 
not handle the indirect cases (which are the most important in my opinion) or 
the cases where there are multiple throws in a try block. However, it is nearly 
impossible to remove the most straightforward cases from the check and also in 
today's Tidy checks or Static Analyzer checkers there is some overlapping with 
warnings. I think that maybe we should consider making the error locations and 
error text to overlap with the warning so an advanced tool (such as 
CodeChecker) could unify them.


https://reviews.llvm.org/D33537



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

[PATCH] D34508: [Analyzer] Bug Reporter Visitor to Display Values of Variables - PRELIMINARY!

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I am considering to restrict the assumptions to nodes marked as interesting and 
to the location of the bug. However, I have difficulties with the latter, it 
seems that the bug location itself is not part of the bug path.


https://reviews.llvm.org/D34508



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I tried to extend the type to avoid overflow scenarios. Unfortunately, this 
breaks essential calculations based on the overflow scenarios (e.g. 
ProgramSate::assumeInbound()). So I see no other option than to abandon this 
patch and return to the local solution in the iterator checkers.


https://reviews.llvm.org/D35109



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

[PATCH] D39048: Dump signed integers in SymIntExpr and IntSymExpr correctly

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.

When dumping SymIntExpr or IntSymExpr signed numbers are dumped as unsigned, 
thus their two's complement is displayed. This small patch fixes this.


https://reviews.llvm.org/D39048

Files:
  lib/StaticAnalyzer/Core/SymbolManager.cpp
  test/Analysis/expr-inspection.c


Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -8,6 +8,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
   int y = 1;
   clang_analyzer_printState();
   for (; y < 3; ++y)
Index: lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -31,14 +31,20 @@
   os << '(';
   getLHS()->dumpToStream(os);
   os << ") "
- << BinaryOperator::getOpcodeStr(getOpcode()) << ' '
- << getRHS().getZExtValue();
+ << BinaryOperator::getOpcodeStr(getOpcode()) << ' ';
+  if (getRHS().isUnsigned())
+os << getRHS().getZExtValue();
+  else
+os << getRHS().getSExtValue();
   if (getRHS().isUnsigned())
 os << 'U';
 }
 
 void IntSymExpr::dumpToStream(raw_ostream &os) const {
-  os << getLHS().getZExtValue();
+  if (getLHS().isUnsigned())
+os << getLHS().getZExtValue();
+  else
+os << getLHS().getSExtValue();
   if (getLHS().isUnsigned())
 os << 'U';
   os << ' '


Index: test/Analysis/expr-inspection.c
===
--- test/Analysis/expr-inspection.c
+++ test/Analysis/expr-inspection.c
@@ -8,6 +8,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
   int y = 1;
   clang_analyzer_printState();
   for (; y < 3; ++y)
Index: lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -31,14 +31,20 @@
   os << '(';
   getLHS()->dumpToStream(os);
   os << ") "
- << BinaryOperator::getOpcodeStr(getOpcode()) << ' '
- << getRHS().getZExtValue();
+ << BinaryOperator::getOpcodeStr(getOpcode()) << ' ';
+  if (getRHS().isUnsigned())
+os << getRHS().getZExtValue();
+  else
+os << getRHS().getSExtValue();
   if (getRHS().isUnsigned())
 os << 'U';
 }
 
 void IntSymExpr::dumpToStream(raw_ostream &os) const {
-  os << getLHS().getZExtValue();
+  if (getLHS().isUnsigned())
+os << getLHS().getZExtValue();
+  else
+os << getLHS().getSExtValue();
   if (getLHS().isUnsigned())
 os << 'U';
   os << ' '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 119576.
baloghadamsoftware added a comment.
Herald added a subscriber: szepet.

I think it is the final attempt. If Symbols are different, the type is 
extended, so we store a correct (but extended) range. However, if the Symbols 
are the same, we do not change the type so checks like `assumeInbound()` are 
not affected.

Anna, Devin, Artem, please review it whether this version is functionally 
correct. (Style is another thing.) If not, then we should go back to the local 
solution in the iterator checkers. However, we must continue the review there, 
it is standing still for half a year.


https://reviews.llvm.org/D35109

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/std-c-library-functions.c
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,913 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+void clang_analyzer_printState();
+
+int f();
+
+void compare_different_symbol_equal() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 0}}
+}
+
+void compare_different_symbol_plus_left_int_equal() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 1}}
+}
+
+void compare_different_symbol_minus_left_int_equal() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 1}}
+}
+
+void compare_different_symbol_plus_right_int_equal() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 2}}
+}
+
+void compare_different_symbol_minus_right_int_equal() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int_equal() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int_equal() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int_equal() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int_equal() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 1}}
+}
+
+void compare_same_symbol_equal() {
+  int x = f(), y = x;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{1 S32b}}
+}
+
+void compare_same_symbol_plus_left_int_equal() {
+  int x = f(), y = x;
+  ++x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_left_int_equal() {
+  int x = f(), y = x;
+  --x;
+  clang_analyzer_dump(x); // expected-

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
Herald added subscribers: mgorny, srhines.

A possible error is to write ``malloc(strlen(s+1))`` instead of 
``malloc(strlen(s)+1)``. Unfortunately the former is also valid syntactically, 
but allocates less memory by two bytes (if ``s`` is at least one character 
long, undefined behavior otherwise) which may result in overflow cases. This 
check detects such cases and also suggests the fix for them.


https://reviews.llvm.org/D39121

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp
  clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
  test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp

Index: test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-misplaced-operator-in-strlen-in-alloc.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s misc-misplaced-operator-in-strlen-in-alloc %t
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+
+size_t strlen(const char*);
+
+void bad_malloc(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) malloc\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_calloc(char *name) {
+  char *new_names = (char*) calloc(2, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_names = \(char\*\) calloc\(2, }}strlen(name) + 1{{\);$}}
+}
+
+void bad_realloc(char * old_name, char *name) {
+  char *new_name = (char*) realloc(old_name, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) realloc\(old_name, }}strlen(name) + 1{{\);$}}
+}
Index: docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - misc-misplaced-operator-in-strlen-in-alloc
+
+misc-misplaced-operator-in-strlen-in-alloc
+==
+
+Finds cases a value is added to or subtracted from the string in the parameter
+of ``strlen()`` method instead of to the result and use its return value as an
+argument of a memory allocation function (``malloc()``, ``calloc()``,
+``realloc()``).
+
+Example code:
+
+.. code-block:: c
+
+void bad_malloc(char *str) {
+  char *c = (char*) malloc(strlen(str + 1));
+}
+
+
+The suggested fix is to add value to the return value of ``strlen()`` and not
+to its argument. In the example above the fix would be
+
+.. code-block:: c
+
+  char *c = (char*) malloc(strlen(str) + 1);
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -7,9 +7,9 @@
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
+   android-cloexec-dup
android-cloexec-epoll-create
android-cloexec-epoll-create1
-   android-cloexec-dup
android-cloexec-fopen
android-cloexec-inotify-init
android-cloexec-inotify-init1
@@ -38,7 +38,7 @@
cert-msc30-c (redirects to cert-msc50-cpp) 
cert-msc50-cpp
cert-oop11-cpp (redirects to misc-move-constructor-init) 
-   cppcoreguidelines-c-copy-assignment-signature
+   cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
@@ -76,8 +76,8 @@
hicpp-explicit-conversions (redirects to google-explicit-constructor) 
hicpp-function-size (redirects to readability-function-size) 
hicpp-invalid-access-moved (redirects to misc-use-after-move) 
-   hicpp-move-const-arg (redirects to misc-move-const-arg) 
hicpp-member-init (redirects to cppcoreguidelines-pro-type-member-init) 
+   hicpp-move-const-arg (redirects to misc-move-const-arg) 
hicpp-named-parameter (redirects to readability-named-parameter) 
hicpp-new-delete-operators (redirects to misc-new-delete-overloads) 
hicpp-no-array-decay (redirects to cppcoreguidelines-pro-bounds-array-to-pointer-decay) 
@@ -95,7 +95,7 @@
hicpp-use-noexcept (redirects to modernize-use-noexcept) 
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
-   hicpp-vararg (redirects to cppcoreguidelines-pro-type-

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 120214.
baloghadamsoftware added a comment.

Updated according to comments.


https://reviews.llvm.org/D39121

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp

Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+
+size_t strlen(const char*);
+
+void bad_malloc(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) malloc\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_alloca(char *name) {
+  char *new_name = (char*) alloca(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) alloca\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_calloc(char *name) {
+  char *new_names = (char*) calloc(2, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_names = \(char\*\) calloc\(2, }}strlen(name) + 1{{\);$}}
+}
+
+void bad_realloc(char * old_name, char *name) {
+  char *new_name = (char*) realloc(old_name, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) realloc\(old_name, }}strlen(name) + 1{{\);$}}
+}
+
+void intentional1(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 1) + 1);
+}
+
+
+void intentional2(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 2));
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -18,6 +18,7 @@
android-cloexec-socket
boost-use-to-string
bugprone-integer-division
+   bugprone-misplaced-operator-in-strlen-in-alloc
bugprone-suspicious-memset-usage
bugprone-undefined-memory-manipulation
cert-dcl03-c (redirects to misc-static-assert) 
Index: docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - bugprone-misplaced-operator-in-strlen-in-alloc
+
+bugprone-misplaced-operator-in-strlen-in-alloc
+==
+
+Finds cases a value is added to or subtracted from the string in the parameter
+of ``strlen()`` method instead of to the result and use its return value as an
+argument of a memory allocation function (``malloc()``, ``calloc()``,
+``realloc()``).
+
+Example code:
+
+.. code-block:: c
+
+void bad_malloc(char *str) {
+  char *c = (char*) malloc(strlen(str + 1));
+}
+
+
+The suggested fix is to add value to the return value of ``strlen()`` and not
+to its argument. In the example above the fix would be
+
+.. code-block:: c
+
+  char *c = (char*) malloc(strlen(str) + 1);
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,14 @@
 Improvements to clang-tidy
 --
 
+- New `bugprone-misplaced-operator-in-strlen-in-alloc
+  `_ check
+
+  Finds cases a value is added to or subtracted from the string in the parameter
+  of ``strlen()`` method instead of to the result and use its return value as an
+  argument of a memory allocation function (``malloc()``, ``calloc()``,
+  ``realloc()``). The check also suggests the appropriate fix.
+
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
 
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
===
--- /dev/null
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
@@ -0,0 +1,37 @@
+//===--- MisplacedOperatorInStrlenInAllocCheck.h - clang-tidy*-

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 5 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp:30
+  Finder->addMatcher(
+  callExpr(callee(functionDecl(hasName("malloc"))),
+   hasArgument(0, anyOf(hasDescendant(BadUse), BadUse)))

xazax.hun wrote:
> Maybe it is worth to have a configurable list of allocation functions?
> 
> Maybe it would be worth to support`alloca` as well?
Support for ``alloca`` added.

I agree, it is worth to have a configurable list, but I think it is better to 
be done in a separate patch.



Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.cpp:44
+const MatchFinder::MatchResult &Result) {
+  // FIXME: Add callback implementation.
+  const auto *Alloc = Result.Nodes.getNodeAs("Alloc");

xazax.hun wrote:
> What is this fixme?
Sorry, I forgot to remove it.



Comment at: clang-tidy/misc/MisplacedOperatorInStrlenInAllocCheck.h:19
+
+/// FIXME: Write a short description.
+///

xazax.hun wrote:
> There is a missing description.
No longer. Sorry, I forgot it.



Comment at: docs/clang-tidy/checks/list.rst:10
android-cloexec-creat
+   android-cloexec-dup
android-cloexec-epoll-create

Eugene.Zelenko wrote:
> I think will be better to fix order of other checks separately from this 
> check.
I agree, but I did not do it. It was the script that created the new check. I 
changed it back to the wrong order.



Comment at: 
docs/clang-tidy/checks/misc-misplaced-operator-in-strlen-in-alloc.rst:16
+void bad_malloc(char *str) {
+  char *c = (char*) malloc(strlen(str + 1));
+}

aaron.ballman wrote:
> xazax.hun wrote:
> > What if this code is intentional for some reason?
> > I think in thase case it could be rewritten like the following (which is 
> > much cleaner):
> > ```
> > char *c = (char*) malloc(strlen(str)-1);
> > ```
> > I think it might be a good idea to mention this possibility as a way to 
> > suppress this warning in the documentation. 
> This is my concern as well -- I'd like to know how chatty this diagnostic is 
> on real-world code bases, especially ones that rely on C rather than C++. A 
> somewhat common code pattern in Win32 coding are double-null-terminated lists 
> of strings, where you have null terminated strings at adjacent memory 
> locations with two null characters for the end of the list. This could result 
> in reasonable code like `malloc(strlen(str + offset) + 1)`.
It is now more conservative: only ``+ 1`` counts, and only if there is no 
additional ``+ 1`` outside.


https://reviews.llvm.org/D39121



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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 4 inline comments as done.
baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#902728, @alexfh wrote:

> Apart from all other comments, I think, this check would better be placed 
> under bugprone/.


I moved it there.


https://reviews.llvm.org/D39121



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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#902121, @martong wrote:

> Consider the use of a function pointer:
>
>   void* malloc(int);
>   int strlen(char*);
>   auto fp = malloc;
>   void bad_malloc(char *str) { char *c = (char *)fp(strlen(str + 1)); }
>
>
> I think, the checker will not match in this case.
>
> One might use allocation functions via a function pointer in case of more 
> possible allocation strategies (e.g having a different strategy for a shared 
> memory allocation).


Good point! But I think we should keep this first patch small, so I will do it 
in a follow-up patch.


https://reviews.llvm.org/D39121



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

[PATCH] D39121: [clang-tidy] Misplaced Operator in Strlen in Alloc

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D39121#902145, @Eugene.Zelenko wrote:

> Same mistake  could be made with new[] operator.


Yes! However, it seems that I need a new matcher for it so I would do it in a 
follow-up patch.


https://reviews.llvm.org/D39121



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

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Of course we would like to try the Z3 for refutation, we do not dispute its 
usefulness. This patch is about something really different. It extends the 
range-based constraint manager in a very natural way. Is the code of the 
range-based constraint manager frozen for some reason? If not, then why not add 
multiplication and division as well, where addition and subtraction is already 
supported?


https://reviews.llvm.org/D49074



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

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340805: [Analyzer] Iterator Checker - Part 3: Invalidation 
check, first for (copy)… (authored by baloghadamsoftware, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D32747

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp

Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -313,6 +313,10 @@
   HelpText<"Check for iterators used outside their valid ranges">,
   DescFile<"IteratorChecker.cpp">;
 
+def InvalidatedIteratorChecker : Checker<"InvalidatedIterator">,
+  HelpText<"Check for use of invalidated iterators">,
+  DescFile<"IteratorChecker.cpp">;
+
 def MisusedMovedObjectChecker: Checker<"MisusedMovedObject">,
  HelpText<"Method calls on a moved-from object and copying a moved-from "
   "object will be reported">,
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,15 @@
   return size_t(_finish - _start);
 }
 
+vector& operator=(const vector &other);
+vector& operator=(vector &&other);
+vector& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -301,8 +310,21 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -338,6 +360,15 @@
   return size_t(_finish - _start);
 }
 
+deque& operator=(const deque &other);
+deque& operator=(deque &&other);
+deque& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -351,11 +382,11 @@
 T &operator[](size_t n) {
   return _start[n];
 }
-
+
 const T &operator[](size_t n) const {
   return _start[n];
 }
-
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -367,7 +398,7 @@
 T& back() { return *(end() - 1); }
 const T& back() const { return *(end() - 1); }
   };
-  
+
   template
   class forward_list {
 struct __item {
@@ -387,6 +418,15 @@
 forward_list(forward_list &&other);
 ~forward_list();
 
+forward_list& operator=(const forward_list &other);
+forward_list& operator=(forward_list &&other);
+forward_list& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_front(const T &value);
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -ana

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Disabled? No, that was a different thing. I have some ideas there as well, how 
to solve it, but it was the rearrangement in the `SValBuilder`. This one is one 
level lower, the `ConstraintManager` which only calculate the range for a 
symbol where we have and assumption for it. Here operations such as `Symbol + 
Integer1 <= Integer2` have been supported for many years (`+` can be `-` as 
well, similarily we can replace `<=` with any other comparison operator). This 
patch adds support for `*` and `/` so it extends support for expressions like 
`Symbol * Integer0 + Integer1 <= Integer2`. It has marginal impact on the 
performance, a constant and surely not exponential.


https://reviews.llvm.org/D49074



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

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163517.
baloghadamsoftware removed a reviewer: george.karpenkov.
baloghadamsoftware added a comment.
Herald added a reviewer: george.karpenkov.

Since https://reviews.llvm.org/rL338263 fixed a bug in the cleanup phase the 
tests for mismatched iterator checker did not pass. The reason for this is that 
the region of some `LazyCompoundVal`s are cleaned up while there are still 
iterator positions connected to the `LazyCompoundVal` itself. This happens 
typically for arguments which are constructed in-place (e.g. `begin()` or 
`end()` of a container is invoked in the argument itself).

We applied a fix here that defers cleanup of such iterator positions. No other 
solution comes to my mind at the moment.

I wanted to upload this fix in a separate patch but I could not create tests 
for it.

@NoQ please review this fix before I commit the patch.


https://reviews.llvm.org/D32845

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/invalidated-iterator.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- /dev/null
+++ test/Analysis/mismatched-iterator.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void good_find(std::vector &v, int n) {
+  std::find(v.cbegin(), v.cend(), n); // no-warning
+}
+
+void good_find_first_of(std::vector &v1, std::vector &v2) {
+  std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v2.cend()); // no-warning
+}
+
+void good_copy(std::vector &v1, std::vector &v2, int n) {
+  std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
+}
+
+void bad_find(std::vector &v1, std::vector &v2, int n) {
+  std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_find_first_of(std::vector &v1, std::vector &v2) {
+  std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+  std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -642,6 +642,12 @@
   template 
   InputIterator find(InputIterator first, InputIterator last, const T &val);
 
+  template 
+  ForwardIterator1 find_first_of(ForwardIterator1 first1,
+ ForwardIterator1 last1,
+ ForwardIterator2 first2,
+ ForwardIterator2 last2);
+
   template 
   OutputIterator copy(InputIterator first, InputIterator last,
   OutputIterator result);
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163671.
baloghadamsoftware added a comment.

Minor changes.


https://reviews.llvm.org/D32845

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/invalidated-iterator.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- /dev/null
+++ test/Analysis/mismatched-iterator.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void good_find(std::vector &v, int n) {
+  std::find(v.cbegin(), v.cend(), n); // no-warning
+}
+
+void good_find_first_of(std::vector &v1, std::vector &v2) {
+  std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v2.cend()); // no-warning
+}
+
+void good_copy(std::vector &v1, std::vector &v2, int n) {
+  std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
+}
+
+void bad_find(std::vector &v1, std::vector &v2, int n) {
+  std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_find_first_of(std::vector &v1, std::vector &v2) {
+  std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+  std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -642,6 +642,12 @@
   template 
   InputIterator find(InputIterator first, InputIterator last, const T &val);
 
+  template 
+  ForwardIterator1 find_first_of(ForwardIterator1 first1,
+ ForwardIterator1 last1,
+ ForwardIterator2 first2,
+ ForwardIterator2 last2);
+
   template 
   OutputIterator copy(InputIterator first, InputIterator last,
   OutputIterator result);
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -198,6 +198,7 @@
  eval::Assume> {
 
   std::unique_ptr OutOfRangeBugType;
+  std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
 
   void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal,
@@ -221,16 +222,23 @@
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
   const SVal &RetVal, const SVal &LHS,
   const SVal &RHS) const;
+  void verifyMatch(CheckerContext &C, const SVal &Iter1,
+   const SVal &Iter2) const;
+
   void reportOutOfRangeBug(const StringRef &Message, const SVal &Val,
CheckerContext &C, ExplodedNode *ErrNode) const;
+  void reportM

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 3 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:317
 
+def MismatchedIteratorChecker : Checker<"MismatchedIterator">,
+  HelpText<"Check for use of iterators of different containers where iterators 
of the same container are expected">,

whisperity wrote:
> Is there any particular order entries of this file should be in? Seems to be 
> broken now, but I guess when this patch comes up to the top of the stack it 
> shall be fixed at a rebase.
Alphabetical order. Now it seems to be OK.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:382
 }
+  } else if (!isa(&Call)) {
+// The main purpose of iterators is to abstract away from different

baloghadamsoftware wrote:
> a.sidorin wrote:
> > The function becomes > 100 lines long. Should we refactor this check into a 
> > separate function to improve readability?
> Yes, I think so this would be a good idea. Should I do it now?
I propose to refactor it later in a separate patch.


https://reviews.llvm.org/D32845



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

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163724.
baloghadamsoftware added a comment.

Comments added, functions renamed.


https://reviews.llvm.org/D32859

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
   std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
 }
 
+void good_move_find1(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cbegin();
+  v1 = std::move(v2);
+  std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = --v2.cend();
+  v1 = std::move(v2);
+  std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cend();
+  v1 = std::move(v2);
+  v2.push_back(n);
+  std::find(v2.cbegin(), i0, n); // no-warning
+}
+
 void bad_find(std::vector &v1, std::vector &v2, int n) {
   std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
 
 void bad_find_first_of(std::vector &v1, std::vector &v2) {
   std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
   std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
+
+void bad_move_find1(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cbegin();
+  v1 = std::move(v2);
+  std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find2(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = --v2.cend();
+  v1 = std::move(v2);
+  std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find3(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cend();
+  v1 = std::move(v2);
+  std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
 return IteratorPosition(Cont, Valid, NewOf);
   }
 
+  IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+  }
+
   bool operator==(const IteratorPosition &X) const {
 return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
   }
@@ -218,7 +222,9 @@
  const SVal &Cont) const;
   void assignToContainer(CheckerContext &C, const Expr *CE, const SVal &RetVal,
  const MemRegion *Cont) const;
-  void handleAssign(CheckerContext &C, const SVal &Cont) const;
+  void handleAssign(CheckerContext &C, const SVal &Cont,
+const Expr *CE = nullptr,
+const SVal &OldCont = UndefinedVal()) const;
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
   const SVal &RetVal, const SVal &LHS,
   const SVal &RHS) const;
@@ -315,6 +321,17 @@
 bool Equal);
 ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+   const MemRegion *Cont,
+   const MemRegion *NewCont,
+   SymbolRef Offset,
+   BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder &SVB, SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
 const ContainerData *getContainerData(ProgramStateRef State,
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
 const auto Op = Func->getOverloadedOperator();
 if (isAssignmentOperator(Op)) {
   const auto *InstCall = dyn_cast(&Call);
-  handleAssign(C, InstCall-

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163725.
baloghadamsoftware added a comment.

) added.


https://reviews.llvm.org/D32859

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -15,11 +15,48 @@
   std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning
 }
 
+void good_move_find1(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cbegin();
+  v1 = std::move(v2);
+  std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find2(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = --v2.cend();
+  v1 = std::move(v2);
+  std::find(i0, v1.cend(), n); // no-warning
+}
+
+void good_move_find3(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cend();
+  v1 = std::move(v2);
+  v2.push_back(n);
+  std::find(v2.cbegin(), i0, n); // no-warning
+}
+
 void bad_find(std::vector &v1, std::vector &v2, int n) {
   std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
 
 void bad_find_first_of(std::vector &v1, std::vector &v2) {
   std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
   std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
+
+void bad_move_find1(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cbegin();
+  v1 = std::move(v2);
+  std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find2(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = --v2.cend();
+  v1 = std::move(v2);
+  std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_move_find3(std::vector &v1, std::vector &v2, int n) {
+  auto i0 = v2.cend();
+  v1 = std::move(v2);
+  std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
 return IteratorPosition(Cont, Valid, NewOf);
   }
 
+  IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+  }
+
   bool operator==(const IteratorPosition &X) const {
 return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
   }
@@ -218,7 +222,9 @@
  const SVal &Cont) const;
   void assignToContainer(CheckerContext &C, const Expr *CE, const SVal &RetVal,
  const MemRegion *Cont) const;
-  void handleAssign(CheckerContext &C, const SVal &Cont) const;
+  void handleAssign(CheckerContext &C, const SVal &Cont,
+const Expr *CE = nullptr,
+const SVal &OldCont = UndefinedVal()) const;
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
   const SVal &RetVal, const SVal &LHS,
   const SVal &RHS) const;
@@ -315,6 +321,17 @@
 bool Equal);
 ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+   const MemRegion *Cont,
+   const MemRegion *NewCont,
+   SymbolRef Offset,
+   BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder &SVB, SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
 const ContainerData *getContainerData(ProgramStateRef State,
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
 const auto Op = Func->getOverloadedOperator();
 if (isAssignmentOperator(Op)) {
   const auto *InstCall = dyn_cast(&Call);
-  handleAssign(C, InstCall->getCXXThisVal());
+  

[PATCH] D32860: [Analyzer] Iterator Checker - Part 6: Mismatched iterator checker for constructors and comparisons

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163736.
baloghadamsoftware added a comment.
Herald added subscribers: Szelethus, mikhail.ramalho.

Checking of constructor parameters moved to `PreCall` check.


https://reviews.llvm.org/D32860

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -3,6 +3,10 @@
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void good_ctor(std::vector &v) {
+  std::vector new_v(v.cbegin(), v.cend()); // no-warning
+}
+
 void good_find(std::vector &v, int n) {
   std::find(v.cbegin(), v.cend(), n); // no-warning
 }
@@ -34,6 +38,14 @@
   std::find(v2.cbegin(), i0, n); // no-warning
 }
 
+void good_comparison(std::vector &v) {
+  if (v.cbegin() == v.cend()) {} // no-warning
+}
+
+void bad_ctor(std::vector &v1, std::vector &v2) {
+  std::vector new_v(v1.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
 void bad_find(std::vector &v1, std::vector &v2, int n) {
   std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
@@ -60,3 +72,9 @@
   v1 = std::move(v2);
   std::find(v1.cbegin(), i0, n); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
+
+void bad_comparison(std::vector &v1, std::vector &v2) {
+  if (v1.cbegin() != v2.cend()) { // expected-warning{{Iterators of different containers used where the same container is expected}}
+*v1.cbegin();
+  }
+}
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -236,6 +236,9 @@
   void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
const SVal &Val2, CheckerContext &C,
ExplodedNode *ErrNode) const;
+  void reportMismatchedBug(const StringRef &Message, const SVal &Val,
+   const MemRegion *Reg, CheckerContext &C,
+   ExplodedNode *ErrNode) const;
   void reportInvalidatedBug(const StringRef &Message, const SVal &Val,
 CheckerContext &C, ExplodedNode *ErrNode) const;
 
@@ -276,6 +279,7 @@
 
 bool isIteratorType(const QualType &Type);
 bool isIterator(const CXXRecordDecl *CRD);
+bool isComparisonOperator(OverloadedOperatorKind OK);
 bool isBeginCall(const FunctionDecl *Func);
 bool isEndCall(const FunctionDecl *Func);
 bool isAssignmentOperator(OverloadedOperatorKind OK);
@@ -397,8 +401,48 @@
   } else {
 verifyDereference(C, Call.getArgSVal(0));
   }
+} else if (ChecksEnabled[CK_MismatchedIteratorChecker] &&
+   isComparisonOperator(Func->getOverloadedOperator())) {
+  // Check for comparisons of iterators of different containers
+  if (const auto *InstCall = dyn_cast(&Call)) {
+if (Call.getNumArgs() < 1)
+  return;
+
+if (!isIteratorType(InstCall->getCXXThisExpr()->getType()) ||
+!isIteratorType(Call.getArgExpr(0)->getType()))
+  return;
+
+verifyMatch(C, InstCall->getCXXThisVal(), Call.getArgSVal(0));
+  } else {
+if (Call.getNumArgs() < 2)
+  return;
+
+if (!isIteratorType(Call.getArgExpr(0)->getType()) ||
+!isIteratorType(Call.getArgExpr(1)->getType()))
+  return;
+
+verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));
+  }
 }
-  } else if (!isa(&Call)) {
+  } else if (isa(&Call)) {
+// Check match of first-last iterator pair in a constructor of a container
+if (Call.getNumArgs() < 2)
+  return;
+
+const auto *Ctr = cast(Call.getDecl());
+if (Ctr->getNumParams() < 2)
+  return;
+
+if (Ctr->getParamDecl(0)->getName() != "first" ||
+Ctr->getParamDecl(1)->getName() != "last")
+  return;
+
+if (!isIteratorType(Call.getArgExpr(0)->getType()) ||
+!isIteratorType(Call.getArgExpr(1)->getType()))
+  return;
+
+verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));
+  } else {
 // The main purpose of iterators is to abstract away from different
 // containers and provide a (maybe limited) uniform access to them.
 // This implies that any correctly written template function that
@@ -1102,6 +1146,16 @@
   C.emitReport(std::move(R));
 }
 
+void IteratorChecker::reportMismatchedBug(const StringRef &Message,
+  const SVal &Val, const MemRegion *Reg,
+  CheckerContext &C,
+  ExplodedNode *ErrNode) const {
+  auto R = llvm::make_

[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 163984.
baloghadamsoftware added a comment.

Updated according to the comments and rebased.


https://reviews.llvm.org/D32902

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -115,8 +115,66 @@
   }
 }
 
+void good_push_back(std::list &L, int n) {
+  auto i0 = --L.cend();
+  L.push_back(n);
+  *++i0; // no-warning
+}
+
+void bad_push_back(std::list &L, int n) {
+  auto i0 = --L.cend();
+  L.push_back(n);
+  ++i0;
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_pop_back(std::list &L, int n) {
+  auto i0 = --L.cend(); --i0;
+  L.pop_back();
+  *i0; // no-warning
+}
+
+void bad_pop_back(std::list &L, int n) {
+  auto i0 = --L.cend(); --i0;
+  L.pop_back();
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_push_front(std::list &L, int n) {
+  auto i0 = L.cbegin();
+  L.push_front(n);
+  *--i0; // no-warning
+}
+
+void bad_push_front(std::list &L, int n) {
+  auto i0 = L.cbegin();
+  L.push_front(n);
+  --i0;
+  *--i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void good_pop_front(std::list &L, int n) {
+  auto i0 = ++L.cbegin();
+  L.pop_front();
+  *i0; // no-warning
+}
+
+void bad_pop_front(std::list &L, int n) {
+  auto i0 = ++L.cbegin();
+  L.pop_front();
+  *--i0; // expected-warning{{Iterator accessed outside of its range}}
+}
+
 void bad_move(std::list &L1, std::list &L2) {
   auto i0 = --L2.cend();
   L1 = std::move(L2);
   *++i0; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+void bad_move_push_back(std::list &L1, std::list &L2, int n) {
+  auto i0 = --L2.cend();
+  L2.push_back(n);
+  L1 = std::move(L2);
+  ++i0;
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -30,3 +30,170 @@
   FL1 = FL2;
   *i0; // expected-warning{{Invalidated iterator accessed}}
 }
+
+void good_push_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend();
+  L.push_back(n);
+  *i0; // no-warning
+  --i1; // no-warning
+}
+
+void good_push_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.push_back(n);
+  *i0; // no-warning
+}
+
+void bad_push_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.push_back(n);
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_push_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend();
+  D.push_back(n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_emplace_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend();
+  L.emplace_back(n);
+  *i0; // no-warning
+  --i1; // no-warning
+}
+
+void good_emplace_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.emplace_back(n);
+  *i0; // no-warning
+}
+
+void bad_emplace_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.emplace_back(n);
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_emplace_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend();
+  D.emplace_back(n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+  L.pop_back();
+  *i0; // no-warning
+  *i2; // no-warning
+}
+
+void bad_pop_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+  L.pop_back();
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--;
+  V.pop_back();
+  *i0; // no-warning
+}
+
+void bad_pop_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--;
+  V.pop_back();
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+  --i2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend(), i2 = i1--;
+  D.pop_back();
+  *i0; // no-warning
+}
+
+void bad_pop_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend(), i2 = i1--;
+  D.pop_back();
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+  --

[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1449-1464
+const CXXRecordDecl *getCXXRecordDecl(const MemRegion *Reg) {
+  QualType Type;
+  if (const auto *TVReg = Reg->getAs()) {
+Type = TVReg->getValueType();
+  } else if (const auto *SymReg = Reg->getAs()) {
+Type = SymReg->getSymbol()->getType();
+  } else {

NoQ wrote:
> Would `getDynamicTypeInfo()` be of any help?
I changed the function to use `getDynamicTypeInfo()`, but now I had to include 
a `ProgramStateRef` parameter to the function and to all its callers so it did 
not become much more simple.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1477-1530
+bool isPushBackCall(const FunctionDecl *Func) {
+  const auto *IdInfo = Func->getIdentifier();
+  if (!IdInfo)
+return false;
+  if (Func->getNumParams() != 1)
+return false;
+  return IdInfo->getName() == "push_back";

NoQ wrote:
> I guess we should think if we want to use `CallDescription` for these when 
> D48027 lands.
Here we do not need the qualified name.


https://reviews.llvm.org/D32902



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

[PATCH] D32904: [Analyzer] Iterator Checker - Part 8: Support for assign, clear, insert, emplace and erase operations

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 164006.
baloghadamsoftware added a comment.
Herald added subscribers: Szelethus, mikhail.ramalho.

Rebased.


https://reviews.llvm.org/D32904

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp
  test/Analysis/mismatched-iterator.cpp

Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -3,6 +3,40 @@
 
 #include "Inputs/system-header-simulator-cxx.h"
 
+void good_insert1(std::vector &v, int n) {
+  v.insert(v.cbegin(), n); // no-warning
+}
+
+
+void good_insert2(std::vector &v, int len, int n) {
+  v.insert(v.cbegin(), len, n); // no-warning
+}
+
+void good_insert3(std::vector &v1, std::vector &v2) {
+  v1.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // no-warning
+}
+
+void good_insert4(std::vector &v, int len, int n) {
+  v.insert(v.cbegin(), {n-1, n, n+1}); // no-warning
+}
+
+void good_insert_find(std::vector &v, int n, int m) {
+  auto i = std::find(v.cbegin(), v.cend(), n);
+  v.insert(i, m); // no-warning
+}
+
+void good_erase1(std::vector &v) {
+  v.erase(v.cbegin()); // no-warning
+}
+
+void good_erase2(std::vector &v) {
+  v.erase(v.cbegin(), v.cend()); // no-warning
+}
+
+void good_emplace(std::vector &v, int n) {
+  v.emplace(v.cbegin(), n); // no-warning
+}
+
 void good_ctor(std::vector &v) {
   std::vector new_v(v.cbegin(), v.cend()); // no-warning
 }
@@ -25,6 +59,38 @@
   std::find(i0, v1.cend(), n); // no-warning
 }
 
+void bad_insert1(std::vector &v1, std::vector &v2, int n) {
+  v2.insert(v1.cbegin(), n); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void bad_insert2(std::vector &v1, std::vector &v2, int len, int n) {
+  v2.insert(v1.cbegin(), len, n); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void bad_insert3(std::vector &v1, std::vector &v2) {
+  v2.insert(v1.cbegin(), v2.cbegin(), v2.cend()); // expected-warning{{Container accessed using foreign iterator argument}}
+  v1.insert(v1.cbegin(), v1.cbegin(), v2.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+  v1.insert(v1.cbegin(), v2.cbegin(), v1.cend()); // expected-warning{{Iterators of different containers used where the same container is expected}}
+}
+
+void bad_insert4(std::vector &v1, std::vector &v2, int len, int n) {
+  v2.insert(v1.cbegin(), {n-1, n, n+1}); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void bad_erase1(std::vector &v1, std::vector &v2) {
+  v2.erase(v1.cbegin()); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void bad_erase2(std::vector &v1, std::vector &v2) {
+  v2.erase(v2.cbegin(), v1.cend()); // expected-warning{{Container accessed using foreign iterator argument}}
+  v2.erase(v1.cbegin(), v2.cend()); // expected-warning{{Container accessed using foreign iterator argument}}
+  v2.erase(v1.cbegin(), v1.cend()); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void bad_emplace(std::vector &v1, std::vector &v2, int n) {
+  v2.emplace(v1.cbegin(), n); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
 void good_move_find2(std::vector &v1, std::vector &v2, int n) {
   auto i0 = --v2.cend();
   v1 = std::move(v2);
@@ -61,6 +127,35 @@
   std::find(i0, v2.cend(), n); // expected-warning{{Iterators of different containers used where the same container is expected}}
 }
 
+void bad_insert_find(std::vector &v1, std::vector &v2, int n, int m) {
+  auto i = std::find(v1.cbegin(), v1.cend(), n);
+  v2.insert(i, m); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void good_overwrite(std::vector &v1, std::vector &v2, int n) {
+  auto i = v1.cbegin();
+  i = v2.cbegin();
+  v2.insert(i, n); // no-warning
+}
+
+void bad_overwrite(std::vector &v1, std::vector &v2, int n) {
+  auto i = v1.cbegin();
+  i = v2.cbegin();
+  v1.insert(i, n); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
+void good_move(std::vector &v1, std::vector &v2) {
+  const auto i0 = ++v2.cbegin();
+  v1 = std::move(v2);
+  v1.erase(i0); // no-warning
+}
+
+void bad_move(std::vector &v1, std::vector &v2) {
+  const auto i0 = ++v2.cbegin();
+  v1 = std::move(v2);
+  v2.erase(i0); // expected-warning{{Container accessed using foreign iterator argument}}
+}
+
 void bad_move_find2(std::vector &v1, std::vector &v2, int n) {
   auto i0 = --v2.cend();
   v1 = std::move(v2);
Index: test/Analysis/invalidated-iterator.cpp
===
--- test/Analysis/invalidated-iterator.cpp
+++ test/Analysis/invalidated-iterator.cpp
@@ -31,6 +31,56 @@
   *i0; // expecte

[PATCH] D32905: [Analyzer] Iterator Checker - Part 9: Evaluation of std::find-like calls

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.
Herald added subscribers: Szelethus, mikhail.ramalho.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1584-1588
+  auto stateFound = state->BindExpr(CE, LCtx, RetVal);
+  auto stateNotFound = state->BindExpr(CE, LCtx, SecondParam);
+
+  C.addTransition(stateFound);
+  C.addTransition(stateNotFound);

NoQ wrote:
> We discussed this in D25660 but i couldn't find what we decided upon. It 
> seems scary to me that in every situation, we declare that it is possible 
> that the element is not in the container. It should be allowed, even if not 
> necessarily efficient, to add the element to the container, and then use 
> `std::find` to obtain an iterator to it. So i think that this state split, 
> even if generally approved by your users, may need to stay under an analyzer 
> checker option.
Maybe instead of an option, I should put it into a separate checker that does 
not emit warnings just simulates these functions, like you did it in rC284960. 
The file name could be `StdCppLibraryFunctions.cpp`. Common functions and 
macros should then be moved into a header that both `StdLibraryFunctions.cpp` 
and `StdCppLibraryFunctions.cpp` includes.


https://reviews.llvm.org/D32905



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

[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 164152.
baloghadamsoftware added a comment.

Modification of the checker not needed anymore. Only tests added.


https://reviews.llvm.org/D32906

Files:
  test/Analysis/iterator-range.cpp
  test/Analysis/mismatched-iterator.cpp


Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -144,6 +144,19 @@
   v1.insert(i, n); // expected-warning{{Container accessed using foreign 
iterator argument}}
 }
 
+template
+bool is_cend(Container cont, Iterator it) {
+  return it == cont.cend();
+}
+
+void good_empty(std::vector &v) {
+  is_cend(v, v.cbegin()); // no-warning
+}
+
+void bad_empty(std::vector &v1, std::vector &v2) {
+  is_cend(v1, v2.cbegin()); // expected-warning@149{{Iterators of different 
containers used where the same container is expected}}
+}
+
 void good_move(std::vector &v1, std::vector &v2) {
   const auto i0 = ++v2.cbegin();
   v1 = std::move(v2);
Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -216,6 +216,11 @@
 *first; // no-warning
 }
 
+void bad_non_std_find(std::vector &V, int e) {
+  auto first = nonStdFind(V.begin(), V.end(), e);
+  *first; // expected-warning{{Iterator accessed outside of its range}}
+}
+
 void tricky(std::vector &V, int e) {
   const auto first = V.begin();
   const auto comp1 = (first != V.end()), comp2 = (first == V.end());


Index: test/Analysis/mismatched-iterator.cpp
===
--- test/Analysis/mismatched-iterator.cpp
+++ test/Analysis/mismatched-iterator.cpp
@@ -144,6 +144,19 @@
   v1.insert(i, n); // expected-warning{{Container accessed using foreign iterator argument}}
 }
 
+template
+bool is_cend(Container cont, Iterator it) {
+  return it == cont.cend();
+}
+
+void good_empty(std::vector &v) {
+  is_cend(v, v.cbegin()); // no-warning
+}
+
+void bad_empty(std::vector &v1, std::vector &v2) {
+  is_cend(v1, v2.cbegin()); // expected-warning@149{{Iterators of different containers used where the same container is expected}}
+}
+
 void good_move(std::vector &v1, std::vector &v2) {
   const auto i0 = ++v2.cbegin();
   v1 = std::move(v2);
Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -216,6 +216,11 @@
 *first; // no-warning
 }
 
+void bad_non_std_find(std::vector &V, int e) {
+  auto first = nonStdFind(V.begin(), V.end(), e);
+  *first; // expected-warning{{Iterator accessed outside of its range}}
+}
+
 void tricky(std::vector &V, int e) {
   const auto first = V.begin();
   const auto comp1 = (first != V.end()), comp2 = (first == V.end());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32906: [Analyzer] Iterator Checker - Part 10: Support for iterators passed as parameter

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Your are right, it is not necessary anymore. Should I keep this patch to add 
the tests only or should I abandon it completely?


https://reviews.llvm.org/D32906



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

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
baloghadamsoftware marked an inline comment as done.
Closed by commit rC341790: [Analyzer] Iterator Checker - Part 4: Mismatched 
iterator checker for function… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D32845?vs=163671&id=164631#toc

Repository:
  rC Clang

https://reviews.llvm.org/D32845

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/invalidated-iterator.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -198,6 +198,7 @@
  eval::Assume> {
 
   std::unique_ptr OutOfRangeBugType;
+  std::unique_ptr MismatchedBugType;
   std::unique_ptr InvalidatedBugType;
 
   void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal,
@@ -221,16 +222,23 @@
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
   const SVal &RetVal, const SVal &LHS,
   const SVal &RHS) const;
+  void verifyMatch(CheckerContext &C, const SVal &Iter1,
+   const SVal &Iter2) const;
+
   void reportOutOfRangeBug(const StringRef &Message, const SVal &Val,
CheckerContext &C, ExplodedNode *ErrNode) const;
+  void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
+   const SVal &Val2, CheckerContext &C,
+   ExplodedNode *ErrNode) const;
   void reportInvalidatedBug(const StringRef &Message, const SVal &Val,
 CheckerContext &C, ExplodedNode *ErrNode) const;
 
 public:
   IteratorChecker();
 
   enum CheckKind {
 CK_IteratorRangeChecker,
+CK_MismatchedIteratorChecker,
 CK_InvalidatedIteratorChecker,
 CK_NumCheckKinds
   };
@@ -312,14 +320,19 @@
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
 bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
+bool isBoundThroughLazyCompoundVal(const Environment &Env,
+   const MemRegion *Reg);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
 
 IteratorChecker::IteratorChecker() {
   OutOfRangeBugType.reset(
   new BugType(this, "Iterator out of range", "Misuse of STL APIs"));
   OutOfRangeBugType->setSuppressOnSink(true);
+  MismatchedBugType.reset(
+  new BugType(this, "Iterator(s) mismatched", "Misuse of STL APIs"));
+  MismatchedBugType->setSuppressOnSink(true);
   InvalidatedBugType.reset(
   new BugType(this, "Iterator invalidated", "Misuse of STL APIs"));
   InvalidatedBugType->setSuppressOnSink(true);
@@ -368,6 +381,65 @@
 verifyDereference(C, Call.getArgSVal(0));
   }
 }
+  } else if (!isa(&Call)) {
+// The main purpose of iterators is to abstract away from different
+// containers and provide a (maybe limited) uniform access to them.
+// This implies that any correctly written template function that
+// works on multiple containers using iterators takes different
+// template parameters for different containers. So we can safely
+// assume that passing iterators of different containers as arguments
+// whose type replaces the same template parameter is a bug.
+//
+// Example:
+// template
+// void f(I1 first1, I1 last1, I2 first2, I2 last2);
+// 
+// In this case the first two arguments to f() must be iterators must belong
+// to the same container and the last to also to the same container but
+// not neccessarily to the same as the first two.
+
+if (!ChecksEnabled[CK_MismatchedIteratorChecker])
+  return;
+
+const auto *Templ = Func->getPrimaryTemplate();
+if (!Templ)
+  return;
+
+const auto *TParams = Templ->getTemplateParameters();
+const auto *TArgs = Func->getTemplateSpecializationArgs();
+
+// Iterate over all the template parameters
+for (size_t I = 0; I < TParams->size(); ++I) {
+  const auto *TPDecl = dyn_cast(TParams->getParam(I));
+  if (!TPDecl)
+continue;
+
+  if (TPDecl->isParameterPack())
+continue;
+
+  const auto TAType = TArgs->get(I).getAsType();
+  if (!isIteratorType(TAType))
+continue;
+
+  SVal LHS = UndefinedVal();
+
+  // For every template parameter which is an iterator type in the
+  // instantiation look for all functions' parameters' type by it and
+  // check whether they belong to the same container
+  for (auto J = 0U; J < Func->getNumParams(); ++J

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341791: [Analyzer] Iterator Checker - Part 5: Move 
Assignment of Containers (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D32859?vs=163725&id=164632#toc

Repository:
  rC Clang

https://reviews.llvm.org/D32859

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -114,6 +114,10 @@
 return IteratorPosition(Cont, Valid, NewOf);
   }
 
+  IteratorPosition reAssign(const MemRegion *NewCont) const {
+return IteratorPosition(NewCont, Valid, Offset);
+  }
+
   bool operator==(const IteratorPosition &X) const {
 return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset;
   }
@@ -218,7 +222,9 @@
  const SVal &Cont) const;
   void assignToContainer(CheckerContext &C, const Expr *CE, const SVal &RetVal,
  const MemRegion *Cont) const;
-  void handleAssign(CheckerContext &C, const SVal &Cont) const;
+  void handleAssign(CheckerContext &C, const SVal &Cont,
+const Expr *CE = nullptr,
+const SVal &OldCont = UndefinedVal()) const;
   void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op,
   const SVal &RetVal, const SVal &LHS,
   const SVal &RHS) const;
@@ -315,6 +321,17 @@
 bool Equal);
 ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State,
const MemRegion *Cont);
+ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State,
+ const MemRegion *Cont,
+ const MemRegion *NewCont);
+ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State,
+   const MemRegion *Cont,
+   const MemRegion *NewCont,
+   SymbolRef Offset,
+   BinaryOperator::Opcode Opc);
+ProgramStateRef rebaseSymbolInIteratorPositionsIf(
+ProgramStateRef State, SValBuilder &SVB, SymbolRef OldSym,
+SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc);
 const ContainerData *getContainerData(ProgramStateRef State,
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
@@ -454,7 +471,12 @@
 const auto Op = Func->getOverloadedOperator();
 if (isAssignmentOperator(Op)) {
   const auto *InstCall = dyn_cast(&Call);
-  handleAssign(C, InstCall->getCXXThisVal());
+  if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
+handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),
+ Call.getArgSVal(0));
+  } else {
+handleAssign(C, InstCall->getCXXThisVal());
+  }
 } else if (isSimpleComparisonOperator(Op)) {
   if (const auto *InstCall = dyn_cast(&Call)) {
 handleComparison(C, Call.getReturnValue(), InstCall->getCXXThisVal(),
@@ -503,9 +525,6 @@
   return;
 
 auto State = C.getState();
-// Already bound to container?
-if (getIteratorPosition(State, Call.getReturnValue()))
-  return;
 
 if (const auto *InstCall = dyn_cast(&Call)) {
   if (isBeginCall(Func)) {
@@ -520,6 +539,10 @@
   }
 }
 
+// Already bound to container?
+if (getIteratorPosition(State, Call.getReturnValue()))
+  return;
+
 // Copy-like and move constructors
 if (isa(&Call) && Call.getNumArgs() == 1) {
   if (const auto *Pos = getIteratorPosition(State, Call.getArgSVal(0))) {
@@ -981,7 +1004,8 @@
   C.addTransition(State);
 }
 
-void IteratorChecker::handleAssign(CheckerContext &C, const SVal &Cont) const {
+void IteratorChecker::handleAssign(CheckerContext &C, const SVal &Cont,
+   const Expr *CE, const SVal &OldCont) const {
   const auto *ContReg = Cont.getAsRegion();
   if (!ContReg)
 return;
@@ -998,6 +1022,65 @@
 State = invalidateAllIteratorPositions(State, ContReg);
   }
 
+  // In case of move, iterators of the old container (except the past-end
+  // iterators) remain valid but refer to the new container
+  if (!OldCont.isUndef()) {
+const auto *OldContReg = OldCont.getAsRegion();
+if (OldContReg) {
+  while (const auto *CBOR = OldContReg->getAs()) {
+OldContReg = CBOR->getSuperRegion();
+  }
+  const auto OldCData = getContainerData(State, OldContReg);
+  if (OldCData) {
+if (const auto OldEndSym = OldCData->getE

[PATCH] D32860: [Analyzer] Iterator Checker - Part 6: Mismatched iterator checker for constructors and comparisons

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341792: [Analyzer] Iterator Checker - Part 6: Mismatched 
iterator checker for… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D32860?vs=163736&id=164635#toc

Repository:
  rC Clang

https://reviews.llvm.org/D32860

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp

Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -236,6 +236,9 @@
   void reportMismatchedBug(const StringRef &Message, const SVal &Val1,
const SVal &Val2, CheckerContext &C,
ExplodedNode *ErrNode) const;
+  void reportMismatchedBug(const StringRef &Message, const SVal &Val,
+   const MemRegion *Reg, CheckerContext &C,
+   ExplodedNode *ErrNode) const;
   void reportInvalidatedBug(const StringRef &Message, const SVal &Val,
 CheckerContext &C, ExplodedNode *ErrNode) const;
 
@@ -276,6 +279,7 @@
 
 bool isIteratorType(const QualType &Type);
 bool isIterator(const CXXRecordDecl *CRD);
+bool isComparisonOperator(OverloadedOperatorKind OK);
 bool isBeginCall(const FunctionDecl *Func);
 bool isEndCall(const FunctionDecl *Func);
 bool isAssignmentOperator(OverloadedOperatorKind OK);
@@ -397,8 +401,48 @@
   } else {
 verifyDereference(C, Call.getArgSVal(0));
   }
+} else if (ChecksEnabled[CK_MismatchedIteratorChecker] &&
+   isComparisonOperator(Func->getOverloadedOperator())) {
+  // Check for comparisons of iterators of different containers
+  if (const auto *InstCall = dyn_cast(&Call)) {
+if (Call.getNumArgs() < 1)
+  return;
+
+if (!isIteratorType(InstCall->getCXXThisExpr()->getType()) ||
+!isIteratorType(Call.getArgExpr(0)->getType()))
+  return;
+
+verifyMatch(C, InstCall->getCXXThisVal(), Call.getArgSVal(0));
+  } else {
+if (Call.getNumArgs() < 2)
+  return;
+
+if (!isIteratorType(Call.getArgExpr(0)->getType()) ||
+!isIteratorType(Call.getArgExpr(1)->getType()))
+  return;
+
+verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));
+  }
 }
-  } else if (!isa(&Call)) {
+  } else if (isa(&Call)) {
+// Check match of first-last iterator pair in a constructor of a container
+if (Call.getNumArgs() < 2)
+  return;
+
+const auto *Ctr = cast(Call.getDecl());
+if (Ctr->getNumParams() < 2)
+  return;
+
+if (Ctr->getParamDecl(0)->getName() != "first" ||
+Ctr->getParamDecl(1)->getName() != "last")
+  return;
+
+if (!isIteratorType(Call.getArgExpr(0)->getType()) ||
+!isIteratorType(Call.getArgExpr(1)->getType()))
+  return;
+
+verifyMatch(C, Call.getArgSVal(0), Call.getArgSVal(1));
+  } else {
 // The main purpose of iterators is to abstract away from different
 // containers and provide a (maybe limited) uniform access to them.
 // This implies that any correctly written template function that
@@ -1102,6 +1146,16 @@
   C.emitReport(std::move(R));
 }
 
+void IteratorChecker::reportMismatchedBug(const StringRef &Message,
+  const SVal &Val, const MemRegion *Reg,
+  CheckerContext &C,
+  ExplodedNode *ErrNode) const {
+  auto R = llvm::make_unique(*MismatchedBugType, Message, ErrNode);
+  R->markInteresting(Val);
+  R->markInteresting(Reg);
+  C.emitReport(std::move(R));
+}
+
 void IteratorChecker::reportInvalidatedBug(const StringRef &Message,
const SVal &Val, CheckerContext &C,
ExplodedNode *ErrNode) const {
@@ -1173,6 +1227,11 @@
  HasPostIncrOp && HasDerefOp;
 }
 
+bool isComparisonOperator(OverloadedOperatorKind OK) {
+  return OK == OO_EqualEqual || OK == OO_ExclaimEqual || OK == OO_Less ||
+ OK == OO_LessEqual || OK == OO_Greater || OK == OO_GreaterEqual;
+}
+
 bool isBeginCall(const FunctionDecl *Func) {
   const auto *IdInfo = Func->getIdentifier();
   if (!IdInfo)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32902: [Analyzer] Iterator Checker - Part 7: Support for push and pop operations

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
baloghadamsoftware marked an inline comment as done.
Closed by commit rL341793: [Analyzer] Iterator Checker - Part 7: Support for 
push and pop operations (authored by baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D32902?vs=163984&id=164636#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32902

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
  cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
  cfe/trunk/test/Analysis/invalidated-iterator.cpp
  cfe/trunk/test/Analysis/iterator-range.cpp

Index: cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -265,6 +265,8 @@
 
 void push_back(const T &value);
 void push_back(T &&value);
+template
+void emplace_back(Args&&... args);
 void pop_back();
 
 T &operator[](size_t n) {
@@ -319,10 +321,14 @@
 
 void push_back(const T &value);
 void push_back(T &&value);
+template
+void emplace_back(Args&&... args);
 void pop_back();
 
 void push_front(const T &value);
 void push_front(T &&value);
+template
+void emplace_front(Args&&... args);
 void pop_front();
 
 iterator begin() { return iterator(_start); }
@@ -373,10 +379,14 @@
 
 void push_back(const T &value);
 void push_back(T &&value);
+template
+void emplace_back(Args&&... args);
 void pop_back();
 
 void push_front(const T &value);
 void push_front(T &&value);
+template
+void emplace_front(Args&&... args);
 void pop_front();
 
 T &operator[](size_t n) {
@@ -431,6 +441,8 @@
 
 void push_front(const T &value);
 void push_front(T &&value);
+template
+void emplace_front(Args&&... args);
 void pop_front();
 
 iterator begin() { return iterator(_start); }
Index: cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
===
--- cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
+++ cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:566 {{Called C++ object pointer is null}}
 #endif
 }
Index: cfe/trunk/test/Analysis/invalidated-iterator.cpp
===
--- cfe/trunk/test/Analysis/invalidated-iterator.cpp
+++ cfe/trunk/test/Analysis/invalidated-iterator.cpp
@@ -30,3 +30,170 @@
   FL1 = FL2;
   *i0; // expected-warning{{Invalidated iterator accessed}}
 }
+
+void good_push_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend();
+  L.push_back(n);
+  *i0; // no-warning
+  --i1; // no-warning
+}
+
+void good_push_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.push_back(n);
+  *i0; // no-warning
+}
+
+void bad_push_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.push_back(n);
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_push_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend();
+  D.push_back(n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_emplace_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend();
+  L.emplace_back(n);
+  *i0; // no-warning
+  --i1; // no-warning
+}
+
+void good_emplace_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.emplace_back(n);
+  *i0; // no-warning
+}
+
+void bad_emplace_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.emplace_back(n);
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_emplace_back_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin(), i1 = D.cend();
+  D.emplace_back(n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+  L.pop_back();
+  *i0; // no-warning
+  *i2; // no-warning
+}
+
+void bad_pop_back_list1(std::list &L, int n) {
+  auto i0 = L.cbegin(), i1 = L.cend(), i2 = i1--;
+  L.pop_back();
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_pop_back_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin(), i1 = V.cend(), i2 = i1--;
+  V.pop_

[PATCH] D32904: [Analyzer] Iterator Checker - Part 8: Support for assign, clear, insert, emplace and erase operations

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341794: [Analyzer] Iterator Checker - Part 8: Support for 
assign, clear, insert… (authored by baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D32904?vs=164006&id=164637#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32904

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  cfe/trunk/test/Analysis/Inputs/system-header-simulator-cxx.h
  cfe/trunk/test/Analysis/diagnostics/explicit-suppression.cpp
  cfe/trunk/test/Analysis/invalidated-iterator.cpp

Index: cfe/trunk/test/Analysis/invalidated-iterator.cpp
===
--- cfe/trunk/test/Analysis/invalidated-iterator.cpp
+++ cfe/trunk/test/Analysis/invalidated-iterator.cpp
@@ -31,6 +31,56 @@
   *i0; // expected-warning{{Invalidated iterator accessed}}
 }
 
+void bad_assign_list1(std::list &L, int n) {
+  auto i0 = L.cbegin();
+  L.assign(10, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_assign_vector1(std::vector &V, int n) {
+  auto i0 = V.cbegin();
+  V.assign(10, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_assign_deque1(std::deque &D, int n) {
+  auto i0 = D.cbegin();
+  D.assign(10, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_assign_forward_list1(std::forward_list &FL, int n) {
+  auto i0 = FL.cbegin();
+  FL.assign(10, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_clear_list1(std::list &L) {
+  auto i0 = L.cend();
+  L.clear();
+  --i0; // no-warning
+}
+
+void bad_clear_list1(std::list &L) {
+  auto i0 = L.cbegin(), i1 = L.cend();
+  L.clear();
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_clear_vector1(std::vector &V) {
+  auto i0 = V.cbegin(), i1 = V.cend();
+  V.clear();
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_clear_deque1(std::deque &D) {
+  auto i0 = D.cbegin(), i1 = D.cend();
+  D.clear();
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  --i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
 void good_push_back_list1(std::list &L, int n) {
   auto i0 = L.cbegin(), i1 = L.cend();
   L.push_back(n);
@@ -197,3 +247,153 @@
   FL.pop_front();
   *i0; // expected-warning{{Invalidated iterator accessed}}
 }
+
+void good_insert_list1(std::list &L, int n) {
+  auto i1 = L.cbegin(), i0 = i1++;
+  L.insert(i1, n);
+  *i0; // no-warning
+  *i1; // no-warning
+}
+
+void good_insert_vector1(std::vector &V, int n) {
+  auto i1 = V.cbegin(), i0 = i1++;
+  V.insert(i1, n);
+  *i0; // no-warning
+}
+
+void bad_insert_vector1(std::vector &V, int n) {
+  auto i1 = V.cbegin(), i0 = i1++;
+  V.insert(i1, n);
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_insert_deque1(std::deque &D, int n) {
+  auto i1 = D.cbegin(), i0 = i1++;
+  D.insert(i1, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_emplace_list1(std::list &L, int n) {
+  auto i1 = L.cbegin(), i0 = i1++;
+  L.emplace(i1, n);
+  *i0; // no-warning
+  *i1; // no-warning
+}
+
+void good_emplace_vector1(std::vector &V, int n) {
+  auto i1 = V.cbegin(), i0 = i1++;
+  V.emplace(i1, n);
+  *i0; // no-warning
+}
+
+void bad_emplace_vector1(std::vector &V, int n) {
+  auto i1 = V.cbegin(), i0 = i1++;
+  V.emplace(i1, n);
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_emplace_deque1(std::deque &D, int n) {
+  auto i1 = D.cbegin(), i0 = i1++;
+  D.emplace(i1, n);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_erase_list1(std::list &L) {
+  auto i2 = L.cbegin(), i0 = i2++, i1 = i2++;
+  L.erase(i1);
+  *i0; // no-warning
+  *i2; // no-warning
+}
+
+void bad_erase_list1(std::list &L) {
+  auto i0 = L.cbegin();
+  L.erase(i0);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_erase_vector1(std::vector &V) {
+  auto i2 = V.cbegin(), i0 = i2++, i1 = i2++;
+  V.erase(i1);
+  *i0; // no-warning
+}
+
+void bad_erase_vector1(std::vector &V) {
+  auto i1 = V.cbegin(), i0 = i1++;
+  V.erase(i0);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_erase_deque1(std::deque &D) {
+  auto i2 = D.cbegin(), i0 = i2++, i1 = i2++;
+  D.erase(i1);
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+  *i1; // expected-warning{{Invalidated iterator accessed}}
+  *i2; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void good_erase_list2(std::list &L) {
+  auto i3 = L.cbegin(), i0 = i3++, i1 = i3++, i2 = i3++;
+  L.erase(i1, i3);
+  *i0; // n

[PATCH] D49536: [Analyzer] Quick Fix for exponential execution time when simpilifying complex additive expressions

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337678: [Analyzer] Quick Fix for exponential execution time 
when simpilifying complex… (authored by baloghadamsoftware, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49536?vs=156225&id=156737#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49536

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  cfe/trunk/test/Analysis/constraint_manager_negate_difference.c
  cfe/trunk/test/Analysis/iterator-range.cpp
  cfe/trunk/test/Analysis/plist-macros.cpp
  cfe/trunk/test/Analysis/svalbuilder-rearrange-comparisons.c

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -318,8 +318,8 @@
   /// \sa shouldDisplayNotesAsEvents
   Optional DisplayNotesAsEvents;
 
-  /// \sa shouldAggressivelySimplifyRelationalComparison
-  Optional AggressiveRelationalComparisonSimplification;
+  /// \sa shouldAggressivelySimplifyBinaryOperation
+  Optional AggressiveBinaryOperationSimplification;
 
   /// \sa getCTUDir
   Optional CTUDir;
@@ -690,19 +690,19 @@
   /// to false when unset.
   bool shouldDisplayNotesAsEvents();
 
-  /// Returns true if SValBuilder should rearrange comparisons of symbolic
-  /// expressions which consist of a sum of a symbol and a concrete integer
-  /// into the format where symbols are on the left-hand side and the integer
-  /// is on the right. This is only done if both symbols and both concrete
-  /// integers are signed, greater than or equal to the quarter of the minimum
-  /// value of the type and less than or equal to the quarter of the maximum
-  /// value of that type.
-  ///
-  /// A + n  B + m becomes A - B  m - n, where A and B symbolic,
-  /// n and m are integers.  is any of '==', '!=', '<', '<=', '>' or '>='.
-  /// The rearrangement also happens with '-' instead of '+' on either or both
-  /// side and also if any or both integers are missing.
-  bool shouldAggressivelySimplifyRelationalComparison();
+  /// Returns true if SValBuilder should rearrange comparisons and additive
+  /// operations of symbolic expressions which consist of a sum of a symbol and
+  /// a concrete integer into the format where symbols are on the left-hand
+  /// side and the integer is on the right. This is only done if both symbols
+  /// and both concrete integers are signed, greater than or equal to the
+  /// quarter of the minimum value of the type and less than or equal to the
+  /// quarter of the maximum value of that type.
+  ///
+  /// A + n  B + m becomes A - B  m - n, where A and B symbolic,
+  /// n and m are integers.  is any of '==', '!=', '<', '<=', '>', '>=',
+  /// '+' or '-'. The rearrangement also happens with '-' instead of '+' on
+  // either or both side and also if any or both integers are missing.
+  bool shouldAggressivelySimplifyBinaryOperation();
 
   /// Returns the directory containing the CTU related files.
   StringRef getCTUDir();
Index: cfe/trunk/test/Analysis/iterator-range.cpp
===
--- cfe/trunk/test/Analysis/iterator-range.cpp
+++ cfe/trunk/test/Analysis/iterator-range.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
Index: cfe/trunk/test/Analysis/plist-macros.cpp
===
--- cfe/trunk/test/Analysis/plist-macros.cpp
+++ cfe/trunk/test/Analysis/plist-macros.cpp
@@ -637,6 +637,69 @@
 // CHECK-NEXT: end
 // CHECK-NEXT:  
 // CHECK-NEXT:   
+// CHECK-NEXT:line36
+// CHECK-NEXT: 

[PATCH] D49627: [CFG] [analyzer] Constructors of member CXXOperatorCallExpr's argument 0 are not argument constructors.

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

"Actually supporting such this-argument construction contexts would address the 
FIXME that we've added in https://reviews.llvm.org/D32642 but this patch 
doesn't go that far."

Maybe a FIXME here so we do not forget to continue it in the future?


https://reviews.llvm.org/D49627



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

[PATCH] D49627: [CFG] [analyzer] Constructors of member CXXOperatorCallExpr's argument 0 are not argument constructors.

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

How much different is a correct `this`-argument construction context from real 
argument construction contexts?


https://reviews.llvm.org/D49627



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

[PATCH] D48427: [Analyzer] Iterator Checker Hotfix: Defer deletion of container data until its last iterator is cleaned up

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC338234: [Analyzer] Iterator Checker Hotfix: Defer deletion 
of container data until its… (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D48427?vs=154066&id=157929#toc

Repository:
  rC Clang

https://reviews.llvm.org/D48427

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -536,7 +537,11 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  // We must keep the container data while it has live iterators to be able
+  // to compare them to the begin and the end of the container.
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1188,6 +1193,22 @@
   return NewState;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
   auto &BVF = State->getBasicVals();
   return compare(State, Val,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -291,6 +291,7 @@
   const MemRegion *Cont);
 ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont,
  const ContainerData &CData);
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont);
 bool isOutOfRange(ProgramStateRef State, const IteratorPosition &Pos);
 bool isZero(ProgramStateRef State, const NonLoc &Val);
 } // namespace
@@ -536,7 +537,11 @@
   auto ContMap = State->get();
   for (const auto Cont : ContMap) {
 if (!SR.isLiveRegion(Cont.first)) {
-  State = State->remove(Cont.first);
+  // We must keep the container data while it has live iterators to be able
+  // to compare them to the begin and the end of the container.
+  if (!hasLiveIterators(State, Cont.first)) {
+State = State->remove(Cont.first);
+  }
 }
   }
 
@@ -1188,6 +1193,22 @@
   return NewState;
 }
 
+bool hasLiveIterators(ProgramStateRef State, const MemRegion *Cont) {
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+if (Reg.second.getContainer() == Cont)
+  return true;
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+if (Sym.second.getContainer() == Cont)
+  return true;
+  }
+
+  return false;
+}
+
 bool isZero(ProgramStateRef State, const NonLoc &Val) {
   auto &BVF = State->getBasicVals();
   return compare(State, Val,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked an inline comment as not done.
baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+  if (Pos && !Pos->isValid()) {
+// If I do not put a tag here, some invalidation tests will fail
+static CheckerProgramPointTag Tag("InvalidatedIteratorChecker",

NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > This needs investigation, because it may be nasty.
> > > 
> > > `generateNonFatalErrorNode()` returns null when the exact same non-fatal 
> > > error node, also produced by the iterator checker with the exact same 
> > > program state and exact same program point and exact same tag on the 
> > > program point already exists. As far as i understand, the only difference 
> > > your tag makes is that the tag is now different, so it is not merged with 
> > > the existing node. However, it is worth it to try to find out why the 
> > > node gets merged at all.
> > > 
> > > This may be caused by an accidental state split. For example, if you are 
> > > calling `generateNonFatalErrorNode()` twice in the same checker callback 
> > > without chaining them together (passing node returned by one as an 
> > > argument to another), this in fact splits states. I'm not immediately 
> > > seeing such places in the code - you seem to be aware of this problem and 
> > > avoiding it well. But still, looking at the topology of the exploded 
> > > graph in the failing test should help finding out what is going on.
> > I made some more investigation this time. Unfortunately the case is not 
> > what you suggest. Only one non-fatal error node is produced. I tested it 
> > with a common tag (a global static so the tag is exactly the same at every 
> > `generateNonFatalErrorNode()`, but the tests still pass. I printed out the 
> > exploded graph and I found that there are indeed two nodes with the same 
> > state ID. The tag is the default tag automatically generated from the name 
> > of the checker. The first state is created in function 
> > `checkPreStatement()` for `CXXOperatorCallExpr` where I copy the state of 
> > the iterator from the formal to the actual `this` parameter. All the test 
> > fails happen at the dereference operator call (`*`) of another operator 
> > call (`++` or `--`). After this copy, when I call 
> > `generateNonFatalErrorNode()` I get `nullptr` because at some point under 
> > the hood (I debugged it when I created it originally) the error node is 
> > considered as "not new". If I use a custom tag here, the state ID remains, 
> > not the node ID changes.
> I think i see the problem. The checker subscribes to both `PreCall` and 
> `PreStmt` (to be exact, `CXXOperatorCallExpr`) and adds transitions 
> in both cases. It results with a predecessor node in `CheckerContext` that's 
> already tagged by the checker. Apparently this never worked, but nobody tried 
> that.
> 
> Ideally, we should make sure those callbacks use different program points, 
> eg. introduce `PreCall`/`PostCall` program point kinds and use them.
> 
> Also i wonder why are you using pre- rather than post-statement callback. You 
> model all other operators in `PostCall`, why did those end up here? Maybe 
> merge them? It is generally better to model pre-conditions and look for bugs 
> in `PreStmt`/`PreCall` (before we don't care what happens within the call), 
> and model effects in `PostStmt`/`PostCall` (because effects don't take effect 
> until the call happens).
That is what I am trying to to: `Post*` for modelling and `Pre*` for checking. 
However, this `PreStmt()` is special since I have to move 
the arguments to the context of the called operator //before// the call.


https://reviews.llvm.org/D32747



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

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 158479.
baloghadamsoftware added a comment.

Pre-statement check of `CXXOperatorCallExpr` merged into pre-call check.


https://reviews.llvm.org/D32747

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp

Index: test/Analysis/invalidated-iterator.cpp
===
--- /dev/null
+++ test/Analysis/invalidated-iterator.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void bad_copy_assign_operator_list1(std::list &L1,
+const std::list &L2) {
+  auto i0 = L1.cbegin();
+  L1 = L2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_vector1(std::vector &V1,
+  const std::vector &V2) {
+  auto i0 = V1.cbegin();
+  V1 = V2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_deque1(std::deque &D1,
+ const std::deque &D2) {
+  auto i0 = D1.cbegin();
+  D1 = D2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_forward_list1(std::forward_list &FL1,
+const std::forward_list &FL2) {
+  auto i0 = FL1.cbegin();
+  FL1 = FL2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,15 @@
   return size_t(_finish - _start);
 }
 
+vector& operator=(const vector &other);
+vector& operator=(vector &&other);
+vector& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -301,8 +310,21 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -338,6 +360,15 @@
   return size_t(_finish - _start);
 }
 
+deque& operator=(const deque &other);
+deque& operator=(deque &&other);
+deque& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -351,11 +382,11 @@
 T &operator[](size_t n) {
   return _start[n];
 }
-
+
 const T &operator[](size_t n) const {
   return _start[n];
 }
-
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -367,7 +398,7 @@
 T& back() { return *(end() - 1); }
 const T& back() const { return *(end() - 1); }
   };
-  
+
   templa

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:605
+  if (Pos && !Pos->isValid()) {
+// If I do not put a tag here, some invalidation tests will fail
+static CheckerProgramPointTag Tag("InvalidatedIteratorChecker",

NoQ wrote:
> baloghadamsoftware wrote:
> > NoQ wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > This needs investigation, because it may be nasty.
> > > > > 
> > > > > `generateNonFatalErrorNode()` returns null when the exact same 
> > > > > non-fatal error node, also produced by the iterator checker with the 
> > > > > exact same program state and exact same program point and exact same 
> > > > > tag on the program point already exists. As far as i understand, the 
> > > > > only difference your tag makes is that the tag is now different, so 
> > > > > it is not merged with the existing node. However, it is worth it to 
> > > > > try to find out why the node gets merged at all.
> > > > > 
> > > > > This may be caused by an accidental state split. For example, if you 
> > > > > are calling `generateNonFatalErrorNode()` twice in the same checker 
> > > > > callback without chaining them together (passing node returned by one 
> > > > > as an argument to another), this in fact splits states. I'm not 
> > > > > immediately seeing such places in the code - you seem to be aware of 
> > > > > this problem and avoiding it well. But still, looking at the topology 
> > > > > of the exploded graph in the failing test should help finding out 
> > > > > what is going on.
> > > > I made some more investigation this time. Unfortunately the case is not 
> > > > what you suggest. Only one non-fatal error node is produced. I tested 
> > > > it with a common tag (a global static so the tag is exactly the same at 
> > > > every `generateNonFatalErrorNode()`, but the tests still pass. I 
> > > > printed out the exploded graph and I found that there are indeed two 
> > > > nodes with the same state ID. The tag is the default tag automatically 
> > > > generated from the name of the checker. The first state is created in 
> > > > function `checkPreStatement()` for `CXXOperatorCallExpr` where I copy 
> > > > the state of the iterator from the formal to the actual `this` 
> > > > parameter. All the test fails happen at the dereference operator call 
> > > > (`*`) of another operator call (`++` or `--`). After this copy, when I 
> > > > call `generateNonFatalErrorNode()` I get `nullptr` because at some 
> > > > point under the hood (I debugged it when I created it originally) the 
> > > > error node is considered as "not new". If I use a custom tag here, the 
> > > > state ID remains, not the node ID changes.
> > > I think i see the problem. The checker subscribes to both `PreCall` and 
> > > `PreStmt` (to be exact, `CXXOperatorCallExpr`) and adds 
> > > transitions in both cases. It results with a predecessor node in 
> > > `CheckerContext` that's already tagged by the checker. Apparently this 
> > > never worked, but nobody tried that.
> > > 
> > > Ideally, we should make sure those callbacks use different program 
> > > points, eg. introduce `PreCall`/`PostCall` program point kinds and use 
> > > them.
> > > 
> > > Also i wonder why are you using pre- rather than post-statement callback. 
> > > You model all other operators in `PostCall`, why did those end up here? 
> > > Maybe merge them? It is generally better to model pre-conditions and look 
> > > for bugs in `PreStmt`/`PreCall` (before we don't care what happens within 
> > > the call), and model effects in `PostStmt`/`PostCall` (because effects 
> > > don't take effect until the call happens).
> > That is what I am trying to to: `Post*` for modelling and `Pre*` for 
> > checking. However, this `PreStmt()` is special since I 
> > have to move the arguments to the context of the called operator //before// 
> > the call.
> Ah, it's that thing, `PreCall` then?
Yes! It seems to be working. Thank you for your help!


https://reviews.llvm.org/D32747



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

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 158701.
baloghadamsoftware added a comment.

Copying iterator position for the `this` pointer of C++ operators from the 
caller's context to the callee's context is no longer needed.


https://reviews.llvm.org/D32747

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/invalidated-iterator.cpp

Index: test/Analysis/invalidated-iterator.cpp
===
--- /dev/null
+++ test/Analysis/invalidated-iterator.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.InvalidatedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void bad_copy_assign_operator_list1(std::list &L1,
+const std::list &L2) {
+  auto i0 = L1.cbegin();
+  L1 = L2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_vector1(std::vector &V1,
+  const std::vector &V2) {
+  auto i0 = V1.cbegin();
+  V1 = V2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_deque1(std::deque &D1,
+ const std::deque &D2) {
+  auto i0 = D1.cbegin();
+  D1 = D2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
+
+void bad_copy_assign_operator_forward_list1(std::forward_list &FL1,
+const std::forward_list &FL2) {
+  auto i0 = FL1.cbegin();
+  FL1 = FL2;
+  *i0; // expected-warning{{Invalidated iterator accessed}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:554 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,15 @@
   return size_t(_finish - _start);
 }
 
+vector& operator=(const vector &other);
+vector& operator=(vector &&other);
+vector& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -301,8 +310,21 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -338,6 +360,15 @@
   return size_t(_finish - _start);
 }
 
+deque& operator=(const deque &other);
+deque& operator=(deque &&other);
+deque& operator=(std::initializer_list ilist);
+
+void assign(size_type count, const T &value);
+template 
+void assign(InputIterator first, InputIterator last);
+void assign(std::initializer_list ilist);
+
 void clear();
 
 void push_back(const T &value);
@@ -351,11 +382,11 @@
 T &operator[](size_t n) {
   return _start[n];
 }
-
+
 const T &operator[](size_t n) const {
   return _start[n];
 }
-
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -367,7 +398,7 @@
 T& back() { return *(end() - 1); }
 con

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Yes, it is working now without the hack.


https://reviews.llvm.org/D32747



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

[PATCH] D50256: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager (for == and != only)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, rnkovacs, szepet, 
whisperity.
Herald added a reviewer: george.karpenkov.

Currently, the default (range-based) constrain manager supports symbolic 
expressions of the format "symbol + integer" or "symbol - integer" to compare 
them to another integer. However, multiplication and division is not supported 
yet.

An example where multiplication (and division) could be useful is the checking 
whether a pointer operation or an array index is out of the bounds of the 
memory region. The index is often a first-degree expression of the format a*x+b 
(e.g. for special sparse such as triangular matrices).

In this patch we only support multiplication and division for two kinds of 
symbolic comparisons: the `==` and the `!=` operators. The rest is to follow in 
a subsequent patch. Negative multipliers and divisors are not supported yet.


https://reviews.llvm.org/D50256

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- /dev/null
+++ test/Analysis/multiplicative-folding.c
@@ -0,0 +1,577 @@
+// RUN: %clang_analyze_cc1 --std=c11 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached(void);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (int)(UINT_MAX & (UINT_MAX >> 1))
+#define INT_MIN (-INT_MAX - 1)
+
+#define ULONG_LONG_MAX (~0UL)
+#define LONG_LONG_MAX (long long)(ULONG_LONG_MAX & (ULONG_LONG_MAX >> 1))
+#define LONG_LONG_MIN (-LONG_LONG_MAX - 1)
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+typedef int int32_t;
+typedef unsigned int uint32_t;
+typedef long long int64_t;
+typedef unsigned long long uint64_t;
+
+void signed_multiplication_eq(int32_t n) {
+  if (n * 2 == 3) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 2 == 4) {
+const int32_t C1 = 0x8002, C2 = 2;
+
+assert(C1 * 2 == 4);
+assert(C2 * 2 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 3 == 4) {
+const int32_t C1 = 0xaaac;
+
+assert(C1 * 3 == 4);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{TRUE}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 4 == -5) {
+clang_analyzer_warnIfReached(); // no-warning
+
+  } else if (n * 4 == -8) {
+const int32_t C1 = 0xbffe, C2 = 0xfffe,
+  C3 = 0x3ffe, C4 = 0x7ffe;
+
+assert(C1 * 4 == -8);
+assert(C2 * 4 == -8);
+assert(C3 * 4 == -8);
+assert(C4 * 4 == -8);
+
+clang_analyzer_eval(n == INT_MIN); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C1); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C1 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C2); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C2 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == 0); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C3); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C3 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4 - 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == C4); //expected-warning{{UNKNOWN}}
+clang_analyzer_eval(n == C4 + 1); //expected-warning{{FALSE}}
+clang_analyzer_eval(n == INT_MAX); //expected-warning{{FALSE}}
+
+  } else if (n * 6 == -7) {
+clang_analyzer_warnIfReached(); /

[PATCH] D49074: [Analyzer] [WIP] Basic support for multiplication and division in the constraint manager

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 159012.
baloghadamsoftware edited the summary of this revision.
baloghadamsoftware added a comment.

Completely rewritten: works correctly for modular arithmetic (multiplication), 
works correctly for truncation (division), only uses integers, no floats.


https://reviews.llvm.org/D49074

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
  test/Analysis/multiplicative-folding.c

Index: test/Analysis/multiplicative-folding.c
===
--- test/Analysis/multiplicative-folding.c
+++ test/Analysis/multiplicative-folding.c
@@ -467,6 +467,1442 @@
   }
 }
 
+void signed_multiplication_lt_0(int32_t n) {
+  if (n * 2 < 3) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 3);
+assert(U1 * 2 < 3);
+assert((U1 + 1) * 2 >= 3);
+assert(L2 * 2 < 3);
+assert((L2 - 1) * 2 >= 3);
+assert(U2 * 2 < 3);
+assert((U2 + 1) * 2 >= 3);
+assert(L3 * 2 < 3);
+assert((L3 - 1) * 2 >= 3);
+assert(INT_MAX * 2 < 3);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_1(int32_t n) {
+  if (n * 2 < 4) {
+int32_t  U1 = 0x8001,
+L2 = 0xc000, U2 = 1,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 4);
+assert(U1 * 2 < 4);
+assert((U1 + 1) * 2 >= 4);
+assert(L2 * 2 < 4);
+assert((L2 - 1) * 2 >= 4);
+assert(U2 * 2 < 4);
+assert((U2 + 1) * 2 >= 4);
+assert(L3 * 2 < 4);
+assert((L3 - 1) * 2 >= 4);
+assert(INT_MAX * 2 < 4);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_2(int32_t n) {
+  if (n * 2 < 5) {
+int32_t  U1 = 0x8002,
+L2 = 0xc000, U2 = 2,
+L3 = 0x4000;
+
+assert(INT_MIN * 2 < 5);
+assert(U1 * 2 < 5);
+assert((U1 + 1) * 2 >= 5);
+assert(L2 * 2 < 5);
+assert((L2 - 1) * 2 >= 5);
+assert(U2 * 2 < 5);
+assert((U2 + 1) * 2 >= 5);
+assert(L3 * 2 < 5);
+assert((L3 - 1) * 2 >= 5);
+assert(INT_MAX * 2 < 5);
+
+if (n < INT_MIN / 2) {
+  clang_analyzer_eval(n == INT_MIN); //expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(n <= U1); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U1); //expected-warning{{FALSE}}
+} else if (n < INT_MAX / 2){
+  clang_analyzer_eval(n < L2); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n <= U2); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n > U2); //expected-warning{{FALSE}}
+} else {
+  clang_analyzer_eval(n < L3); //expected-warning{{FALSE}}
+  clang_analyzer_eval(n >= L3); //expected-warning{{TRUE}}
+  clang_analyzer_eval(n == INT_MAX); //expected-warning{{UNKNOWN}}
+}
+  }
+}
+
+void signed_multiplication_lt_3(int32_t n) {
+  if (n * 3 < 4) {
+int32_t  U1 = 0xaaab,
+L2 = 0xd556, U2 = 1,
+L3 = 0x2aab, U3 = 0x5556;
+
+assert(INT_MIN * 3 < 4);
+assert(U1 * 3 < 4);
+assert((U1 + 1) * 3 >= 4);
+assert(L2 * 3 < 4);
+assert((L2 - 1) * 3 >= 4);
+assert(U2 * 3 < 4);
+assert((U2 + 1) * 3 >= 4);
+assert(L3 * 3 < 4);
+assert((L3 - 1) * 3 >= 4);
+assert(U3 * 3 < 4);
+assert((U3 + 1) * 3 >= 4);
+
+if (n <

[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:388
+// template parameters for different containers. So we can safely
+// assume that passing iterators of different containers as arguments
+// whose type replaces the same template parameter is a bug.

baloghadamsoftware wrote:
> a.sidorin wrote:
> > While this assumption is sane and is true for  functions, user 
> > code can have other design solutions. There is nothing that prevents users 
> > from writing a function looking like:
> > ```
> > template 
> > void f(IterTy FromBegin, IterTy FromEnd, IterTy ToBegin, IterTy ToEnd);
> > ```
> > and there is nothing wrong with it.
> > One of  possible solutions is to restrict checker to check only functions 
> > from std namespace. What do you think?
> We can restrict, of course, but first we should measure how it performs on 
> real code. With the restriction, we can get rid of some false positives but 
> we may also loose some true positives.
One more thing:

The main purpose of iterators is to make algorithms independent of the data 
representation. So you can decide whether your algorithm works on a specific 
representation and create non-template function that takes reference for the 
specific container itself or you make it generic so you use template function 
which takes iterators. If you chose this latter one for an algorithm that works 
on two different container then there is no point to restrict the function to 
only work on two containers with exactly the same representation. Either 
specific or generic, but there is no point for something in between.


https://reviews.llvm.org/D32845



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

[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D32859##inline-360206, @NoQ wrote:

> I do not immediately understand what is this useful for. At least tests don't 
> look like they make use of these offset manipulations(?)
>
> Without full understanding, i wonder: when we overwrite one container with 
> another, why don't we just overwrite all symbols associated with it, instead 
> of creating a mixture of old and new symbols?
>
> Or maybe this is an accidental part of another patch, that has something to 
> do with resizes?


I do not see which lines exactly you commented but I suppose it is about not 
reassigning all iterator positions to the new container upon moving. According 
to [[ C++ Reference | en.cppreference.com/w/cpp/container/vector/operator%3D ]] 
the past-end iterators are not moved to the new container.


https://reviews.llvm.org/D32859



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

[PATCH] D32747: [Analyzer] Iterator Checker - Part 3: Invalidation check, first for (copy) assignments

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.




https://reviews.llvm.org/D32747



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
Herald added a subscriber: mgorny.

Finds functions which should not throw exceptions: Destructors, move 
constructors, move assignment operators, the main() function, swap() functions, 
functions marked with throw() or noexcept and functions given as option to the 
checker.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Supersedes https://reviews.llvm.org/D32350


https://reviews.llvm.org/D33537



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

[PATCH] D32350: [Analyzer] Exception checker for misuse: uncaught/noncompliant throws

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware abandoned this revision.
baloghadamsoftware added a comment.

Superseded by https://reviews.llvm.org/D33537


https://reviews.llvm.org/D32350



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

[PATCH] D32592: [Analyzer] Iterator Checker - Part 1: Minimal Checker for a Simple Test Case

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Any comments regarding the last changes?


https://reviews.llvm.org/D32592



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

[PATCH] D32592: [Analyzer] Iterator Checker - Part 1: Minimal Checker for a Simple Test Case

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100239.
baloghadamsoftware added a comment.

Comments added and fixed.


https://reviews.llvm.org/D32592

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/CMakeLists.txt
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  lib/StaticAnalyzer/Checkers/IteratorPastEndChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-past-end.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- /dev/null
+++ test/Analysis/iterator-range.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void clang_analyzer_warnIfReached();
+
+void simple_good_end(const std::vector &v) {
+  auto i = v.end();
+  if (i != v.end()) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+  }
+}
+
+void simple_bad_end(const std::vector &v) {
+  auto i = v.end();
+  *i; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/iterator-past-end.cpp
===
--- test/Analysis/iterator-past-end.cpp
+++ /dev/null
@@ -1,205 +0,0 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorPastEnd -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorPastEnd -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
-
-#include "Inputs/system-header-simulator-cxx.h"
-
-void simple_good(const std::vector &v) {
-  auto i = v.end();
-  if (i != v.end())
-*i; // no-warning
-}
-
-void simple_good_negated(const std::vector &v) {
-  auto i = v.end();
-  if (!(i == v.end()))
-*i; // no-warning
-}
-
-void simple_bad(const std::vector &v) {
-  auto i = v.end();
-  *i; // expected-warning{{Iterator accessed past its end}}
-}
-
-void copy(const std::vector &v) {
-  auto i1 = v.end();
-  auto i2 = i1;
-  *i2; // expected-warning{{Iterator accessed past its end}}
-}
-
-void decrease(const std::vector &v) {
-  auto i = v.end();
-  --i;
-  *i; // no-warning
-}
-
-void copy_and_decrease1(const std::vector &v) {
-  auto i1 = v.end();
-  auto i2 = i1;
-  --i1;
-  *i1; // no-warning
-}
-
-void copy_and_decrease2(const std::vector &v) {
-  auto i1 = v.end();
-  auto i2 = i1;
-  --i1;
-  *i2; // expected-warning{{Iterator accessed past its end}}
-}
-
-void copy_and_increase1(const std::vector &v) {
-  auto i1 = v.begin();
-  auto i2 = i1;
-  ++i1;
-  if (i1 == v.end())
-*i2; // no-warning
-}
-
-void copy_and_increase2(const std::vector &v) {
-  auto i1 = v.begin();
-  auto i2 = i1;
-  ++i1;
-  if (i2 == v.end())
-*i2; // expected-warning{{Iterator accessed past its end}}
-}
-
-void good_find(std::vector &vec, int e) {
-  auto first = std::find(vec.begin(), vec.end(), e);
-  if (vec.end() != first)
-*first; // no-warning
-}
-
-void bad_find(std::vector &vec, int e) {
-  auto first = std::find(vec.begin(), vec.end(), e);
-  *first; // expected-warning{{Iterator accessed past its end}}
-}
-
-void good_find_end(std::vector &vec, std::vector &seq) {
-  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
-  if (vec.end() != last)
-*last; // no-warning
-}
-
-void bad_find_end(std::vector &vec, std::vector &seq) {
-  auto last = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
-  *last; // expected-warning{{Iterator accessed past its end}}
-}
-
-void good_find_first_of(std::vector &vec, std::vector &seq) {
-  auto first =
-  std::find_first_of(vec.begin(), vec.end(), seq.begin(), seq.end());
-  if (vec.end() != first)
-*first; // no-warning
-}
-
-void bad_find_first_of(std::vector &vec, std::vector &seq) {
-  auto first = std::find_end(vec.begin(), vec.end(), seq.begin(), seq.end());
-  *first; // expected-warning{{Iterator accessed past its end}}
-}
-
-bool odd(int i) { return i % 2; }
-
-void good_find_if(std::vector &vec) {
-  auto first = std::find_if(vec.begin(), vec.end(), odd);
-  if (vec.end() != first)
-*first; // no-warning
-}
-
-void bad_find_if(std::vector &vec, int e) {
-  auto first = std::find_if(vec.begin(), vec.end(), odd);
-  *first; // expected-warning{{Iterator accessed past its end}}
-}
-
-void good_find_if_not(std::vector &vec) {
-  auto first = std::find_if_not(vec.begin(), vec.end(), odd);
-  if (vec.end() != first)
-*first; // no-w

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100369.
baloghadamsoftware added a comment.

Docs fixed according to the comments.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#764834, @Prazek wrote:

> How is that compared to https://reviews.llvm.org/D19201 and the clang patch 
> mentioned in this patch?


Actually, this check does much more. It does not only check for noexcept (and 
throw()) functions, but also for destructors, move constructors, move 
assignments, the main() function, swap() functions and also functions given as 
option. A more important difference is that we traverse the whole call-chain 
and check all the throw statements and try-catch blocks so indirectly throwing 
functions are also reported and no flase positives are caused by throw and 
catch in the same try block.


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100374.
baloghadamsoftware added a comment.

Check added to the Release Notes.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,245 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning:

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100382.
baloghadamsoftware added a comment.

Ignoring bad_alloc.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/ExceptionEscapeCheck.cpp
  clang-tidy/misc/ExceptionEscapeCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-exception-escape.rst
  test/clang-tidy/misc-exception-escape.cpp

Index: test/clang-tidy/misc-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-exception-escape.cpp
@@ -0,0 +1,256 @@
+// RUN: %check_clang_tidy %s misc-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: misc-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: misc-exception-escape.EnabledFunctions, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function '~throwing_destructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: function 'throwing_move_constructor' throws
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: function 'operator=' throws
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_noexcept' throws
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throwing_throw_nothing' throws
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch' throws
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_catch_some' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_each' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_and_catch_all' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_and_rethrow' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_throw' throws
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'throw_catch_rethrow_the_rest' throws
+  try {
+throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'throw_derived_catch_base' throws
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_try' throws
+  try {
+try {
+  throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_try_nested_try' throws
+  try {
+throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'try_nested_catch' throws
+  try {
+try {
+  throw 1;
+} catch(int &) {
+  throw 1.1;
+}
+  } catch(double &) {
+  }
+}
+
+void catch_nested_try() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: function 'catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1; 
+} catch(int &) {
+}
+  }
+}
+
+void bad_catch_nested_try() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_catch_nested_try' throws
+  try {
+throw 1;
+  } catch(int &) {
+try {
+  throw 1.1; 
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void implicit_int_thrower() {
+  throw 1;
+}
+
+void explicit_int_thrower() throw(int);
+
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit' throws
+  implicit_int_thrower();
+}
+
+void indirect_explicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'ind

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 100848.
baloghadamsoftware added a comment.

Rebased to (committed) Part1 (https://reviews.llvm.org/rL304160) and updated 
according to comments.


https://reviews.llvm.org/D32642

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -13,7 +13,102 @@
   }
 }
 
+void simple_good_end_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end())) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+  }
+}
+
 void simple_bad_end(const std::vector &v) {
   auto i = v.end();
   *i; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+void simple_good_begin(const std::vector &v) {
+  auto i = v.begin();
+  if (i != v.begin()) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_good_begin_negated(const std::vector &v) {
+  auto i = v.begin();
+  if (!(i == v.begin())) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_bad_begin(const std::vector &v) {
+  auto i = v.begin();
+  *--i; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void tricky(std::vector &V, int e) {
+  const auto first = V.begin();
+  const auto comp1 = (first != V.end()), comp2 = (first == V.end());
+  if (comp1)
+*first;
+}
+
+void loop(std::vector &V, int e) {
+  auto start = V.begin();
+  while (true) {
+auto item = std::find(start, V.end(), e);
+if (item == V.end())
+  break;
+*item;  // no-warning
+start = ++item; // no-warning
+  }
+}
+
+void bad_move(std::list &L1, std::list &L2) {
+  auto i0 = --L2.cend();
+  L1 = std::move(L2);
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,12 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -295,6 +301,8 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void clear();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -330,6 +338,16 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -369,6 +387,12 @@
 forward_list(forward_list &&other);
 ~forward_list();
 
+void clear();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { r

[PATCH] D31370: [clang-tidy] Prototype to check for exception specification

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

The matcher in https://reviews.llvm.org/D33537 could be reused here as well. 
Maybe I should make it common, but it is more complex than any of the common 
matchers.


Repository:
  rL LLVM

https://reviews.llvm.org/D31370



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

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

There is a patch (https://reviews.llvm.org/D33537) for a check which is a 
superset of this: It does not only check for noexcept (and throw()) functions, 
but also for destructors, move constructors, move assignments, the main() 
function, swap() functions and also functions given as option. A more important 
difference is that we traverse the whole call-chain and check all the throw 
statements and try-catch blocks so indirectly throwing functions are also 
reported and no flase positives are caused by throw and catch in the same try 
block.


https://reviews.llvm.org/D19201



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D33537#770264, @aaron.ballman wrote:

> I think we should try to get as much of this functionality in 
> https://reviews.llvm.org/D3 as possible; there is a considerable amount 
> of overlap between that functionality and this functionality. This check can 
> then cover only the parts that are not reasonably handled by the frontend 
> check instead of duplicating diagnostics the user already receives.


I suppose that the frontend check will not travarse the call-graph just check 
direct throws. Should we only check indirect throws then?


https://reviews.llvm.org/D33537



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

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Any progress in the review?


https://reviews.llvm.org/D32642



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

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I tried `SValBuilder::evalBinOp()` first but it did not help too much. It could 
decide only if I compared the same conjured symbols or different ones, but 
nothing more. It always gave me `UnknownVal`. Even if comparing ${conj_X} == 
${conj_X} + n where n was a concrete integer. So I have to compare the symbol 
part and the concrete integer part separately. Waiting is not an option for us 
since we are a bit delayed with this checker. I have to bring them out of alpha 
until the end of the year. If Z3 constraint solver is accepted and will be the 
default constraint manager, then I can somewhat simplify my code. That patch is 
under review for long and I am not sure whether it will be the default ever.


https://reviews.llvm.org/D32642



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

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 103568.
baloghadamsoftware added a comment.

Minor fixes according to the comments.


https://reviews.llvm.org/D32642

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -13,7 +13,102 @@
   }
 }
 
+void simple_good_end_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end())) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+  }
+}
+
 void simple_bad_end(const std::vector &v) {
   auto i = v.end();
   *i; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+void simple_good_begin(const std::vector &v) {
+  auto i = v.begin();
+  if (i != v.begin()) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_good_begin_negated(const std::vector &v) {
+  auto i = v.begin();
+  if (!(i == v.begin())) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_bad_begin(const std::vector &v) {
+  auto i = v.begin();
+  *--i; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void tricky(std::vector &V, int e) {
+  const auto first = V.begin();
+  const auto comp1 = (first != V.end()), comp2 = (first == V.end());
+  if (comp1)
+*first;
+}
+
+void loop(std::vector &V, int e) {
+  auto start = V.begin();
+  while (true) {
+auto item = std::find(start, V.end(), e);
+if (item == V.end())
+  break;
+*item;  // no-warning
+start = ++item; // no-warning
+  }
+}
+
+void bad_move(std::list &L1, std::list &L2) {
+  auto i0 = --L2.cend();
+  L1 = std::move(L2);
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,12 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -295,6 +301,8 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void clear();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -330,6 +338,16 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -369,6 +387,12 @@
 forward_list(forward_list &&other);
 ~forward_list();
 
+void clear();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
Index: lib/StaticAnalyzer/Chec

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

> I'm sure that simplification `(($x + N) + M) ~> ($x + (M + N))` is already 
> working in `SValBuilder`.

No, it is unfortunately not working: I tried to increase `${conj_X}` by `1`, 
then again by `1`, and I got symbolic expression `(${conj_X}+1)+1)`.


https://reviews.llvm.org/D32642



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

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 103728.
baloghadamsoftware added a comment.

`SymbolManager::getSymIntExpr()` replaced by `SValBuilder::evalBinOp()`, 
function `compact()` eliminated.


https://reviews.llvm.org/D32642

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -13,7 +13,102 @@
   }
 }
 
+void simple_good_end_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end())) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+  }
+}
+
 void simple_bad_end(const std::vector &v) {
   auto i = v.end();
   *i; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+void simple_good_begin(const std::vector &v) {
+  auto i = v.begin();
+  if (i != v.begin()) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_good_begin_negated(const std::vector &v) {
+  auto i = v.begin();
+  if (!(i == v.begin())) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_bad_begin(const std::vector &v) {
+  auto i = v.begin();
+  *--i; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void tricky(std::vector &V, int e) {
+  const auto first = V.begin();
+  const auto comp1 = (first != V.end()), comp2 = (first == V.end());
+  if (comp1)
+*first;
+}
+
+void loop(std::vector &V, int e) {
+  auto start = V.begin();
+  while (true) {
+auto item = std::find(start, V.end(), e);
+if (item == V.end())
+  break;
+*item;  // no-warning
+start = ++item; // no-warning
+  }
+}
+
+void bad_move(std::list &L1, std::list &L2) {
+  auto i0 = --L2.cend();
+  L1 = std::move(L2);
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- test/Analysis/Inputs/system-header-simulator-cxx.h
+++ test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -252,6 +252,12 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -295,6 +301,8 @@
 list& operator=(list &&other);
 list& operator=(std::initializer_list ilist);
 
+void clear();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() const { return const_iterator(_start); }
@@ -330,6 +338,16 @@
   return size_t(_finish - _start);
 }
 
+void clear();
+
+void push_back(const T &value);
+void push_back(T &&value);
+void pop_back();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 T &operator[](size_t n) {
   return _start[n];
 }
@@ -369,6 +387,12 @@
 forward_list(forward_list &&other);
 ~forward_list();
 
+void clear();
+
+void push_front(const T &value);
+void push_front(T &&value);
+void pop_front();
+
 iterator begin() { return iterator(_start); }
 const_iterator begin() const { return const_iterator(_start); }
 const_iterator cbegin() con

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Now I can improve `SValBuilder` to compare `{conj_X}+n` to `conj_X}+m`, but I 
am not sure if it helps to simplify `compare()` much. How to handle cases where 
I have to compare `{conj_X}+n` to `{conj_Y}+m`, an we have a range `[k..k]` for 
`{conj_X}-{conj_Y}` in the constraint manager. I still need to decompose the 
two expressions, retrieve the single length range and adjust one of the sides 
of the comparison. I think I should not add such complicated code (i.e. 
retrieving single length range from the constrain manager) to `SValBuilder`.


https://reviews.llvm.org/D32642



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.
Herald added subscribers: a.sidorin, rnkovacs, szepet.

This one is not blocked anymore since I removed the dependency.


https://reviews.llvm.org/D35110



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35109#956075, @NoQ wrote:

> I'm totally fine with assuming the MAX/4 constraint on checker side - 
> extension math would still be better than the MAX/4 pattern-matching in core 
> because extension math should be more useful on its own. Otherwise, i'm also 
> fine with MAX/4 `>`/`<` under an off-by-default flag, if this is indeed 
> blocking future reviews. I'm just really uncomfortable with huge manual 
> checker-side symbolic computations, essentially declaring that checkers 
> should make their own solvers. Analyzer progress is already slowed down 
> dramatically by technical debt in a lot of places - we rarely have time to 
> address it, but at least we shouldn't introduce much more. This doesn't, in 
> my opinion, reduce the fascination of that whole thing you've constructed. It 
> is wonderful.


The problem with the type extension approach is that it is mathematically "more 
correct than it should be". I mean it fixes all overflow cases, even 
intentional ones which we probably do not want to do. I am not speaking about 
intentional overflow cases in the checkers or in the core infrastructure which 
could be fixed but intentional overlfow cases in the code we analyze.

So probably the MAX/4 approach is the correct one. But how to add a flag for 
this? Is it a flag enabled by the user or is it automatically enabled if the 
checker is enabled?


https://reviews.llvm.org/D35109



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35110#968284, @baloghadamsoftware wrote:

> This one is not blocked anymore since I removed the dependency.


But I have to modify the test cases...


https://reviews.llvm.org/D35110



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

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35109#969109, @NoQ wrote:

> I guess it'd be an `-analyzer-config` flag. You can add it to the 
> `AnalyzerOptions` object, which has access to these flags and can be accessed 
> from `AnalysisManager`.


OK, I can do that. BUt how should I call it? The name should be something the 
user also understands, not referring to some internal stuff. Any ideas?


https://reviews.llvm.org/D35109



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Strange, but modifying the tests from `m  n` to `m - n  0`  
does not help. The statement `if (m - n  0)` does not store a range 
for `m - n` in the constraint manager. With the other patch which automatically 
changes `m  n` to `m - n  0` the range is stored 
automatically.


https://reviews.llvm.org/D35110



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

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, dcoughlin.
Herald added subscribers: a.sidorin, szepet.

This patch is a "light" version of https://reviews.llvm.org/D35109:

Since the range-based constraint manager (default) is weak in handling 
comparisons where symbols are on both sides it is wise to rearrange them to 
have symbols only on the left side. Thus e.g. `A + n >= B + m` becomes `A - B 
>= m - n` which enables the constraint manager to store a range `m - n .. 
MAX_VALUE` for the symbolic expression `A - B`. This can be used later to check 
whether e.g. `A + k == B + l` can be true, which is also rearranged to `A - B 
== l - k` so the constraint manager can check whether `l - k` is in the range 
(thus greater than or equal to `m - n`).

The restriction in this version is the the rearrangement happens only if the 
concrete integers are within the range `[min/4 .. max/4]` where `min` and `max` 
are the minimal and maximal values of their type.

The rearrangement is not enabled by default. It has to be enabled by using 
`-analyzer-config aggressive-relational-comparison-simplification=true`.

Co-author of this patch is Artem Dergachev (NoQ).


https://reviews.llvm.org/D41938

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,928 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+void clang_analyzer_printState();
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+int g();
+int f() {
+  int x = g();
+  // Assert that no overflows occur in this test file.
+  // Assuming that concrete integers are also within that range.
+  if (x > ((int)INT_MAX / 4))
+exit(1);
+  if (x < -(((int)INT_MAX) / 4))
+exit(1);
+  return x;
+}
+
+void compare_different_symbol_equal() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 0}}
+}
+
+void compare_different_symbol_plus_left_int_equal() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_minus_left_int_equal() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 1}}
+}
+
+void compare_different_symbol_plus_right_int_equal() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 2}}
+}
+
+void compare_different_symbol_minus_right_int_equal() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int_equal() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int_equal() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$5{int}) - (conj_$2{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int_equal() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$5{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int_equal() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35110#972430, @NoQ wrote:

> In https://reviews.llvm.org/D35110#969782, @baloghadamsoftware wrote:
>
> > Strange, but modifying the tests from `m  n` to `m - n  
> > 0`  does not help. The statement `if (m - n  0)` does not store a 
> > range for `m - n` in the constraint manager. With the other patch which 
> > automatically changes `m  n` to `m - n  0` the range is 
> > stored automatically.
>
>
> I guess we can easily assume how a `SymIntExpr` relates to a number by 
> storing a range on the opaque left-hand-side symbol, no matter how 
> complicated it is, but we cannot assume how a symbol relates to a symbol 
> (there's no obvious range to store). That's just how `assumeSym` currently 
> works.


Actually it happens because `m - n` evaluates to `Unknown`. The code part 
responsible for this is the beginning of `SValBuilder::makeSymExprValNN()`, 
which prevents `m - n`-like symbolic expression unless one of `m` or `n` is 
`Tainted`. Anna added this part 5-6 years ago because some kind of bug, but it 
seems that it still exists. If I try to remove it then one test executes for 
days (with loop max count 63 or 64) and two tests fail with an assert.


https://reviews.llvm.org/D35110



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 129441.
baloghadamsoftware added a comment.

Updated to be based upon https://reviews.llvm.org/D41938.


https://reviews.llvm.org/D35110

Files:
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
   clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
 }
 
-//---
-// False positives
-//---
-
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
   clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,64 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+void assert_in_range(int x) {
+  // Assert that no overflows occur in this test file.
+  // Assuming that concrete integers are also within that range.
+  if (x > ((int)INT_MAX / 4))
+exit(1);
+  if (x < -(((int)INT_MAX) / 4))
+exit(1);
+}
+
+void assert_in_range_2(int m, int n) {
+  assert_in_range(m);
+  assert_in_range(n);
+}
+
+void equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m != n)
+return;
+  clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m == n)
+return;
+  clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m < n)
+return;
+  clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m <= n)
+return;
+  clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m > n)
+return;
+  clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m >= n)
+return;
+  clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -256,6 +256,29 @@
 return newRanges;
   }
 
+  // Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal
+  // signed value and vice versa.
+  RangeSet Negate(BasicValueFactory &BV, Factory &F) const {
+PrimRangeSet newRanges = F.getEmptySet();
+
+for (iterator i = begin(), e = end(); i != e; ++i) {
+  const llvm::APSInt &from = i->From(), &to = i->To();
+  const llvm::APSInt &newFrom = (to.isMinSignedValue() ?
+ BV.getMaxValue(to) :
+ (to.isMaxSignedValue() ?
+  BV.getMinValue(to) :
+  BV.getValue(- to)));
+  const llvm::APSInt &newTo = (from.isMinSignedValue() ?
+   BV.getMaxValue(from) :
+

[PATCH] D32642: [Analyzer] Iterator Checker - Part 2: Increment, decrement operators and ahead-of-begin checks

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 129445.
baloghadamsoftware added a comment.

Updated to be based upon https://reviews.llvm.org/D41938 and 
https://reviews.llvm.org/D35110.


https://reviews.llvm.org/D32642

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/Inputs/system-header-simulator-cxx.h
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/diagnostics/explicit-suppression.cpp
  test/Analysis/iterator-range.cpp

Index: test/Analysis/iterator-range.cpp
===
--- test/Analysis/iterator-range.cpp
+++ test/Analysis/iterator-range.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=false %s -verify
-// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
@@ -13,7 +13,110 @@
   }
 }
 
+void simple_good_end_negated(const std::vector &v) {
+  auto i = v.end();
+  if (!(i == v.end())) {
+clang_analyzer_warnIfReached();
+*i; // no-warning
+  }
+}
+
 void simple_bad_end(const std::vector &v) {
   auto i = v.end();
   *i; // expected-warning{{Iterator accessed outside of its range}}
 }
+
+void simple_good_begin(const std::vector &v) {
+  auto i = v.begin();
+  if (i != v.begin()) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_good_begin_negated(const std::vector &v) {
+  auto i = v.begin();
+  if (!(i == v.begin())) {
+clang_analyzer_warnIfReached();
+*--i; // no-warning
+  }
+}
+
+void simple_bad_begin(const std::vector &v) {
+  auto i = v.begin();
+  *--i; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void decrease(const std::vector &v) {
+  auto i = v.end();
+  --i;
+  *i; // no-warning
+}
+
+void copy_and_decrease1(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i1; // no-warning
+}
+
+void copy_and_decrease2(const std::vector &v) {
+  auto i1 = v.end();
+  auto i2 = i1;
+  --i1;
+  *i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase1(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i1 == v.end())
+*i2; // no-warning
+}
+
+void copy_and_increase2(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (i2 == v.end())
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void copy_and_increase3(const std::vector &v) {
+  auto i1 = v.begin();
+  auto i2 = i1;
+  ++i1;
+  if (v.end() == i2)
+*i2; // expected-warning{{Iterator accessed outside of its range}}
+}
+
+void tricky(std::vector &V, int e) {
+  const auto first = V.begin();
+  const auto comp1 = (first != V.end()), comp2 = (first == V.end());
+  if (comp1)
+*first;
+}
+
+void loop(std::vector &V, int e) {
+  auto start = V.begin();
+  while (true) {
+auto item = std::find(start, V.end(), e);
+if (item == V.end())
+  break;
+*item;  // no-warning
+start = ++item; // no-warning
+  }
+}
+
+void bad_move(std::list &L1, std::list &L2) {
+  auto i0 = --L2.cend();
+  L1 = std::move(L2);
+  *++i0; // expected-warning{{Iterator accessed outside of its range}}
+}
Index: test/Analysis/diagnostics/explicit-suppression.cpp
===
--- test/Analysis/diagnostics/explicit-suppression.cpp
+++ test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:490 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:514 {{Called C++ object pointer is null}}
 #endif
 }
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- test/Analysis/constraint_manager_negate_difference.c
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=deb

[PATCH] D44079: [ASTImporter] Allow testing of import sequences; fix import of typedefs for anonymous decls

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Hi,

Thank you for the patch.

It seems that the new approach still does not solve a problem with anonymous 
structures in typedefs.In C++ a copy constructor is automatically generated, 
and its parameter is the anonymous structure itself. This triggers caching 
`NoLinkage` for it during the import of the constructor, but later fails with 
an assert because at the end the computed linkage is `ExternalLinkage`. It 
seems that anonymous structures are somewhat tricky in the original AST. For 
this struct:

  typedef struct {
int n;
  } T;

the original AST is:

  CXXRecordDecl 0x1abcb38  line:1:9 imported 
struct definition
  |-FieldDecl 0x1abce68  col:7 imported n 'int'
  |-CXXConstructorDecl 0x1abced0  col:9 imported implicit used  'void 
(void) throw()' inline default trivial
  | `-CompoundStmt 0x1abd2e0 
  `-CXXConstructorDecl 0x1abcfd8  col:9 imported implicit  'void (const 
T &)' inline default trivial noexcept-unevaluated 0x1abcfd8
`-ParmVarDecl 0x1abd138  col:9 imported 'const T &'
  TypedefDecl 0x1abcc78  col:3 imported 
referenced T 'struct T':'T'
  `-ElaboratedType 0x1abccd0 'struct T' sugar imported
`-RecordType 0x1abcc50 'T' imported
  `-CXXRecord 0x1abcb38 ''

But the imported one is:

  CXXRecordDecl 0x1a51400  col:9 struct definition
  |-FieldDecl 0x1a51540  col:7 n 'int'
  |-CXXConstructorDecl 0x1a515e0  col:9 implicit used  'void (void) 
throw()' inline trivial
  | `-CompoundStmt 0x1a51688 
  `-CXXConstructorDecl 0x1a51768  col:9 implicit  'void (const struct 
(anonymous at /tmp/first.cpp:1:9) &)' inline trivial noexcept-unevaluated 
0x1a51768
`-ParmVarDecl 0x1a51708  col:9 'const struct (anonymous at 
/tmp/first.cpp:1:9) &'
  TypedefDecl 0x1a518b0  col:3 T 'struct 
(anonymous struct at /tmp/first.cpp:1:9)':'struct (anonymous at 
/tmp/first.cpp:1:9)'
  `-ElaboratedType 0x1a51860 'struct (anonymous struct at /tmp/first.cpp:1:9)' 
sugar
`-RecordType 0x1a514a0 'struct (anonymous at /tmp/first.cpp:1:9)'
  `-CXXRecord 0x1a51400 ''


Repository:
  rC Clang

https://reviews.llvm.org/D44079



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

[PATCH] D44079: [ASTImporter] Allow testing of import sequences; fix import of typedefs for anonymous decls

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

By moving the code that sets the type name of an anoynmous declaration from 
Import(Decl*) to ImportDefinition(RecordDecl*, RecordDecl*, 
ImportDefinitionKind) (and the same for Enum) we will not crash upon importing 
typedefs containing anonymous strcutures. This is a patch-over-patch for it, 
including the test cases:

F5887321: D44079-Mod.diff 


Repository:
  rC Clang

https://reviews.llvm.org/D44079



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

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:330
+  nonloc::ConcreteInt(Max), SVB.getConditionType());
+  if (auto DV = IsCappedFromAbove.getAs()) {
+if (State->assume(*DV, false))

george.karpenkov wrote:
> 6 lines of branching is probably better expressed as
> 
> ```
> if (!isa(IsCappedFromAbove) || 
> State->assume(*dyn_cast(IsCappedFromAbove), false))
>return false
> ```
SVal is not a pointer, so isa<>() and dyn_cast<>() does not work here.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:376
+  // Fail to decompose: "reduce" the problem to the "$x + 0" case.
+  return std::make_tuple(Sym, BO_Add, BV.getValue(0, Sym->getType()));
+}

george.karpenkov wrote:
> Is it beneficial to do this though? At the point where `decomposeSymbol` is 
> called we are still checking whether the rearrangement could be performed, so 
> maybe just returning a false flag would be better?
Failing to decompose a symbol does not mean the rearrangement could not be 
performed. If we have just A on the left side instead of A+m or A-m, then we 
regard it as A+0.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:432
+// "$x - $y" vs. "$y - $x" because those are solver's keys.
+if (LInt > RInt) {
+  ResultSym = SymMgr.getSymSymExpr(RSym, BO_Sub, LSym, SymTy);

george.karpenkov wrote:
> I think this could be shortened and made more explicit by constructing the 
> LHS and RHS first, and then reversing both and the comparison operator if RHS 
> is negative.
Are you sure it would be shorter? Anyway, how to reverse a SymSymExpr?


https://reviews.llvm.org/D41938



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

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 138954.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D41938

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/conditional-path-notes.c
  test/Analysis/explain-svals.cpp
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,931 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+int g();
+int f() {
+  int x = g();
+  // Assert that no overflows occur in this test file.
+  // Assuming that concrete integers are also within that range.
+  assert(x <= ((int)INT_MAX / 4));
+  assert(x >= -((int)INT_MAX / 4));
+  return x;
+}
+
+void compare_different_symbol_equal() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 0}}
+}
+
+void compare_different_symbol_plus_left_int_equal() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_minus_left_int_equal() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$9{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 1}}
+}
+
+void compare_different_symbol_plus_right_int_equal() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 2}}
+}
+
+void compare_different_symbol_minus_right_int_equal() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int_equal() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int_equal() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$9{int}) - (conj_$2{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int_equal() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int_equal() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$9{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{((conj_$2{int}) - (conj_$9{int})) == 1}}
+}
+
+void compare_same_symbol_equal() {
+  int x = f(), y = x;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_eval(x == y);
+  // expected-warning@-1{{TRUE}}
+}
+
+void compare_same_symbol_plus_left_int_equal() {
+  int x = f(), y = x;
+  ++x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_eval(x == y);
+  // expected-warning@-1{{FALSE}}
+}
+
+void compare_same_s

[PATCH] D41938: [Analyzer] SValBuilder Comparison Rearrangement (with Restrictions and Analyzer Option)

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware marked 12 inline comments as done.
baloghadamsoftware added inline comments.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:326
+
+  llvm::APSInt Max = AT.getMaxValue() >> 2; // Divide by 4.
+  SVal IsCappedFromAbove =

george.karpenkov wrote:
> Would just division produce the same result? Also probably it's better to 
> make "4" a constant, at least with `#define`
I changed it to division, but I am not sure if we a constant would be more 
readable here than 4.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:337
+
+  llvm::APSInt Min = -Max;
+  SVal IsCappedFromBelow =

george.karpenkov wrote:
> 326-335 duplicates 338-346.
> Perhaps we could have
> 
> ```
> static bool isCappedFrom(bool Above, llvm::APSInt bound, ...)
> ```
> 
> ?
isInRelation(), because the opcode itself is passed instead of a bool.


https://reviews.llvm.org/D41938



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

[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware accepted this revision.
baloghadamsoftware added a comment.

Oh, it slipped through somehow. Thanks for fixing this!


Repository:
  rC Clang

https://reviews.llvm.org/D47417



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Sorry, Artem, but it does not work this way. Even if the symbolic expressions 
are constrained to `[-MAX/4..MAX/4]`, after rearrangement the difference still 
uses the whole range, thus `m>n` becomes `m-n>0`, where in the false branch the 
range for `m-n` is `[MIN..0]`. Then if we check `n>=m` the range set is 
reversed to `[MIN..MIN]U[0..MAX]` which results `UNKNOWN` for `n-m`. It does 
not solve any of our problems and there is no remedy on the checker's side.


https://reviews.llvm.org/D35110



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Maybe if we could apply somehow a `[-MAX/2..MAX/2]` constraint to both sides of 
the rearranged equality in SimpleSValBuilder.


https://reviews.llvm.org/D35110



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

[PATCH] D47417: [analyzer] Add missing state transition in IteratorChecker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Did the tests execute? I am not sure. First problem is the the container may 
become dead before the iterator, so its `Begin` and `End` symbols may be 
inaccessible. This is easy to solve by marking the container of the iterator as 
live. However, there is a second problem that disables correct tracking of 
iterators: memory regions are marked as dead, however there are 
`LazyCompoundVal`s referring to them. Is this maybe a bug in `SymbolReaper`?


Repository:
  rC Clang

https://reviews.llvm.org/D47417



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D35110#1119496, @NoQ wrote:

> Which expressions are constrained? Why does the difference use the whole 
> range? Is it something that could have been fixed by the "enforce that 
> separately" part in my old comment:
>
> > iterator-related symbols are all planned to be within range [-2²⁹, -2²⁹], 
> > right? So if we subtract one such symbol from another, it's going to be in 
> > range [-2³⁰, 2³⁰]. Can we currently infer that? Or maybe we should make the 
> > iterator checker to enforce that separately?
>
> ?


`RangedConstraintManager` currently does not support `Sym+Sym`-type of 
expressions, only `Sym+Int`-type ones. That is why it cannot calculate that the 
result is within `[-2³⁰, 2³⁰]`. In the iterator checkers we do not know 
anything about the rearranged expressions, it has no access to the 
sum/difference, the whole purpose of your proposal was to put in into the 
infrastructure. The checker enforces everything it can but it does not help.


https://reviews.llvm.org/D35110



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

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.
Herald added a subscriber: mikhail.ramalho.

Any idea how to proceed?


https://reviews.llvm.org/D35110



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

[PATCH] D47445: [ASTImporter] Corrected diagnostic client handling in tests.

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC334804: [ASTImporter] Corrected diagnostic client handling 
in tests. (authored by baloghadamsoftware, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47445?vs=149412&id=151459#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47445

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  unittests/AST/ASTImporterTest.cpp


Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -275,6 +275,12 @@
   this->PP = std::move(PP);
 }
 
+void ASTUnit::enableSourceFileDiagnostics() {
+  assert(getDiagnostics().getClient() && Ctx &&
+  "Bad context for source file");
+  getDiagnostics().getClient()->BeginSourceFile(Ctx->getLangOpts(), PP.get());
+}
+
 /// Determine the set of code-completion contexts in which this
 /// declaration should be shown.
 static unsigned getDeclShowContexts(const NamedDecl *ND,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
   ASTContext &FromCtx = FromAST->getASTContext(),
   &ToCtx = ToAST->getASTContext();
 
+  FromAST->enableSourceFileDiagnostics();
+  ToAST->enableSourceFileDiagnostics();
+
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
 
@@ -172,7 +175,9 @@
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  Unit->enableSourceFileDiagnostics();
+}
   };
 
   // We may have several From contexts and related translation units. In each
@@ -214,6 +219,7 @@
 ToCode = ToSrcCode;
 assert(!ToAST);
 ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->enableSourceFileDiagnostics();
 
 ASTContext &FromCtx = FromTU.Unit->getASTContext(),
&ToCtx = ToAST->getASTContext();
@@ -261,6 +267,7 @@
 ToCode = ToSrcCode;
 assert(!ToAST);
 ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+ToAST->enableSourceFileDiagnostics();
 
 return ToAST->getASTContext().getTranslationUnitDecl();
   }
@@ -274,6 +281,7 @@
   // Build the AST from an empty file.
   ToAST =
   tooling::buildASTFromCodeWithArgs(/*Code=*/"", ToArgs, "empty.cc");
+  ToAST->enableSourceFileDiagnostics();
 }
 
 // Create a virtual file in the To Ctx which corresponds to the file from
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -438,6 +438,15 @@
   void setASTContext(ASTContext *ctx) { Ctx = ctx; }
   void setPreprocessor(std::shared_ptr pp);
 
+  /// Enable source-range based diagnostic messages.
+  ///
+  /// If diagnostic messages with source-range information are to be expected
+  /// and AST comes not from file (e.g. after LoadFromCompilerInvocation) this
+  /// function has to be called.
+  /// The function is to be called only once and the AST should be associated
+  /// with the same source file afterwards.
+  void enableSourceFileDiagnostics();
+
   bool hasSema() const { return (bool)TheSema; }
 
   Sema &getSema() const { 


Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -275,6 +275,12 @@
   this->PP = std::move(PP);
 }
 
+void ASTUnit::enableSourceFileDiagnostics() {
+  assert(getDiagnostics().getClient() && Ctx &&
+  "Bad context for source file");
+  getDiagnostics().getClient()->BeginSourceFile(Ctx->getLangOpts(), PP.get());
+}
+
 /// Determine the set of code-completion contexts in which this
 /// declaration should be shown.
 static unsigned getDeclShowContexts(const NamedDecl *ND,
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -98,6 +98,9 @@
   ASTContext &FromCtx = FromAST->getASTContext(),
   &ToCtx = ToAST->getASTContext();
 
+  FromAST->enableSourceFileDiagnostics();
+  ToAST->enableSourceFileDiagnostics();
+
   ASTImporter Importer(ToCtx, ToAST->getFileManager(),
FromCtx, FromAST->getFileManager(), false);
 
@@ -172,7 +175,9 @@
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(U

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 151666.
baloghadamsoftware added a comment.

New warning message, more detailed docs.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,265 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~throwing_destructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'throwing_move_constructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_noexcept' which should not throw exceptions
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_some' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_each' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_all' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_rethrow' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_throw' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_rethrow_the_rest' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base' which should not throw exceptions
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'bad_try_nested_try' which should not throw exceptions
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 151720.
baloghadamsoftware added a comment.

-(-2^n) == -2^n


https://reviews.llvm.org/D35110

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  test/Analysis/constraint_manager_negate_difference.c
  test/Analysis/ptr-arith.c

Index: test/Analysis/ptr-arith.c
===
--- test/Analysis/ptr-arith.c
+++ test/Analysis/ptr-arith.c
@@ -265,49 +265,26 @@
   clang_analyzer_eval((rhs - lhs) > 0); // expected-warning{{TRUE}}
 }
 
-//---
-// False positives
-//---
-
 void zero_implies_reversed_equal(int *lhs, int *rhs) {
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{UNKNOWN}}
   if ((rhs - lhs) == 0) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs != lhs); // expected-warning{{FALSE}}
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval((rhs - lhs) == 0); // expected-warning{{FALSE}}
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
   clang_analyzer_eval(rhs != lhs); // expected-warning{{TRUE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(rhs != lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void canonical_equal(int *lhs, int *rhs) {
   clang_analyzer_eval(lhs == rhs); // expected-warning{{UNKNOWN}}
   if (lhs == rhs) {
-#ifdef ANALYZER_CM_Z3
 clang_analyzer_eval(rhs == lhs); // expected-warning{{TRUE}}
-#else
-clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 return;
   }
   clang_analyzer_eval(lhs == rhs); // expected-warning{{FALSE}}
-
-#ifdef ANALYZER_CM_Z3
   clang_analyzer_eval(rhs == lhs); // expected-warning{{FALSE}}
-#else
-  clang_analyzer_eval(rhs == lhs); // expected-warning{{UNKNOWN}}
-#endif
 }
 
 void compare_element_region_and_base(int *p) {
Index: test/Analysis/constraint_manager_negate_difference.c
===
--- /dev/null
+++ test/Analysis/constraint_manager_negate_difference.c
@@ -0,0 +1,77 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+
+void clang_analyzer_eval(int);
+
+void exit(int);
+
+#define UINT_MAX (~0U)
+#define INT_MAX (UINT_MAX & (UINT_MAX >> 1))
+
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+unsigned int __line, __const char *__function)
+ __attribute__ ((__noreturn__));
+#define assert(expr) \
+  ((expr)  ? (void)(0)  : __assert_fail (#expr, __FILE__, __LINE__, __func__))
+
+void assert_in_range(int x) {
+  assert(x <= ((int)INT_MAX / 4));
+  assert(x >= -(((int)INT_MAX) / 4));
+}
+
+void assert_in_wide_range(int x) {
+  assert(x <= ((int)INT_MAX / 2));
+  assert(x >= -(((int)INT_MAX) / 2));
+}
+
+void assert_in_range_2(int m, int n) {
+  assert_in_range(m);
+  assert_in_range(n);
+}
+
+void equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m != n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n == m); // expected-warning{{TRUE}}
+}
+
+void non_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m == n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n != m); // expected-warning{{TRUE}}
+}
+
+void less_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m < n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n <= m); // expected-warning{{TRUE}}
+}
+
+void less(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m <= n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n < m); // expected-warning{{TRUE}}
+}
+
+void greater_or_equal(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m > n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n >= m); // expected-warning{{TRUE}}
+}
+
+void greater(int m, int n) {
+  assert_in_range_2(m, n);
+  if (m >= n)
+return;
+  assert_in_wide_range(m - n);
+  clang_analyzer_eval(n > m); // expected-warning{{TRUE}}
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -174,6 +174,34 @@
   return newRanges;
 }
 
+// Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal
+// signed value.
+RangeSet RangeSet::Negate(BasicValueFactory &BV, Factory &F) const {
+  PrimRangeSet newRanges = F.getEmptySet();
+
+  for (iterator i = begin(), e = end(); i != e; ++i) {
+const llvm::APSInt &from = i->From(), &to = i->To();
+   

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I added extra assertion into the test for the difference. Interestingly, it 
also works if I assert `n-m` is in the range instead of `m-n`.


https://reviews.llvm.org/D35110



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.



Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in 
function 'indirect_implicit' which should not throw exceptions
+  implicit_int_thrower();

dberris wrote:
> Can we make the warning more accurate here? Something like:
> 
> ```
> warning: call to 'implicit_int_thrower' may throw an exception and propagate 
> through noexcept function 'indirect_implicit'
> ```
> 
> It would be helpful to diagnose the point at which the exception may be 
> thrown from within the function (if it's an operator, a function call, etc.) 
> that doesn't have exceptions handled. If you can highlight not just the line 
> number but the actual expression in which the uncaught exception may 
> propagate, it would make this warning much better.
> 
> If you think it's worth it (or if it's feasible), having a FixIt hint to wrap 
> a block of statements where exceptions may propagate in a `try { ... } catch 
> (...) { ... }` block would turn this warning from a good warning, to a great 
> warning -- potentially something that could be automatically applied by a 
> tool as well.
I think this is a good future improvement. However, I tried to make the first 
version as small as possible, and then enhance it incrementally. Even the 
current code is too big for a Clang-Tidy check.


https://reviews.llvm.org/D33537



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

[PATCH] D33537: [clang-tidy] Exception Escape Checker

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 151876.
baloghadamsoftware added a comment.

Typo fixed.


https://reviews.llvm.org/D33537

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tidy/bugprone/ExceptionEscapeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-exception-escape.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-exception-escape.cpp

Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -0,0 +1,265 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-std=c++11 -config="{CheckOptions: [{key: bugprone-exception-escape.IgnoredExceptions, value: 'ignored1,ignored2'}, {key: bugprone-exception-escape.FunctionsThatShouldNotThrow, value: 'enabled1,enabled2,enabled3'}]}" --
+
+struct throwing_destructor {
+  ~throwing_destructor() {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function '~throwing_destructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_constructor {
+  throwing_move_constructor(throwing_move_constructor&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: an exception may be thrown in function 'throwing_move_constructor' which should not throw exceptions
+throw 1;
+  }
+};
+
+struct throwing_move_assignment {
+  throwing_move_assignment& operator=(throwing_move_assignment&&) {
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+throw 1;
+  }
+};
+
+void throwing_noexcept() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_noexcept' which should not throw exceptions
+  throw 1;
+}
+
+void throwing_throw_nothing() throw() {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throwing_throw_nothing' which should not throw exceptions
+  throw 1;
+}
+
+void throw_and_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_some(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_some' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  }
+}
+
+void throw_and_catch_each(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_each' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(double &) {
+  }
+}
+
+void throw_and_catch_all(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_catch_all' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(...) {
+  }
+}
+
+void throw_and_rethrow() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_and_rethrow' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw;
+  }
+}
+
+void throw_catch_throw() noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_throw' which should not throw exceptions
+  try {
+throw 1;
+  } catch(int &) {
+throw 2;
+  }
+}
+
+void throw_catch_rethrow_the_rest(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_catch_rethrow_the_rest' which should not throw exceptions
+  try {
+if (n) throw 1;
+throw 1.1;
+  } catch(int &) {
+  } catch(...) {
+throw;
+  }
+}
+
+class base {};
+class derived: public base {};
+
+void throw_derived_catch_base() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base' which should not throw exceptions
+  try {
+throw derived();
+  } catch(base &) {
+  }
+}
+
+void try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'try_nested_try' which should not throw exceptions
+  try {
+try {
+  if (n) throw 1;
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void bad_try_nested_try(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'bad_try_nested_try' which should not throw exceptions
+  try {
+if (n) throw 1;
+try {
+  throw 1.1;
+} catch(int &) {
+}
+  } catch(double &) {
+  }
+}
+
+void try_nested_catch() noexcept {
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:6: warning: an exc

[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I tested all parts of the Iterator Checkers, all tests passed.


https://reviews.llvm.org/D35110



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

[PATCH] D48427: [Analyzer] Fix for D47417 to make the tests pass

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: NoQ, rnkovacs.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, szepet, 
xazax.hun, whisperity.
Herald added a reviewer: george.karpenkov.

https://reviews.llvm.org/D47417 is a fix for an accidentally missing 
transition. However, if we apply that fix, the checker will remove data from 
the GDM which is still needed. In this fix we mark regions of containers with 
active iterators alive.


https://reviews.llvm.org/D48427

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -168,7 +168,7 @@
 class IteratorChecker
 : public Checker,
- check::DeadSymbols,
+ check::LiveSymbols, check::DeadSymbols,
  eval::Assume> {
 
   std::unique_ptr OutOfRangeBugType;
@@ -198,6 +198,7 @@
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const MaterializeTemporaryExpr *MTE,
  CheckerContext &C) const;
+  void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
  bool Assumption) const;
@@ -363,6 +364,22 @@
   C.addTransition(State);
 }
 
+void IteratorChecker::checkLiveSymbols(ProgramStateRef State,
+   SymbolReaper &SR) const {
+  // Keep containers alive while iterators are alive
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+const auto Pos = Reg.second;
+SR.markLive(Pos.getContainer());
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+const auto Pos = Sym.second;
+SR.markLive(Pos.getContainer());
+  }
+}
+
 void IteratorChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
   // Cleanup
@@ -395,6 +412,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,


Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -168,7 +168,7 @@
 class IteratorChecker
 : public Checker,
- check::DeadSymbols,
+ check::LiveSymbols, check::DeadSymbols,
  eval::Assume> {
 
   std::unique_ptr OutOfRangeBugType;
@@ -198,6 +198,7 @@
   void checkPostCall(const CallEvent &Call, CheckerContext &C) const;
   void checkPostStmt(const MaterializeTemporaryExpr *MTE,
  CheckerContext &C) const;
+  void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
   void checkDeadSymbols(SymbolReaper &SR, CheckerContext &C) const;
   ProgramStateRef evalAssume(ProgramStateRef State, SVal Cond,
  bool Assumption) const;
@@ -363,6 +364,22 @@
   C.addTransition(State);
 }
 
+void IteratorChecker::checkLiveSymbols(ProgramStateRef State,
+   SymbolReaper &SR) const {
+  // Keep containers alive while iterators are alive
+  auto RegionMap = State->get();
+  for (const auto Reg : RegionMap) {
+const auto Pos = Reg.second;
+SR.markLive(Pos.getContainer());
+  }
+
+  auto SymbolMap = State->get();
+  for (const auto Sym : SymbolMap) {
+const auto Pos = Sym.second;
+SR.markLive(Pos.getContainer());
+  }
+}
+
 void IteratorChecker::checkDeadSymbols(SymbolReaper &SR,
CheckerContext &C) const {
   // Cleanup
@@ -395,6 +412,8 @@
   State = State->remove(Comp.first);
 }
   }
+
+  C.addTransition(State);
 }
 
 ProgramStateRef IteratorChecker::evalAssume(ProgramStateRef State, SVal Cond,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48460: [analyzer] Fix invalidation on C++ const methods.

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I find it a strange behavior that this-expression is sometimes a pointer, 
sometimes a record declaration. If references are resolved, why are pointers 
not?

This is an important fix, but I wonder how many other places are in the code 
where we do not handle this-expressions by pointers.


Repository:
  rC Clang

https://reviews.llvm.org/D48460



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

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 168787.
baloghadamsoftware added a comment.

Updated according to the comments.


https://reviews.llvm.org/D52727

Files:
  clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tidy/performance/ForRangeCopyCheck.h
  clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tidy/performance/UnnecessaryCopyInitialization.h
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.h
  clang-tidy/utils/Matchers.h
  docs/clang-tidy/checks/performance-for-range-copy.rst
  docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  test/clang-tidy/performance-for-range-copy-allowed-types.cpp
  test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
  test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp

Index: test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+struct OtherType {
+  ~OtherType();
+};
+
+void negativeSmartPointer(SmartPointer P) {
+}
+
+void negative_smart_pointer(smart_pointer p) {
+}
+
+void negativeSmartPtr(SmartPtr P) {
+}
+
+void negative_smart_ptr(smart_ptr p) {
+}
+
+void negativeSmartReference(SmartReference R) {
+}
+
+void negative_smart_reference(smart_reference r) {
+}
+
+void negativeSmartRef(SmartRef R) {
+}
+
+void negative_smart_ref(smart_ref r) {
+}
+
+void positiveOtherType(OtherType O) {
+  // CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]
+  // CHECK-FIXES: void positiveOtherType(const OtherType& O) {
+}
Index: test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+struct OtherType {
+  ~OtherType();
+};
+
+const SmartPointer &getSmartPointer();
+const smart_pointer &get_smart_pointer();
+const SmartPtr &getSmartPtr();
+const smart_ptr &get_smart_ptr();
+const SmartReference &getSmartReference();
+const smart_reference &get_smart_reference();
+const SmartRef &getSmartRef();
+const smart_ref &get_smart_ref();
+const OtherType &getOtherType();
+
+void negativeSmartPointer() {
+  const auto P = getSmartPointer();
+}
+
+void negative_smart_pointer() {
+  const auto p = get_smart_pointer();
+}
+
+void negativeSmartPtr() {
+  const auto P = getSmartPtr();
+}
+
+void negative_smart_ptr() {
+  const auto p = get_smart_ptr();
+}
+
+void negativeSmartReference() {
+  const auto R = getSmartReference();
+}
+
+void negative_smart_reference() {
+  const auto r = get_smart_reference();
+}
+
+void negativeSmartRef() {
+  const auto R = getSmartRef();
+}
+
+void negative_smart_ref() {
+  const auto r = get_smart_ref();
+}
+
+void positiveOtherType() {
+  const auto O = getOtherType();
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const auto& O = getOtherType();
+}
Index: test/clang-tidy/performance-for-range-copy-allowed-types.cpp
===
--- /dev/null
+++ test/clang-tidy/performance-for-range-copy-allowed-types.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -config="{CheckOptions: [{key: performance-fo