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

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

tbaeder updated this revision to Diff 521082.

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,21 @@
   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}}
+  }
+
+  constexpr int negativeShift() { // cxx17-error {{never produces a constant expression}} \
+                                  // ref-cxx17-error {{never produces a constant expression}}
+    return -1 << 2; // cxx17-warning {{shifting a negative signed value is undefined}} \
+                    // ref-cxx17-warning {{shifting a negative signed value is undefined}} \
+                    // cxx17-note {{left shift of negative value -1}} \
+                    // ref-cxx17-note {{left shift of negative value -1}}
+  }
 };
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();
@@ -126,6 +127,20 @@
     S.CCEDiag(E, diag::note_constexpr_large_shift) << Val << Ty << Bits;
     return false;
   }
+
+  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.
+    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);
+  }
+
+  // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
+  // E1 x 2^E2 module 2^N.
+
   return true;
 }
 
@@ -1612,7 +1627,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 +1644,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.521082.patch
Type: text/x-patch
Size: 3960 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230510/04333b08/attachment-0001.bin>