Skip to content

Commit

Permalink
Fix coredump with mul_I -> div_I ops
Browse files Browse the repository at this point in the history
Fixes R#2280 rakudo/rakudo#2280

The issue in the ticket stems from -2147483648 being big enough to fit into
32bit int, yet its negation, 2147483648, being 1 bit too big. The assumption
in the small-int portion of the div_I op is that if we divide MVMint32 by
MVMint32, we'll always get an MVMint32-sized result, however if we divide
-2147483648 by -1—both of which are MVMint32-sized, we get 2147483648 that's
1 bit too big to fit into the result.

We fix that issue by using MVMint64 for the result along with casts to ensure
we upgrade to big-enough size soon enough.

Next, comes the question of why
`nqp::div_I(nqp::mul_I(1073741824, -2, Int), -1, Int)` crashes, yet if you
manually calculate `nqp::div_I(-2147483648, -1, Int)`, you no longer get the
crash. This is because in the former case, we start with two small-ints,
the mul_I op process them and since the -2147483648 result is big enough
to fit into a 32-bit int, it gives a small-int back.

In the latter case, however, we start off with a 2147483648—a bit larger than
a small-int, so it's a big-int—and feed it to the negation op, which simply sets
the sign of the big-int and gives us a big-int back. Now at div_I op, instead
of two small-ints, we have one big-int, so we go through the big-int path
and we never hit the small-int path that has a bug.
  • Loading branch information
zoffixznet committed Sep 25, 2018
1 parent 08675c0 commit ad3a80c
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/math/bigintops.c
Expand Up @@ -647,7 +647,7 @@ MVMObject *MVM_bigint_div(MVMThreadContext *tc, MVMObject *result_type, MVMObjec
} else {
MVMint32 num = ba->u.smallint.value;
MVMint32 denom = bb->u.smallint.value;
MVMint32 value;
MVMint64 value;
if ((cmp_a == MP_LT) ^ (cmp_b == MP_LT)) {
if (denom == 0) {
MVM_exception_throw_adhoc(tc, "Division by zero");
Expand All @@ -658,7 +658,7 @@ MVMObject *MVM_bigint_div(MVMThreadContext *tc, MVMObject *result_type, MVMObjec
value = num / denom;
}
} else {
value = num / denom;
value = (MVMint64)num / denom;
}
store_int64_result(bc, value);
}
Expand Down

0 comments on commit ad3a80c

Please sign in to comment.