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

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 440333.
wyt added a comment.

Add macro for only testing asserts in debug mode.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128658/new/

https://reviews.llvm.org/D128658

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


Index: clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
@@ -16,6 +16,7 @@
 
 using namespace clang;
 using namespace dataflow;
+using testing::_;
 
 class DataflowAnalysisContextTest : public ::testing::Test {
 protected:
@@ -276,6 +277,34 @@
       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}}),
+               _);
+}
+
+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}}),
+               _);
+}
+#endif
+
 TEST_F(DataflowAnalysisContextTest, SubstituteFlowConditionsAtomicFC) {
   auto &X = Context.createAtomicBoolValue();
   auto &True = Context.getBoolLiteralValue(true);
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -174,8 +174,12 @@
     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: {
@@ -216,6 +220,10 @@
 BoolValue &DataflowAnalysisContext::buildAndSubstituteFlowCondition(
     AtomicBoolValue &Token,
     llvm::DenseMap<AtomicBoolValue *, BoolValue *> Substitutions) {
+  // Do not substitute true/false boolean literals.
+  assert(
+      Substitutions.find(&getBoolLiteralValue(true)) == Substitutions.end() &&
+      Substitutions.find(&getBoolLiteralValue(false)) == Substitutions.end());
   llvm::DenseMap<BoolValue *, BoolValue *> SubstitutionsCache(
       Substitutions.begin(), Substitutions.end());
   return buildAndSubstituteFlowConditionWithCache(Token, SubstitutionsCache);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D128658.440333.patch
Type: text/x-patch
Size: 3037 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220627/c04af51b/attachment.bin>