pp.c: Clip shift amount of <<, >> to avoid overflow

Previously the right operand of bitwise shift operators (shift amount)
was implicitly cast from IV to int, but it might lead wrong results
if IV does not fit in int.
And also, shifting INT_MIN bits used to yield the shiftee unchanged
(treated as 0-bit shift instead of negative shift).

2 files changed, 36 insertions, 3 deletions

diff --git a/pp.c b/pp.c
index baf0777a47..68b4e46156 100644
--- a/pp.c
+++ b/pp.c
@@ -1973,6 +1973,22 @@ PP(pp_subtract)
 
 #define IV_BITS (IVSIZE * 8)
 
+/* Taking the right operand of bitwise shift operators, returns an int
+ * indicating the shift amount clipped to the range [-IV_BITS, +IV_BITS].
+ */
+static int
+S_shift_amount(pTHX_ SV *const svr)
+{
+    const IV iv = SvIV_nomg(svr);
+
+    /* Note that [INT_MIN, INT_MAX] cannot be used as the clipping bound;
+     * INT_MIN will cause overflow in "shift = -shift;" in S_{iv,uv}_shift.
+     */
+    if (SvIsUV(svr))
+        return SvUVX(svr) > IV_BITS ? IV_BITS : (int)SvUVX(svr);
+    return iv < -IV_BITS ? -IV_BITS : iv > IV_BITS ? IV_BITS : (int)iv;
+}
+
 static UV S_uv_shift(UV uv, int shift, bool left)
 {
    if (shift < 0) {
@@ -2026,7 +2042,7 @@ PP(pp_left_shift)
     svr = POPs;
     svl = TOPs;
     {
-      const IV shift = SvIV_nomg(svr);
+      const int shift = S_shift_amount(aTHX_ svr);
       if (PL_op->op_private & HINT_INTEGER) {
           SETi(IV_LEFT_SHIFT(SvIV_nomg(svl), shift));
       }
@@ -2044,7 +2060,7 @@ PP(pp_right_shift)
     svr = POPs;
     svl = TOPs;
     {
-      const IV shift = SvIV_nomg(svr);
+      const int shift = S_shift_amount(aTHX_ svr);
       if (PL_op->op_private & HINT_INTEGER) {
 	  SETi(IV_RIGHT_SHIFT(SvIV_nomg(svl), shift));
       }
diff --git a/t/op/bop.t b/t/op/bop.t
index 31b6531a03..a84992e391 100644
--- a/t/op/bop.t
+++ b/t/op/bop.t
@@ -18,7 +18,7 @@ BEGIN {
 # If you find tests are failing, please try adding names to tests to track
 # down where the failure is, and supply your new names as a patch.
 # (Just-in-time test naming)
-plan tests => 503;
+plan tests => 510;
 
 # numerics
 ok ((0xdead & 0xbeef) == 0x9ead);
@@ -40,6 +40,23 @@ ok ((~0 > 0 && do { use integer; ~0 } == -1));
     is($shifted, $iv_min, "IV_MIN << 0 yields IV_MIN under 'use integer'");
 }
 
+# GH #18691
+# Exercise some corner cases on shifting more bits than the size of IV/UV.
+# All these should work even if the shift amount doesn't fit in IV or UV.
+is(4 << 2147483648, 0, "4 << 2147483648 yields 0");
+is(16 << 4294967295, 0, "16 << 4294967295 yields 0");
+is(8 >> 4294967296, 0, "8 >> 4294967296 yields 0");
+is(11 << 18446744073709551615, 0, "11 << 18446744073709551615 yields 0");
+is(do { use integer; -9 >> 18446744073709551616 }, -1,
+   "-9 >> 18446744073709551616 under 'use integer' yields -1");
+is(do { use integer; -4 << -2147483648 }, -1,
+   "-4 << -2147483648 under 'use integer' yields -1");
+# Quotes around -9223372036854775808 below are to make it a single term.
+# Without quotes, it will be parsed as an expression with an unary minus
+# operator which will clip the result to IV range under "use integer".
+is(do { use integer; -5 >> '-9223372036854775808' }, 0,
+   "-5 >> -9223372036854775808 under 'use integer' yields 0");
+
 my $bits = 0;
 for (my $i = ~0; $i; $i >>= 1) { ++$bits; }
 my $cusp = 1 << ($bits - 1);