[clang] fa34210 - [clang][dataflow] Do not allow substitution of true/false boolean literals in `buildAndSubstituteFlowCondition`

[Dmitri Gribenko via cfe-commits]

Author: Wei Yi Tee
Date: 2022-06-27T21:04:52+02:00
New Revision: fa34210fa69f64a96dc64983b3de00ddd21e55e1

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

LOG: [clang][dataflow] Do not allow substitution of true/false boolean literals in `buildAndSubstituteFlowCondition`

Reviewed By: gribozavr2, xazax.hun

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

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
    clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 4c7f0d1f94fa7..e08fc71c51dc7 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -220,8 +220,12 @@ BoolValue &DataflowAnalysisContext::substituteBoolValue(
     llvm::DenseMap<BoolValue *, BoolValue *> &SubstitutionsCache) {
   auto IT = SubstitutionsCache.find(&Val);
   if (IT != SubstitutionsCache.end()) {
+    // Return memoized result of substituting this boolean value.
     return *IT->second;
   }
+
+  // Handle substitution on the boolean value (and its subvalues), saving the
+  // result into `SubstitutionsCache`.
   BoolValue *Result;
   switch (Val.getKind()) {
   case Value::Kind::AtomicBool: {
@@ -262,6 +266,10 @@ BoolValue &DataflowAnalysisContext::substituteBoolValue(
 BoolValue &DataflowAnalysisContext::buildAndSubstituteFlowCondition(
     AtomicBoolValue &Token,
     llvm::DenseMap<AtomicBoolValue *, BoolValue *> Substitutions) {
+  assert(
+      Substitutions.find(&getBoolLiteralValue(true)) == Substitutions.end() &&
+      Substitutions.find(&getBoolLiteralValue(false)) == Substitutions.end() &&
+      "Do not substitute true/false boolean literals");
   llvm::DenseMap<BoolValue *, BoolValue *> SubstitutionsCache(
       Substitutions.begin(), Substitutions.end());
   return buildAndSubstituteFlowConditionWithCache(Token, SubstitutionsCache);

diff  --git a/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
index 26bc37bda617b..758b1a8b21a2b 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
@@ -276,6 +276,34 @@ TEST_F(DataflowAnalysisContextTest, EquivBoolVals) {
       Context.getOrCreateConjunction(X, Context.getOrCreateConjunction(Y, Z))));
 }
 
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST
+TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsTrueUnchanged) {
+  auto &True = Context.getBoolLiteralValue(true);
+  auto &Other = Context.createAtomicBoolValue();
+
+  // FC = True
+  auto &FC = Context.makeFlowConditionToken();
+  Context.addFlowConditionConstraint(FC, True);
+
+  // `True` should never be substituted
+  EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&True, &Other}}),
+               "Do not substitute true/false boolean literals");
+}
+
+TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsFalseUnchanged) {
+  auto &False = Context.getBoolLiteralValue(false);
+  auto &Other = Context.createAtomicBoolValue();
+
+  // FC = False
+  auto &FC = Context.makeFlowConditionToken();
+  Context.addFlowConditionConstraint(FC, False);
+
+  // `False` should never be substituted
+  EXPECT_DEATH(Context.buildAndSubstituteFlowCondition(FC, {{&False, &Other}}),
+               "Do not substitute true/false boolean literals");
+}
+#endif
+
 TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsAtomicFC) {
   auto &X = Context.createAtomicBoolValue();
   auto &True = Context.getBoolLiteralValue(true);