Author: Jun Zhang Date: 2022-12-30T13:14:44+08:00 New Revision: eda2eaabf2949c08ba94c92b9aad6fccb3c8eaa2
URL: https://github.com/llvm/llvm-project/commit/eda2eaabf2949c08ba94c92b9aad6fccb3c8eaa2 DIFF: https://github.com/llvm/llvm-project/commit/eda2eaabf2949c08ba94c92b9aad6fccb3c8eaa2.diff LOG: [clang][dataflow] Fix crash when having boolean-to-integral casts. 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 <j...@junz.org> Differential Revision: https://reviews.llvm.org/D140753 Added: Modified: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index c883f90f5554b..b8e3e93390602 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -93,7 +93,19 @@ static Value *mergeDistinctValues(QualType Type, Value &Val1, Environment::ValueModel &Model) { // Join distinct boolean values preserving information about the constraints // in the respective path conditions. - if (auto *Expr1 = dyn_cast<BoolValue>(&Val1)) { + if (Type->isBooleanType()) { + // FIXME: The type check above is a workaround and should be unnecessary. + // However, right now we can end up with BoolValue's in integer-typed + // variables due to our incorrect handling of boolean-to-integer casts (we + // just propagate the BoolValue to the result of the cast). For example: + // std::optional<bool> o; + // + // + // int x; + // if (o.has_value()) { + // x = o.value(); + // } + auto *Expr1 = cast<BoolValue>(&Val1); auto *Expr2 = cast<BoolValue>(&Val2); auto &MergedVal = MergedEnv.makeAtomicBoolValue(); MergedEnv.addToFlowCondition(MergedEnv.makeOr( diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp index 4d9c57f0dacd5..1fcede5d62865 100644 --- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp @@ -2970,6 +2970,23 @@ TEST_P(UncheckedOptionalAccessTest, CtorInitializerValue) { 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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits