Properly handle parsing failure in FormulaEvaluator

Fixed a segmentation fault when the FormulaEvaluator was unable to fully parse an expression which contained multiple binary operators.
cms-sw · Feb 26, 2018 · 8b117c5 · 8b117c5
1 parent 20de6fd
commit 8b117c5
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 0 deletions.
diff --git a/CommonTools/Utils/src/FormulaEvaluator.cc b/CommonTools/Utils/src/FormulaEvaluator.cc
@@ -297,6 +297,11 @@ namespace {
         fullExpression.evaluator.reset();
       }
 
+      if(fullExpression.evaluator == nullptr) {
+        //we had a parsing problem
+        return fullExpression;
+      }
+
       //Now to handle precedence
       auto topNode = fullExpression.top;
       auto binaryEval = dynamic_cast<reco::formula::BinaryOperatorEvaluatorBase*>(fullExpression.evaluator.get());

diff --git a/CommonTools/Utils/test/testFormulaEvaluator.cc b/CommonTools/Utils/test/testFormulaEvaluator.cc
@@ -9,6 +9,8 @@
 
 #include "CommonTools/Utils/interface/FormulaEvaluator.h"
 
+#include "FWCore/Utilities/interface/Exception.h"
+
 #include <algorithm>
 #include <cmath>
 #include "TMath.h"
@@ -895,4 +897,61 @@ testFormulaEvaluator::checkFormulaEvaluator() {
     }
   }
 
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("doesNotExist(2)");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("doesNotExist(2) + abs(-1)");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("abs(-1) + doesNotExist(2)");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("abs(-1) + ( 5 * doesNotExist(2))");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("( 5 * doesNotExist(2)) + abs(-1)");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    auto t = [] () {
+      reco::FormulaEvaluator f("TMath::Exp(2)");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
+  {
+    //this was previously causing a seg fault
+    auto t = [] () {
+      reco::FormulaEvaluator f("1 + 2 * 3 + 5 * doesNotExist(2) ");
+    };
+
+    CPPUNIT_ASSERT_THROW( t(), cms::Exception );
+  }
+
 }