[Analyzer] Quick Fix for exponential execution time when simpilifying complex additiv...

Patch https://reviews.llvm.org/rC329780 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:
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.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@337678 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index aa59d6ff538777392634aba9f6a2c649f497a594..9d292cfddb0c8f48fb64006352801f5fa46ac655 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -318,8 +318,8 @@ private:
   /// \sa shouldDisplayNotesAsEvents
   Optional<bool> DisplayNotesAsEvents;
 
-  /// \sa shouldAggressivelySimplifyRelationalComparison
-  Optional<bool> AggressiveRelationalComparisonSimplification;
+  /// \sa shouldAggressivelySimplifyBinaryOperation
+  Optional<bool> AggressiveBinaryOperationSimplification;
 
   /// \sa getCTUDir
   Optional<StringRef> CTUDir;
@@ -690,19 +690,19 @@ public:
   /// 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();

diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index eac07e659dca36b75deed4fd7df7a59c793bdf5d..9b2dc32e06004546cdb4ffb677d353a79bc3335d 100644 (file)
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -463,12 +463,12 @@ bool AnalyzerOptions::shouldDisplayNotesAsEvents() {
   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() {

diff --git a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 7163e380e6c0abf9fdd3922018050bf102afac46..beae0dfae28963b9d96195f8dca1fdb3079f2f32 100644 (file)
--- a/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -456,14 +456,17 @@ static Optional<NonLoc> tryRearrange(ProgramStateRef State,
   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;

diff --git a/test/Analysis/constraint_manager_negate_difference.c b/test/Analysis/constraint_manager_negate_difference.c
index 6856f48bb04ff1e0f6ddd263f5de925dd8f8bcd9..2236c1693b90e290737dd7325f698ccd58126c96 100644 (file)
--- a/test/Analysis/constraint_manager_negate_difference.c
+++ b/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);
 

diff --git a/test/Analysis/iterator-range.cpp b/test/Analysis/iterator-range.cpp
index 45d9f194d4caa6bb24d438a68260448d9e292724..8a4ba29b0cd764776c2c9b68d2ec0e0cd2127acd 100644 (file)
--- a/test/Analysis/iterator-range.cpp
+++ b/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"
 

diff --git a/test/Analysis/plist-macros.cpp b/test/Analysis/plist-macros.cpp
index 53f7a19c13fa2a061bed45adcc22b3a9369d940a..4f7d3a25457435b4fb243bbd4d0d5851948f3af5 100644 (file)
--- a/test/Analysis/plist-macros.cpp
+++ b/test/Analysis/plist-macros.cpp
@@ -637,6 +637,69 @@ void test2(int *p) {
 // 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>

diff --git a/test/Analysis/svalbuilder-rearrange-comparisons.c b/test/Analysis/svalbuilder-rearrange-comparisons.c
index 2303ce693ca073e919a672ffba81c7f3cf613d86..ac186120fe11fcf83f22cb63300e4c9ea68443de 100644 (file)
--- a/test/Analysis/svalbuilder-rearrange-comparisons.c
+++ b/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);