[clang][dataflow] Fix crash when having boolean-to-integral casts.

[junaire]

Summary

Since now we just ignore all (implicit) integral casts, treating the
resulting value as the same as the underlying value, it could cause
inconsistency between values after Join if in some paths the type
doesn't strictly match. This could cause intermittent crashes.

std::optional<bool> o;
int x;
if (o.has_value()) {
  x = o.value();
}

Fixes: https://github.com/llvm/llvm-project/issues/59728

Signed-off-by: Jun Zhang <[e-mail]>

[gribozavr2]

Thank you for your contribution!

While adding a conditional check fixes the crash, the problem's root cause must be deeper. Mismatched types indicate that one code path in dataflow analysis computes a bool type for a storage location, while a different code path computes an integer type. That's the actual root cause. Could you try to investigate the reasons for that, and try to fix it? The dataflow analysis should be computing values of the same type no matter through which path we arrived at a program point.

[junaire]

In D140753#4019322, @gribozavr2 wrote:

Thank you for your contribution!

While adding a conditional check fixes the crash, the problem's root cause must be deeper. Mismatched types indicate that one code path in dataflow analysis computes a bool type for a storage location, while a different code path computes an integer type. That's the actual root cause. Could you try to investigate the reasons for that, and try to fix it? The dataflow analysis should be computing values of the same type no matter through which path we arrived at a program point.

Thanks for your thoughts about the bug. Sure I'd love to try to investigate and will let you know what I finally got :)

[junaire]

Update patch according to @ymandel 's suggestion. Thanks!

[junaire]

Fix typos

[gribozavr2]

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

97–106

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

 static Value *mergeDistinctValues(QualType Type, Value &Val1,
                                   const Environment &Env1, Value &Val2,
                                   const Environment &Env2,
                                   Environment &MergedEnv,
                                   Environment::ValueModel &Model) {
   // Join distinct boolean values preserving information about the constraints
   // in the respective path conditions.
   if (auto *Expr1 = dyn_cast<BoolValue>(&Val1)) {
-    auto *Expr2 = cast<BoolValue>(&Val2);
+    if (auto *Expr2 = dyn_cast<BoolValue>(&Val2)) {
       auto &MergedVal = MergedEnv.makeAtomicBoolValue();
       MergedEnv.addToFlowCondition(MergedEnv.makeOr(
           MergedEnv.makeAnd(Env1.getFlowConditionToken(),
                             MergedEnv.makeIff(MergedVal, *Expr1)),
           MergedEnv.makeAnd(Env2.getFlowConditionToken(),
                             MergedEnv.makeIff(MergedVal, *Expr2))));
       return &MergedVal;
     }
+  }
 
   // FIXME: Consider destroying `MergedValue` immediately if `ValueModel::merge`
   // returns false to avoid storing unneeded values in `DACtx`.
   if (Value *MergedVal = MergedEnv.createValue(Type))
     if (Model.merge(Type, Val1, Env1, Val2, Env2, *MergedVal, MergedEnv))
       return MergedVal;
 
   return nullptr;

clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

struct Target {
         opt.value();
       }
       $ns::$optional<int> opt;
     };
   )",
       cxxConstructorDecl(ofClass(hasName("Target"))));
 }
 
+// This is regression test, it shouldn't crash.
+TEST_P(UncheckedOptionalAccessTest, Bitfield) {
+  using namespace ast_matchers;
+  ExpectDiagnosticsFor(
+      R"(
+    #include "unchecked_optional_access_test.h"
+    struct Dst {
+      unsigned int n : 1;
+    };
+    void target() {
+      $ns::$optional<bool> v;
+      Dst d;
+      if (v.has_value())
+        d.n = v.value();
+    }
+  )");
+}
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
 // - invalidation (passing optional by non-const reference/pointer)