authorgravatar for jacobly@ziglang.orgJacob Young <jacobly@ziglang.org> 2025-03-11 08:56:45-04:00
committergravatar for jacobly@ziglang.orgJacob Young <jacobly@ziglang.org> 2025-03-23 21:59:07-04:00
log4f47be5c6bbf67ed6812978759307d5d6de041ba
tree9a92f24e9df8fbb913f26cce4a593712f040964c
parent7199a86b9776a0d44e9fceb98ac6567eb6db3d6e

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.

3 files changed, 109 insertions(+), 114 deletions(-)

lib/std/math/big/int.zig+42-101
...@@ -1755,119 +1755,60 @@ pub const Mutable = struct {...@@ -1755,119 +1755,60 @@ pub const Mutable = struct {
1755 y.shiftRight(y.toConst(), norm_shift);1755 y.shiftRight(y.toConst(), norm_shift);
1756 }1756 }
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
1793 /// Truncate an integer to a number of bits, following 2s-complement semantics.1758 /// Truncate an integer to a number of bits, following 2s-complement semantics.
1794 /// r may alias a.1759 /// `r` may alias `a`.
1795 ///1760 ///
1796 /// Asserts `r` has enough storage to store the result.1761 /// Asserts `r` has enough storage to compute the result.
1797 /// The upper bound is `calcTwosCompLimbCount(a.len)`.1762 /// The upper bound is `calcTwosCompLimbCount(a.len)`.
1798 pub fn truncate(r: *Mutable, a: Const, signedness: Signedness, bit_count: usize) void {1763 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
1805 // Handle 0-bit integers.1764 // Handle 0-bit integers.
1806 if (req_limbs == 0 or abs_trunc_a.eqlZero()) {1765 if (bit_count == 0) {
1766 @branchHint(.unlikely);
1807 r.set(0);1767 r.set(0);
1808 return;1768 return;
1809 }1769 }
18101770
1811 const bit = @as(Log2Limb, @truncate(bit_count - 1));1771 const max_limbs = calcTwosCompLimbCount(bit_count);
1812 const signmask = @as(Limb, 1) << bit; // 0b0..010...0 where 1 is the sign bit.1772 const sign_bit = @as(Limb, 1) << @truncate(bit_count - 1);
1813 const mask = (signmask << 1) -% 1; // 0b0..01..1 where the leftmost 1 is the sign bit.1773 const mask = @as(Limb, maxInt(Limb)) >> @truncate(-%bit_count);
18141774
1815 if (!a.positive) {1775 // Guess whether the result will have the same sign as `a`.
1816 // Convert the integer from sign-magnitude into twos-complement.1776 // * If the result will be signed zero, the guess is `true`.
1817 // -x = ~(x - 1)1777 // * If the result will be the minimum signed integer, the guess is `false`.
1818 // Note, we simply take req_limbs * @bitSizeOf(Limb) as the1778 // * If the result will be unsigned zero, the guess is `a.positive`.
1819 // target bit count.1779 // * Otherwise the guess is correct.
18201780 const same_sign_guess = switch (signedness) {
1821 r.addScalar(abs_trunc_a, -1);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 result1785 const abs_trunc_a: Const = .{
1824 @memset(r.limbs[r.len..req_limbs], 0);1786 .positive = true,
1825 r.len = req_limbs;1787 .limbs = a.limbs[0..llnormalize(a.limbs[0..@min(a.limbs.len, max_limbs)])],
18261788 };
1827 // Without truncating, we can already peek at the sign bit of the result here.1789 if (same_sign_guess or abs_trunc_a.eqlZero()) {
1828 // Note that it will be 0 if the result is negative, as we did not apply the flip here.1790 // One of the following is true:
1829 // If the result is negative, we have1791 // * The result is zero.
1830 // -(-x & mask)1792 // * The result is non-zero and has the same sign as `a`.
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 {
1852 r.copy(abs_trunc_a);1793 r.copy(abs_trunc_a);
1853 // If the integer fits within target bits, no wrapping is required.1794 if (max_limbs <= r.len) r.limbs[max_limbs - 1] &= mask;
1854 if (r.len < req_limbs) return;
1855
1856 r.limbs[r.len - 1] &= mask;
1857 r.normalize(r.len);1795 r.normalize(r.len);
18581796 r.positive = a.positive or r.eqlZero();
1859 if (signedness == .signed and r.limbs[r.len - 1] & signmask != 0) {1797 } else {
1860 // Convert 2s-complement back to sign-magnitude.1798 // One of the following is true:
1861 // Sign-extend the upper bits so that they are inverted correctly.1799 // * The result is the minimum signed integer.
1862 r.limbs[r.len - 1] |= ~mask;1800 // * The result is unsigned zero.
1863 llnot(r.limbs[0..r.len]);1801 // * The result is non-zero and has the opposite sign as `a`.
18641802 r.addScalar(abs_trunc_a, -1);
1865 // Note, can only overflow if r holds 0xFFF...F which can only happen if1803 llnot(r.limbs[0..r.len]);
1866 // a holds 0.1804 @memset(r.limbs[r.len..max_limbs], maxInt(Limb));
1867 r.addScalar(r.toConst(), 1);1805 r.limbs[max_limbs - 1] &= mask;
18681806 r.normalize(max_limbs);
1869 r.positive = false;1807 r.positive = switch (signedness) {
1870 }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 };
1871 }1812 }
1872 }1813 }
18731814
lib/std/math/big/int_test.zig+66-11
...@@ -1020,7 +1020,7 @@ test "mul large" {...@@ -1020,7 +1020,7 @@ test "mul large" {
1020 // Generate a number that's large enough to cross the thresholds for the use1020 // Generate a number that's large enough to cross the thresholds for the use
1021 // of subquadratic algorithms1021 // of subquadratic algorithms
1022 for (a.limbs) |*p| {1022 for (a.limbs) |*p| {
1023 p.* = std.math.maxInt(Limb);1023 p.* = maxInt(Limb);
1024 }1024 }
1025 a.setMetadata(true, 50);1025 a.setMetadata(true, 50);
10261026
...@@ -1104,7 +1104,7 @@ test "mulWrap large" {...@@ -1104,7 +1104,7 @@ test "mulWrap large" {
1104 // Generate a number that's large enough to cross the thresholds for the use1104 // Generate a number that's large enough to cross the thresholds for the use
1105 // of subquadratic algorithms1105 // of subquadratic algorithms
1106 for (a.limbs) |*p| {1106 for (a.limbs) |*p| {
1107 p.* = std.math.maxInt(Limb);1107 p.* = maxInt(Limb);
1108 }1108 }
1109 a.setMetadata(true, 50);1109 a.setMetadata(true, 50);
11101110
...@@ -1961,23 +1961,78 @@ test "truncate to mutable with fewer limbs" {...@@ -1961,23 +1961,78 @@ test "truncate to mutable with fewer limbs" {
1961 .positive = undefined,1961 .positive = undefined,
1962 };1962 };
1963 res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));1963 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);
1965 res.truncate(.{ .positive = true, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));1965 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);
1967 res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .unsigned, @bitSizeOf(Limb));1967 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);
1969 res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));1969 res.truncate(.{ .positive = false, .limbs = &.{ 0, 1 } }, .signed, @bitSizeOf(Limb));
1970 try testing.expect(res.eqlZero());1970 try testing.expect(res.positive and res.len == 1 and res.limbs[0] == 0);
1971 res.truncate(.{ .positive = true, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));1971 res.truncate(.{ .positive = true, .limbs = &.{ maxInt(Limb), 1 } }, .unsigned, @bitSizeOf(Limb));
1972 try testing.expect(res.toConst().orderAgainstScalar(std.math.maxInt(Limb)).compare(.eq));1972 try testing.expect(res.toConst().orderAgainstScalar(maxInt(Limb)).compare(.eq));
1973 res.truncate(.{ .positive = true, .limbs = &.{ std.math.maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));1973 res.truncate(.{ .positive = true, .limbs = &.{ maxInt(Limb), 1 } }, .signed, @bitSizeOf(Limb));
1974 try testing.expect(res.toConst().orderAgainstScalar(-1).compare(.eq));1974 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));
1976 try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));1976 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));
1978 try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));1978 try testing.expect(res.toConst().orderAgainstScalar(1).compare(.eq));
1979}1979}
19801980
1981test "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
1994test "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
2019test "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
1981test "saturate single signed positive" {2036test "saturate single signed positive" {
1982 var a = try Managed.initSet(testing.allocator, 0xBBBB_BBBB);2037 var a = try Managed.initSet(testing.allocator, 0xBBBB_BBBB);
1983 defer a.deinit();2038 defer a.deinit();
src/codegen/c.zig+1-2
...@@ -8175,7 +8175,7 @@ fn formatIntLiteral(...@@ -8175,7 +8175,7 @@ fn formatIntLiteral(
8175 try writer.writeAll(string);8175 try writer.writeAll(string);
8176 } else {8176 } else {
8177 try data.ctype.renderLiteralPrefix(writer, data.kind, ctype_pool);8177 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);
8179 @memset(wrap.limbs[wrap.len..], 0);8179 @memset(wrap.limbs[wrap.len..], 0);
8180 wrap.len = wrap.limbs.len;8180 wrap.len = wrap.limbs.len;
8181 const limbs_per_c_limb = @divExact(wrap.len, c_limb_info.count);8181 const limbs_per_c_limb = @divExact(wrap.len, c_limb_info.count);
...@@ -8207,7 +8207,6 @@ fn formatIntLiteral(...@@ -8207,7 +8207,6 @@ fn formatIntLiteral(
8207 c_limb_int_info.signedness = .signed;8207 c_limb_int_info.signedness = .signed;
8208 c_limb_ctype = c_limb_info.ctype.toSigned();8208 c_limb_ctype = c_limb_info.ctype.toSigned();
82098209
8210 c_limb_mut.positive = wrap.positive;
8211 c_limb_mut.truncate(8210 c_limb_mut.truncate(
8212 c_limb_mut.toConst(),8211 c_limb_mut.toConst(),
8213 .signed,8212 .signed,