[PATCH] D85528: [analyzer] Fix cast evaluation on scoped enums in ExprEngine

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, Szelethus, baloghadamsoftware, vsavchenko, xazax.hun, martong, ASDenysPetrov.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a project: clang.
steakhal requested review of this revision.

We ignored the cast if the enum was **scoped**.
This is bad since there is **no implicit conversion** from the scoped enum **to** the corresponding **underlying type**.

This materialized in crashes on analyzing the LLVM itself using the Z3 refutation.
Refutation synthesized the given Z3 expression with the wrong bitwidth in the end.

Now, we evaluate the cast according to the standard.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85528

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/test/Analysis/z3-refute-enum-crash.cpp


Index: clang/test/Analysis/z3-refute-enum-crash.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/z3-refute-enum-crash.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config crosscheck-with-z3=true -verify %s
+
+void clang_analyzer_warnIfReached();
+
+using sugar_t = unsigned char;
+enum class ScopedSugared : sugar_t {};
+enum class ScopedPrimitive : unsigned char {};
+enum UnscopedSugared : sugar_t {};
+enum UnscopedPrimitive : unsigned char {};
+
+ScopedSugared conjure_scoped_enum_with_sugar_type();
+ScopedPrimitive conjure_scoped_enum_with_primitive_type();
+UnscopedSugared conjure_unscoped_enum_with_sugar_type();
+UnscopedPrimitive conjure_unscoped_enum_with_primitive_type();
+
+void test() {
+  auto var1 = conjure_scoped_enum_with_sugar_type();
+  auto var2 = conjure_scoped_enum_with_primitive_type();
+  auto var3 = conjure_unscoped_enum_with_sugar_type();
+  auto var4 = conjure_unscoped_enum_with_primitive_type();
+
+  int sym1 = static_cast<unsigned char>(var1) & 0x0F;
+  if (sym1)
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym2 = static_cast<unsigned char>(var2) & 0x0F;
+  if (sym2)
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym3 = static_cast<unsigned char>(var3) & 0x0F;
+  if (sym3)
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+
+  int sym4 = static_cast<unsigned char>(var4) & 0x0F;
+  if (sym4)
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} no-crash
+}
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -95,11 +95,22 @@
   }
 
   bool haveSameType(QualType Ty1, QualType Ty2) {
+    const auto IsIntegralOrUnscopedCompleteEnumerationType = [](QualType Ty) {
+      const Type *CanonicalType = Ty.getCanonicalType().getTypePtr();
+      if (const auto *ET = dyn_cast<EnumType>(CanonicalType))
+        return ET->getDecl()->isComplete() && !ET->getDecl()->isScoped();
+
+      return Ty->isIntegralOrEnumerationType();
+    };
+
     // FIXME: Remove the second disjunct when we support symbolic
     // truncation/extension.
-    return (Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2) ||
-            (Ty1->isIntegralOrEnumerationType() &&
-             Ty2->isIntegralOrEnumerationType()));
+    const bool BothHaveSameCanonicalTypes =
+        Context.getCanonicalType(Ty1) == Context.getCanonicalType(Ty2);
+    const bool BothHaveIntegralLikeTypes =
+        IsIntegralOrUnscopedCompleteEnumerationType(Ty1) &&
+        IsIntegralOrUnscopedCompleteEnumerationType(Ty2);
+    return BothHaveSameCanonicalTypes || BothHaveIntegralLikeTypes;
   }
 
   SVal evalCast(SVal val, QualType castTy, QualType originalType);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D85528.283908.patch
Type: text/x-patch
Size: 3082 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200807/0e60b862/attachment.bin>