junaire updated this revision to Diff 485654.
junaire added a comment.
Update patch according to @ymandel 's suggestion. Thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140753/new/
https://reviews.llvm.org/D140753
Files:
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Index:
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2970,6 +2970,23 @@
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)
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -93,7 +93,18 @@
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: This is sort of workaound. Since now we just ignore all
(implicit)
+ // integral casts, treating the resulting value as same as the underling
+ // value, it could cause inconsistency between values after `Join` if in
+ // some paths the type doesn't strictly match.
+ // Eg:
+ // 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(
Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2970,6 +2970,23 @@
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)
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -93,7 +93,18 @@
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: This is sort of workaound. Since now we just ignore all (implicit)
+ // integral casts, treating the resulting value as same as the underling
+ // value, it could cause inconsistency between values after `Join` if in
+ // some paths the type doesn't strictly match.
+ // Eg:
+ // 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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
