Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigInteger correctness regression from .NET 8 #109669

Closed
hez2010 opened this issue Nov 9, 2024 · 5 comments · Fixed by #109684
Closed

BigInteger correctness regression from .NET 8 #109669

hez2010 opened this issue Nov 9, 2024 · 5 comments · Fixed by #109684
Labels
area-System.Numerics in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Milestone

Comments

@hez2010
Copy link
Contributor

hez2010 commented Nov 9, 2024

Description

BigInteger produces incorrect results in .NET 9, while this doesn't repro with .NET 8 or earlier.

Reproduction Steps

Console.WriteLine(new BigInteger(-4294967296) & new BigInteger(-1919810));
Console.WriteLine(new BigInteger(-4042322161) & new BigInteger(-252645136));
Console.WriteLine(new BigInteger(-8589934592) | new BigInteger(-21474836480));

Console.WriteLine(BigInteger.Parse("-4294967296") & BigInteger.Parse("-1919810"));
Console.WriteLine(BigInteger.Parse("-4042322161") & BigInteger.Parse("-252645136"));
Console.WriteLine(BigInteger.Parse("-8589934592") | BigInteger.Parse("-21474836480"));

Expected behavior

-4294967296
-4294967296
-4294967296
-4294967296
-4294967296
-4294967296

Actual behavior

0
0
0
0
0
0

Regression?

Yes

Known Workarounds

No

Configuration

.NET 9 rc2

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Nov 9, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-numerics
See info in area-owners.md if you want to be subscribed.

@hez2010 hez2010 changed the title BigInteger regression from .NET 8 BigInteger correctness regression from .NET 8 Nov 9, 2024
@tannergooding
Copy link
Member

Bug looks to be in the internal span constructor and sign preservation being incorrectly tracked as false.

@jeffhandley
Copy link
Member

Thanks for reporting this, @hez2010. We will fix this and backport into 9.0 servicing.

@tannergooding Do you think this is limited to a small domain of values? We will need to prepare a Known Issues entry on https://github.com/dotnet/core/blob/main/release-notes/9.0/known-issues.md for this.

@jeffhandley jeffhandley added regression-from-last-release and removed untriaged New issue has not been triaged by the area owner labels Nov 9, 2024
@jeffhandley jeffhandley added this to the 9.0.x milestone Nov 9, 2024
@Rob-Hague
Copy link
Contributor

In my understanding, it affects the operators &, |, ^ whose result is a negative number of the form -2^(32n), n>1. In other words, whose result is 0xFFFFFFFF 00000000 ... 00000000 for any positive number of trailing zero blocks, such that the length of the two's complement of this value is greater than the number of trailing zero blocks.

An assert can fire for values with nonzero trailing blocks, but it does not result in correctness issues. But for the affected values, it can also give rise to an exception when formatting, because of invalid internal state e.g.

BigInteger a = new BigInteger(MemoryMarshal.AsBytes([uint.MaxValue, 0u, 0u]), isBigEndian: true);

BigInteger or = a | a;

Console.WriteLine(or == a); // False (should be True)

Console.WriteLine(or); // System.IndexOutOfRangeException: Index was outside the bounds of the array.
//    at System.Number.FormatBigInteger(Boolean targetSpan, BigInteger value, String formatString, ReadOnlySpan`1 formatSpan, NumberFormatInfo info, Span`1 destination, Int32& charsWritten, Boolean& spanSuccess)
//    at System.Number.FormatBigInteger(BigInteger value, String format, NumberFormatInfo info)
//    at System.Numerics.BigInteger.ToString()

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Nov 10, 2024
@huoyaoyuan
Copy link
Member

The fix was already submitted at #105456

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Numerics in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants