[bigint] Fix ignored bit in recursive FFT multiplication
When the FFT multiplication algorithm invokes itself for the recursive steps, the input is "mod Fn"-normalized, i.e. it is at most of the shape (1 << N), but we only read N bits of it, so in the rare case where it was exactly 1 << N, that lone top bit was ignored, leading to an incorrect result of the overall multiplication. Fixed: chromium:1228267 Change-Id: I7b245fc3701696d95e5d75fb970f02d72ce40ff8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3032081 Reviewed-by: Maya Lekova <mslekova@chromium.org> Commit-Queue: Jakob Kummerow <jkummerow@chromium.org> Cr-Commit-Position: refs/heads/master@{#75755}
This commit is contained in:
parent
bee1543ef0
commit
cdb882518d
@ -496,6 +496,16 @@ void FFTContainer::Start_Default(Digits X, int chunk_size, int theta,
|
||||
int i = 0;
|
||||
for (; i < n_ && len > 0; i++, current_theta += theta) {
|
||||
chunk_size = std::min(chunk_size, len);
|
||||
// For invocations via MultiplyFFT_Inner, X.len() == n_ * chunk_size + 1,
|
||||
// because the outer layer's "K" is passed as the inner layer's "N".
|
||||
// Since X is (mod Fn)-normalized on the outer layer, there is the rare
|
||||
// corner case where X[n_ * chunk_size] == 1. Detect that case, and handle
|
||||
// the extra bit as part of the last chunk; we always have the space.
|
||||
if (i == n_ - 1 && len == chunk_size + 1) {
|
||||
DCHECK(X[n_ * chunk_size] <= 1); // NOLINT(readability/check)
|
||||
DCHECK(length_ >= chunk_size + 1);
|
||||
chunk_size++;
|
||||
}
|
||||
if (current_theta != 0) {
|
||||
// Multiply with theta^i, and reduce modulo 2^K + 1.
|
||||
// We pass theta as a shift amount; it really means 2^theta.
|
||||
@ -507,6 +517,7 @@ void FFTContainer::Start_Default(Digits X, int chunk_size, int theta,
|
||||
pointer += chunk_size;
|
||||
len -= chunk_size;
|
||||
}
|
||||
DCHECK(len == 0); // NOLINT(readability/check)
|
||||
for (; i < n_; i++) {
|
||||
memset(part_[i], 0, part_length_in_bytes);
|
||||
}
|
||||
|
23
test/mjsunit/harmony/bigint/regress-fftmul-2.js
Normal file
23
test/mjsunit/harmony/bigint/regress-fftmul-2.js
Normal file
@ -0,0 +1,23 @@
|
||||
// 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.
|
||||
|
||||
function fastAssertEquals(expected, actual) {
|
||||
if (expected === actual) return;
|
||||
// Formatting large BigInts to base-10 string is slow and verbose, so format
|
||||
// them to hex strings and report only a prefix.
|
||||
throw new MjsUnitAssertionError(
|
||||
'Failure:\nexpected:\n0x' +
|
||||
expected.toString(16).substring(0, 1000) +
|
||||
'...n\nfound:\n0x' +
|
||||
actual.toString(16).substring(0, 1000) + '...n');
|
||||
}
|
||||
|
||||
function regress_1228267(power) {
|
||||
let a = 2n ** power;
|
||||
let a_squared = a * a;
|
||||
let expected = 2n ** (2n * power);
|
||||
fastAssertEquals(expected, a_squared);
|
||||
}
|
||||
regress_1228267(1273000n); // This triggered the bug on 32-bit platforms.
|
||||
regress_1228267(2564000n); // This triggered the bug on 64-bit platforms.
|
Loading…
Reference in New Issue
Block a user