Skip to content

Commit ed284c1

Browse files
jacobly0jacobly0
committed
big.int: fix yet another truncate bug
Too many bugs have been found with `truncate` at this point, so it was rewritten from scratch. Based on the doc comment, the utility of `convertToTwosComplement` over `r.truncate(a, .unsigned, bit_count)` is unclear and it has a subtle behavior difference that is almost certainly a bug, so it was deleted.
1 parent 9c9d393 commit ed284c1

File tree

3 files changed

+109
-114
lines changed

3 files changed

+109
-114
lines changed

‎lib/std/math/big/int.zig

Lines changed: 42 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,119 +1755,60 @@ pub const Mutable = struct {
17551755
y.shiftRight(y.toConst(), norm_shift);
17561756
}
17571757

1758-
/// If a is positive, this passes through to truncate.
1759-
/// If a is negative, then r is set to positive with the bit pattern ~(a - 1).
1760-
/// r may alias a.
1761-
///
1762-
/// Asserts `r` has enough storage to store the result.
1763-
/// The upper bound is `calcTwosCompLimbCount(a.len)`.
1764-
pub fn convertToTwosComplement(r: *Mutable, a: Const, signedness: Signedness, bit_count: usize) void {
1765-
if (a.positive) {
1766-
r.truncate(a, signedness, bit_count);
1767-
return;
1768-
}
1769-
1770-
const req_limbs = calcTwosCompLimbCount(bit_count);
1771-
if (req_limbs == 0 or a.eqlZero()) {
1772-
r.set(0);
1773-
return;
1774-
}
1775-
1776-
const bit = @as(Log2Limb, @truncate(bit_count - 1));
1777-
const signmask = @as(Limb, 1) << bit;
1778-
const mask = (signmask << 1) -% 1;
1779-
1780-
r.addScalar(a.abs(), -1);
1781-
if (req_limbs > r.len) {
1782-
@memset(r.limbs[r.len..req_limbs], 0);
1783-
}
1784-
1785-
assert(r.limbs.len >= req_limbs);
1786-
r.len = req_limbs;
1787-
1788-
llnot(r.limbs[0..r.len]);
1789-
r.limbs[r.len - 1] &= mask;
1790-
r.normalize(r.len);
1791-
}
1792-
17931758
/// Truncate an integer to a number of bits, following 2s-complement semantics.
1794-
/// r may alias a.
1759+
/// `r` may alias `a`.
17951760
///
1796-
/// Asserts `r` has enough storage to store the result.
1761+
/// Asserts `r` has enough storage to compute the result.
17971762
/// The upper bound is `calcTwosCompLimbCount(a.len)`.
17981763
pub fn truncate(r: *Mutable, a: Const, signedness: Signedness, bit_count: usize) void {
1799-
const req_limbs = calcTwosCompLimbCount(bit_count);
1800-
const abs_trunc_a: Const = .{
1801-
.positive = true,
1802-
.limbs = a.limbs[0..@min(a.limbs.len, req_limbs)],
1803-
};
1804-
18051764
// Handle 0-bit integers.
1806-
if (req_limbs == 0 or abs_trunc_a.eqlZero()) {
1765+
if (bit_count == 0) {
1766+
@branchHint(.unlikely);
18071767
r.set(0);
18081768
return;
18091769
}
18101770

1811-
const bit = @as(Log2Limb, @truncate(bit_count - 1));
1812-
const signmask = @as(Limb, 1) << bit; // 0b0..010...0 where 1 is the sign bit.
1813-
const mask = (signmask << 1) -% 1; // 0b0..01..1 where the leftmost 1 is the sign bit.
1814-
1815-
if (!a.positive) {
1816-
// Convert the integer from sign-magnitude into twos-complement.
1817-
// -x = ~(x - 1)
1818-
// Note, we simply take req_limbs * @bitSizeOf(Limb) as the
1819-
// target bit count.
1820-
1821-
r.addScalar(abs_trunc_a, -1);
1771+
const max_limbs = calcTwosCompLimbCount(bit_count);
1772+
const sign_bit = @as(Limb, 1) << @truncate(bit_count - 1);
1773+
const mask = @as(Limb, maxInt(Limb)) >> @truncate(-%bit_count);
1774+
1775+
// Guess whether the result will have the same sign as `a`.
1776+
// * If the result will be signed zero, the guess is `true`.
1777+
// * If the result will be the minimum signed integer, the guess is `false`.
1778+
// * If the result will be unsigned zero, the guess is `a.positive`.
1779+
// * Otherwise the guess is correct.
1780+
const same_sign_guess = switch (signedness) {
1781+
.signed => max_limbs > a.limbs.len or a.limbs[max_limbs - 1] & sign_bit == 0,
1782+
.unsigned => a.positive,
1783+
};
18221784

1823-
// Zero-extend the result
1824-
@memset(r.limbs[r.len..req_limbs], 0);
1825-
r.len = req_limbs;
1826-
1827-
// Without truncating, we can already peek at the sign bit of the result here.
1828-
// Note that it will be 0 if the result is negative, as we did not apply the flip here.
1829-
// If the result is negative, we have
1830-
// -(-x & mask)
1831-
// = ~(~(x - 1) & mask) + 1
1832-
// = ~(~((x - 1) | ~mask)) + 1
1833-
// = ((x - 1) | ~mask)) + 1
1834-
// Note, this is only valid for the target bits and not the upper bits
1835-
// of the most significant limb. Those still need to be cleared.
1836-
// Also note that `mask` is zero for all other bits, reducing to the identity.
1837-
// This means that we still need to use & mask to clear off the upper bits.
1838-
1839-
if (signedness == .signed and r.limbs[r.len - 1] & signmask == 0) {
1840-
// Re-add the one and negate to get the result.
1841-
r.limbs[r.len - 1] &= mask;
1842-
// Note, addition cannot require extra limbs here as we did a subtraction before.
1843-
r.addScalar(r.toConst(), 1);
1844-
r.normalize(r.len);
1845-
r.positive = false;
1846-
} else {
1847-
llnot(r.limbs[0..r.len]);
1848-
r.limbs[r.len - 1] &= mask;
1849-
r.normalize(r.len);
1850-
}
1851-
} else {
1785+
const abs_trunc_a: Const = .{
1786+
.positive = true,
1787+
.limbs = a.limbs[0..llnormalize(a.limbs[0..@min(a.limbs.len, max_limbs)])],
1788+
};
1789+
if (same_sign_guess or abs_trunc_a.eqlZero()) {
1790+
// One of the following is true:
1791+
// * The result is zero.
1792+
// * The result is non-zero and has the same sign as `a`.
18521793
r.copy(abs_trunc_a);
1853-
// If the integer fits within target bits, no wrapping is required.
1854-
if (r.len < req_limbs) return;
1855-
1856-
r.limbs[r.len - 1] &= mask;
1794+
if (max_limbs <= r.len) r.limbs[max_limbs - 1] &= mask;
18571795
r.normalize(r.len);
1858-
1859-
if (signedness == .signed and r.limbs[r.len - 1] & signmask != 0) {
1860-
// Convert 2s-complement back to sign-magnitude.
1861-
// Sign-extend the upper bits so that they are inverted correctly.
1862-
r.limbs[r.len - 1] |= ~mask;
1863-
llnot(r.limbs[0..r.len]);
1864-
1865-
// Note, can only overflow if r holds 0xFFF...F which can only happen if
1866-
// a holds 0.
1867-
r.addScalar(r.toConst(), 1);
1868-
1869-
r.positive = false;
1870-
}
1796+
r.positive = a.positive or r.eqlZero();
1797+
} else {
1798+
// One of the following is true:
1799+
// * The result is the minimum signed integer.
1800+
// * The result is unsigned zero.
1801+
// * The result is non-zero and has the opposite sign as `a`.
1802+
r.addScalar(abs_trunc_a, -1);
1803+
llnot(r.limbs[0..r.len]);
1804+
@memset(r.limbs[r.len..max_limbs], maxInt(Limb));
1805+
r.limbs[max_limbs - 1] &= mask;
1806+
r.normalize(max_limbs);
1807+
r.positive = switch (signedness) {
1808+
// The only value with the sign bit still set is the minimum signed integer.
1809+
.signed => !a.positive and r.limbs[max_limbs - 1] & sign_bit == 0,
1810+
.unsigned => !a.positive or r.eqlZero(),
1811+
};
18711812
}
18721813
}
18731814

‎lib/std/math/big/int_test.zig

Lines changed: 66 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1020,7 +1020,7 @@ test "mul large" {
10201020
// Generate a number that's large enough to cross the thresholds for the use
10211021
// of subquadratic algorithms
10221022
for (a.limbs) |*p| {
1023-
p.* = std.math.maxInt(Limb);
1023+
p.* = maxInt(Limb);
10241024
}
10251025
a.setMetadata(true, 50);
10261026

@@ -1104,7 +1104,7 @@ test "mulWrap large" {
11041104
// Generate a number that's large enough to cross the thresholds for the use
11051105
// of subquadratic algorithms
11061106
for (a.limbs) |*p| {
1107-
p.* = std.math.maxInt(Limb);
1107+
p.* = maxInt(Limb);
11081108
}
11091109
a.setMetadata(true, 50);
11101110

@@ -1961,23 +1961,78 @@ test "truncate to mutable with fewer limbs" {
19611961
.positive = undefined,
19621962
};
19631963
res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));
1964-
try testing.expect(res.eqlZero());
1964+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
19651965
res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));
1966-
try testing.expect(res.eqlZero());
1966+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
19671967
res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));
1968-
try testing.expect(res.eqlZero());
1968+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
19691969
res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));
1970-
try testing.expect(res.eqlZero());
1971-
res.truncate(.{ .positive = true, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));
1972-
try testing.expect(res.toConst().orderAgainstScalar(std.math.maxInt(Limb)).compare(.eq));
1973-
res.truncate(.{ .positive = true, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));
1970+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
1971+
res.truncate(.{ .positive = true, .limbs = &.{ maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));
1972+
try testing.expect(res.toConst().orderAgainstScalar(maxInt(Limb)).compare(.eq));
1973+
res.truncate(.{ .positive = true, .limbs = &.{ maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));
19741974
try testing.expect(res.toConst().orderAgainstScalar(-1).compare(.eq));
1975-
res.truncate(.{ .positive = false, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));
1975+
res.truncate(.{ .positive = false, .limbs = &.{ maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));
19761976
try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));
1977-
res.truncate(.{ .positive = false, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));
1977+
res.truncate(.{ .positive = false, .limbs = &.{ maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));
19781978
try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));
19791979
}
19801980

1981+
test "truncate value that normalizes after being masked" {
1982+
var res_limbs: [2]Limb = undefined;
1983+
var res: Mutable = .{
1984+
.limbs = &res_limbs,
1985+
.len = undefined,
1986+
.positive = undefined,
1987+
};
1988+
res.truncate(.{ .positive = true, .limbs = &.{ 0, 2 } }, .signed, 1 + @bitSizeOf(Limb));
1989+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
1990+
res.truncate(.{ .positive = true, .limbs = &.{ 1, 2 } }, .signed, 1 + @bitSizeOf(Limb));
1991+
try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));
1992+
}
1993+
1994+
test "truncate to zero" {
1995+
var res_limbs: [1]Limb = undefined;
1996+
var res: Mutable = .{
1997+
.limbs = &res_limbs,
1998+
.len = undefined,
1999+
.positive = undefined,
2000+
};
2001+
res.truncate(.{ .positive = true, .limbs = &.{0} }, .signed, @bitSizeOf(Limb));
2002+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2003+
res.truncate(.{ .positive = false, .limbs = &.{0} }, .signed, @bitSizeOf(Limb));
2004+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2005+
res.truncate(.{ .positive = true, .limbs = &.{0} }, .unsigned, @bitSizeOf(Limb));
2006+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2007+
res.truncate(.{ .positive = false, .limbs = &.{0} }, .unsigned, @bitSizeOf(Limb));
2008+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2009+
res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));
2010+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2011+
res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));
2012+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2013+
res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));
2014+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2015+
res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));
2016+
try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
2017+
}
2018+
2019+
test "truncate to minimum signed integer" {
2020+
var res_limbs: [1]Limb = undefined;
2021+
var res: Mutable = .{
2022+
.limbs = &res_limbs,
2023+
.len = undefined,
2024+
.positive = undefined,
2025+
};
2026+
res.truncate(.{ .positive = true, .limbs = &.{1 << @bitSizeOf(Limb) - 1} }, .signed, @bitSizeOf(Limb));
2027+
try testing.expect(res.toConst().orderAgainstScalar(-1 << @bitSizeOf(Limb) - 1).compare(.eq));
2028+
res.truncate(.{ .positive = false, .limbs = &.{1 << @bitSizeOf(Limb) - 1} }, .signed, @bitSizeOf(Limb));
2029+
try testing.expect(res.toConst().orderAgainstScalar(-1 << @bitSizeOf(Limb) - 1).compare(.eq));
2030+
res.truncate(.{ .positive = true, .limbs = &.{1 << @bitSizeOf(Limb) - 1} }, .unsigned, @bitSizeOf(Limb));
2031+
try testing.expect(res.toConst().orderAgainstScalar(1 << @bitSizeOf(Limb) - 1).compare(.eq));
2032+
res.truncate(.{ .positive = false, .limbs = &.{1 << @bitSizeOf(Limb) - 1} }, .unsigned, @bitSizeOf(Limb));
2033+
try testing.expect(res.toConst().orderAgainstScalar(1 << @bitSizeOf(Limb) - 1).compare(.eq));
2034+
}
2035+
19812036
test "saturate single signed positive" {
19822037
var a = try Managed.initSet(testing.allocator, 0xBBBB_BBBB);
19832038
defer a.deinit();

‎src/codegen/c.zig

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8175,7 +8175,7 @@ fn formatIntLiteral(
81758175
try writer.writeAll(string);
81768176
} else {
81778177
try data.ctype.renderLiteralPrefix(writer, data.kind, ctype_pool);
8178-
wrap.convertToTwosComplement(int, data.int_info.signedness, c_bits);
8178+
wrap.truncate(int, .unsigned, c_bits);
81798179
@memset(wrap.limbs[wrap.len..], 0);
81808180
wrap.len = wrap.limbs.len;
81818181
const limbs_per_c_limb = @divExact(wrap.len, c_limb_info.count);
@@ -8207,7 +8207,6 @@ fn formatIntLiteral(
82078207
c_limb_int_info.signedness = .signed;
82088208
c_limb_ctype = c_limb_info.ctype.toSigned();
82098209

8210-
c_limb_mut.positive = wrap.positive;
82118210
c_limb_mut.truncate(
82128211
c_limb_mut.toConst(),
82138212
.signed,

0 commit comments

Comments
 (0)