[PATCH] D150209: [clang][Interp] Add more shift error checking

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 520954.

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

https://reviews.llvm.org/D150209

Files:
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/test/AST/Interp/shifts.cpp


Index: clang/test/AST/Interp/shifts.cpp
===================================================================
--- clang/test/AST/Interp/shifts.cpp
+++ clang/test/AST/Interp/shifts.cpp
@@ -152,4 +152,14 @@
   constexpr signed int R = (sizeof(unsigned) * 8) + 1;
   constexpr decltype(L) M  = (R > 32 && R < 64) ?  L << R : 0;
   constexpr decltype(L) M2 = (R > 32 && R < 64) ?  L >> R : 0;
+
+
+  constexpr int signedShift() { // cxx17-error {{never produces a constant expression}} \
+                                // ref-cxx17-error {{never produces a constant expression}}
+    return 1024 << 31; // cxx17-warning {{signed shift result}} \
+                       // ref-cxx17-warning {{signed shift result}} \
+                       // cxx17-note {{signed left shift discards bits}} \
+                       // ref-cxx17-note {{signed left shift discards bits}}
+  }
+
 };
Index: clang/lib/AST/Interp/Interp.h
===================================================================
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -109,8 +109,9 @@
 bool CheckCtorCall(InterpState &S, CodePtr OpPC, const Pointer &This);
 
 /// Checks if the shift operation is legal.
-template <typename RT>
-bool CheckShift(InterpState &S, CodePtr OpPC, const RT &RHS, unsigned Bits) {
+template <typename LT, typename RT>
+bool CheckShift(InterpState &S, CodePtr OpPC, const LT &LHS, const RT &RHS,
+                unsigned Bits) {
   if (RHS.isNegative()) {
     const SourceInfo &Loc = S.Current->getSource(OpPC);
     S.CCEDiag(Loc, diag::note_constexpr_negative_shift) << RHS.toAPSInt();
@@ -125,6 +126,16 @@
     QualType Ty = E->getType();
     S.CCEDiag(E, diag::note_constexpr_large_shift) << Val << Ty << Bits;
     return false;
+  } else if (LHS.isSigned() && !S.getLangOpts().CPlusPlus20) {
+    const Expr *E = S.Current->getExpr(OpPC);
+    // C++11 [expr.shift]p2: A signed left shift must have a non-negative
+    // operand, and must not overflow the corresponding unsigned type.
+    // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
+    // E1 x 2^E2 module 2^N.
+    if (LHS.isNegative())
+      S.CCEDiag(E, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
+    else if (LHS.toUnsigned().countLeadingZeros() < static_cast<unsigned>(RHS))
+      S.CCEDiag(E, diag::note_constexpr_lshift_discards);
   }
   return true;
 }
@@ -1612,7 +1623,7 @@
   const auto &LHS = S.Stk.pop<LT>();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift<RT>(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
     return false;
 
   Integral<LT::bitWidth(), false> R;
@@ -1629,7 +1640,7 @@
   const auto &LHS = S.Stk.pop<LT>();
   const unsigned Bits = LHS.bitWidth();
 
-  if (!CheckShift<RT>(S, OpPC, RHS, Bits))
+  if (!CheckShift(S, OpPC, LHS, RHS, Bits))
     return false;
 
   Integral<LT::bitWidth(), false> R;
Index: clang/lib/AST/Interp/Integral.h
===================================================================
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -127,7 +127,11 @@
     return Compare(V, RHS.V);
   }
 
-  unsigned countLeadingZeros() const { return llvm::countl_zero<ReprT>(V); }
+  unsigned countLeadingZeros() const {
+    if constexpr (!Signed)
+      return llvm::countl_zero<ReprT>(V);
+    llvm_unreachable("Don't call countLeadingZeros() on signed types.");
+  }
 
   Integral truncate(unsigned TruncBits) const {
     if (TruncBits >= Bits)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150209.520954.patch
Type: text/x-patch
Size: 3466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230510/96437dea/attachment-0001.bin>