[bigint] Fix Karatsuba intermediate result length

When adding up the results of the recursive steps, the Karatsuba
algorithm can temporarily have intermediate results that are one
bit bigger than the final result. This patch makes sure we handle
that case correctly.
Since that extra bit would always get subtracted again, the old
code would not have caused incorrect results or memory corruption,
but it did run into DCHECK-failures, and potentially could have
caused segfaults.

Bug: v8:11515, chromium:1223724
Change-Id: I3592835d01cc36def8f0a9bae625e9249864ef78
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2988758
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75509}
This commit is contained in:
Jakob Kummerow 2021-06-25 16:34:39 +02:00 committed by V8 LUCI CQ
parent 337d53e654
commit a4b573e5cc
4 changed files with 36 additions and 19 deletions

View File

@ -102,7 +102,7 @@ void ProcessorImpl::KaratsubaStart(RWDigits Z, Digits X, Digits Y,
if (Y1.len() > 0) { if (Y1.len() > 0) {
KaratsubaChunk(T, X0, Y1, scratch); KaratsubaChunk(T, X0, Y1, scratch);
if (should_terminate()) return; if (should_terminate()) return;
AddAt(Z + k, T); AddAndReturnOverflow(Z + k, T); // Can't overflow.
} }
// Add Xi * Y0 << i and Xi * Y1 * b << (i + k). // Add Xi * Y0 << i and Xi * Y1 * b << (i + k).
@ -111,11 +111,11 @@ void ProcessorImpl::KaratsubaStart(RWDigits Z, Digits X, Digits Y,
Digits Xi(X, i, k); Digits Xi(X, i, k);
KaratsubaChunk(T, Xi, Y0, scratch); KaratsubaChunk(T, Xi, Y0, scratch);
if (should_terminate()) return; if (should_terminate()) return;
AddAt(Z + i, T); AddAndReturnOverflow(Z + i, T); // Can't overflow.
if (Y1.len() > 0) { if (Y1.len() > 0) {
KaratsubaChunk(T, Xi, Y1, scratch); KaratsubaChunk(T, Xi, Y1, scratch);
if (should_terminate()) return; if (should_terminate()) return;
AddAt(Z + (i + k), T); AddAndReturnOverflow(Z + (i + k), T); // Can't overflow.
} }
} }
} }
@ -163,14 +163,16 @@ void ProcessorImpl::KaratsubaMain(RWDigits Z, Digits X, Digits Y,
RWDigits P2(scratch, n, n); RWDigits P2(scratch, n, n);
KaratsubaMain(P2, X1, Y1, scratch_for_recursion, n2); KaratsubaMain(P2, X1, Y1, scratch_for_recursion, n2);
if (should_terminate()) return; if (should_terminate()) return;
RWDigits Z1 = Z + n; RWDigits Z2 = Z + n;
int end = std::min(Z1.len(), P2.len()); int end = std::min(Z2.len(), P2.len());
for (int i = 0; i < end; i++) Z1[i] = P2[i]; for (int i = 0; i < end; i++) Z2[i] = P2[i];
for (int i = end; i < n; i++) { for (int i = end; i < n; i++) {
DCHECK(P2[i] == 0); // NOLINT(readability/check) DCHECK(P2[i] == 0); // NOLINT(readability/check)
} }
AddAt(Z + n2, P0); // The intermediate result can be one digit too large; the subtraction
AddAt(Z + n2, P2); // below will fix this.
digit_t overflow = AddAndReturnOverflow(Z + n2, P0);
overflow += AddAndReturnOverflow(Z + n2, P2);
RWDigits X_diff(scratch, 0, n2); RWDigits X_diff(scratch, 0, n2);
RWDigits Y_diff(scratch, n2, n2); RWDigits Y_diff(scratch, n2, n2);
int sign = 1; int sign = 1;
@ -179,10 +181,12 @@ void ProcessorImpl::KaratsubaMain(RWDigits Z, Digits X, Digits Y,
RWDigits P1(scratch, n, n); RWDigits P1(scratch, n, n);
KaratsubaMain(P1, X_diff, Y_diff, scratch_for_recursion, n2); KaratsubaMain(P1, X_diff, Y_diff, scratch_for_recursion, n2);
if (sign > 0) { if (sign > 0) {
AddAt(Z + n2, P1); overflow += AddAndReturnOverflow(Z + n2, P1);
} else { } else {
SubAt(Z + n2, P1); overflow -= SubAndReturnBorrow(Z + n2, P1);
} }
// The intermediate result may have been bigger, but the final result fits.
DCHECK(overflow == 0); // NOLINT(readability/check)
} }
} // namespace bigint } // namespace bigint

View File

@ -10,29 +10,32 @@
namespace v8 { namespace v8 {
namespace bigint { namespace bigint {
void AddAt(RWDigits Z, Digits X) { digit_t AddAndReturnOverflow(RWDigits Z, Digits X) {
X.Normalize(); X.Normalize();
if (X.len() == 0) return; if (X.len() == 0) return 0;
digit_t carry = 0; digit_t carry = 0;
int i = 0; int i = 0;
for (; i < X.len(); i++) { for (; i < X.len(); i++) {
Z[i] = digit_add3(Z[i], X[i], carry, &carry); Z[i] = digit_add3(Z[i], X[i], carry, &carry);
} }
for (; carry != 0; i++) { for (; i < Z.len() && carry != 0; i++) {
Z[i] = digit_add2(Z[i], carry, &carry); Z[i] = digit_add2(Z[i], carry, &carry);
} }
return carry;
} }
void SubAt(RWDigits Z, Digits X) { digit_t SubAndReturnBorrow(RWDigits Z, Digits X) {
X.Normalize(); X.Normalize();
if (X.len() == 0) return 0;
digit_t borrow = 0; digit_t borrow = 0;
int i = 0; int i = 0;
for (; i < X.len(); i++) { for (; i < X.len(); i++) {
Z[i] = digit_sub2(Z[i], X[i], borrow, &borrow); Z[i] = digit_sub2(Z[i], X[i], borrow, &borrow);
} }
for (; borrow != 0; i++) { for (; i < Z.len() && borrow != 0; i++) {
Z[i] = digit_sub(Z[i], borrow, &borrow); Z[i] = digit_sub(Z[i], borrow, &borrow);
} }
return borrow;
} }
void Add(RWDigits Z, Digits X, Digits Y) { void Add(RWDigits Z, Digits X, Digits Y) {

View File

@ -13,11 +13,11 @@
namespace v8 { namespace v8 {
namespace bigint { namespace bigint {
// Z += X. // Z += X. Returns carry on overflow.
void AddAt(RWDigits Z, Digits X); digit_t AddAndReturnOverflow(RWDigits Z, Digits X);
// Z -= X. // Z -= X. Returns borrow on overflow.
void SubAt(RWDigits Z, Digits X); digit_t SubAndReturnBorrow(RWDigits Z, Digits X);
// Z := X + Y. // Z := X + Y.
void Add(RWDigits Z, Digits X, Digits Y); void Add(RWDigits Z, Digits X, Digits Y);

View File

@ -0,0 +1,10 @@
// Copyright 2021 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Regression test for crbug.com/1223724
const kBits = 500 * 64;
const a = BigInt.asUintN(kBits, -1n);
const b = a * a;
const expected = (a << BigInt(kBits)) - a;
assertEquals(expected, b);