baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
Herald added subscribers: mikhail.ramalho, a.sidorin, dkrupp, rnkovacs, szepet,
xazax.hun, whisperity.
Herald added a reviewer: george.karpenkov.
Patch [[https://reviews.llvm.org/rC329780 | [Analyzer] SValBuilder Comparison
Rearrangement (with Restrictions and Analyzer Option) ]] not only rearranges
comparisons but also binary expressions. This latter behavior is not protected
by the analyzer option. Hower, since no complexity threshold is enforced to the
symbols this may result in exponential execution time if the expressions are
too complex: Huge static analysis performance regression for very simple
testcase <https://bugs.llvm.org/show_bug.cgi?id=38208> For a quick fix we
extended the analyzer option to also cover the additive cases.
This is only a temporary fix, the final solution should be enforcing the
complexity threshold to the symbols.
https://reviews.llvm.org/D49536
Files:
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
test/Analysis/PR38208.c
test/Analysis/constraint_manager_negate_difference.c
test/Analysis/iterator-range.cpp
test/Analysis/plist-macros.cpp
test/Analysis/svalbuilder-rearrange-comparisons.c
Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===================================================================
--- test/Analysis/svalbuilder-rearrange-comparisons.c
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-binary-operation-simplification=true -verify %s
void clang_analyzer_dump(int x);
void clang_analyzer_eval(int x);
Index: test/Analysis/plist-macros.cpp
===================================================================
--- test/Analysis/plist-macros.cpp
+++ test/Analysis/plist-macros.cpp
@@ -637,6 +637,69 @@
// CHECK-NEXT: <key>end</key>
// CHECK-NEXT: <array>
// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>event</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <key>ranges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>25</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>depth</key><integer>0</integer>
+// CHECK-NEXT: <key>extended_message</key>
+// CHECK-NEXT: <string>Assuming the condition is true</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Assuming the condition is true</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>kind</key><string>control</string>
+// CHECK-NEXT: <key>edges</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>start</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>36</integer>
+// CHECK-NEXT: <key>col</key><integer>7</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>end</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>37</integer>
// CHECK-NEXT: <key>col</key><integer>5</integer>
// CHECK-NEXT: <key>file</key><integer>0</integer>
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 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: 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=debug.ExprInspection,core.builtin -analyzer-config aggressive-relational-comparison-simplification=true -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection,core.builtin -analyzer-config aggressive-binary-operation-simplification=true -verify %s
void clang_analyzer_eval(int);
Index: test/Analysis/PR38208.c
===================================================================
--- /dev/null
+++ test/Analysis/PR38208.c
@@ -0,0 +1,43 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+
+// expected-no-diagnostics
+
+int foo(int a, int b) {
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ a += b; b -= a;
+ return a + b;
+}
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===================================================================
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -456,14 +456,17 @@
auto &Opts =
StateMgr.getOwningEngine()->getAnalysisManager().getAnalyzerOptions();
+ // FIXME: After putting complexity threshold to the symbols we can always
+ // rearrange additive operations but rearrange comparisons only if
+ // option is set.
+ if(!Opts.shouldAggressivelySimplifyBinaryOperation())
+ return None;
+
SymbolRef LSym = Lhs.getAsSymbol();
if (!LSym)
return None;
- // Always rearrange additive operations but rearrange comparisons only if
- // option is set.
- if (BinaryOperator::isComparisonOp(Op) &&
- Opts.shouldAggressivelySimplifyRelationalComparison()) {
+ if (BinaryOperator::isComparisonOp(Op)) {
SingleTy = LSym->getType();
if (ResultTy != SVB.getConditionType())
return None;
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -463,12 +463,12 @@
return DisplayNotesAsEvents.getValue();
}
-bool AnalyzerOptions::shouldAggressivelySimplifyRelationalComparison() {
- if (!AggressiveRelationalComparisonSimplification.hasValue())
- AggressiveRelationalComparisonSimplification =
- getBooleanOption("aggressive-relational-comparison-simplification",
+bool AnalyzerOptions::shouldAggressivelySimplifyBinaryOperation() {
+ if (!AggressiveBinaryOperationSimplification.hasValue())
+ AggressiveBinaryOperationSimplification =
+ getBooleanOption("aggressive-binary-operation-simplification",
/*Default=*/false);
- return AggressiveRelationalComparisonSimplification.getValue();
+ return AggressiveBinaryOperationSimplification.getValue();
}
StringRef AnalyzerOptions::getCTUDir() {
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -318,8 +318,8 @@
/// \sa shouldDisplayNotesAsEvents
Optional<bool> DisplayNotesAsEvents;
- /// \sa shouldAggressivelySimplifyRelationalComparison
- Optional<bool> AggressiveRelationalComparisonSimplification;
+ /// \sa shouldAggressivelySimplifyBinaryOperation
+ Optional<bool> AggressiveBinaryOperationSimplification;
/// \sa getCTUDir
Optional<StringRef> 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 <REL> B + m becomes A - B <REL> m - n, where A and B symbolic,
- /// n and m are integers. <REL> 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 <OP> B + m becomes A - B <OP> m - n, where A and B symbolic,
+ /// n and m are integers. <OP> 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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
