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

Bug: Incorrect result for 32-bit right shift of specific negative values in BigInteger #27358

Closed
aklette opened this issue Sep 11, 2018 · 1 comment · Fixed by #54115
Closed
Labels
area-System.Numerics bug Cost:S Work that requires one engineer up to 1 week help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@aklette
Copy link

aklette commented Sep 11, 2018

Versions

System.Numerics Runtime Version v4.0.30319
VS 2017, .NetFx 4.7.2, C# 7.3, x86, x64, Debug & Release

Demonstration

The following Console App demonstrates the bug
The pattern described in the comment repeats
This is not a display issue: on iteration j == 32

  • bi2's debugger-displayed value is {0} zero
  • In the Watch window, ToString() returns "0"
  • In the Watch window, bi2 + 1 returns a BigInteger value {1}
  • The same is true at iteration j == 64
        using System;
        using System.Numerics;

        namespace ConsoleApp1
        {
          class Program
          {
            static unsafe void Main(string[] args)
            {
              BigInteger bi = new BigInteger(new byte[]
        // Pattern: <--  LoWrd non-0   -->   
                  { 0x01, 0x00, 0x00, 0x00, 
        //          <-- N x 4-byte words all 0                 --> 
                    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
        //          < HiWrd all bits set >
                    0xff, 0xff, 0xff, 0xff });

              var bytes = bi.ToByteArray();
              Console.WriteLine(bi);
              Console.Write("Before:             ");
              for (int i = 0; i < bytes.Length; i++)
                Console.Write($"{bytes[i]:X2}{(i + 1 == bytes.Length ? "" : ", ")}");
              Console.WriteLine($" (sign {bi.Sign})");
              for (int j = 0; j < 66; j++)
              {
                BigInteger bi2 = bi >> j;
                bytes = bi2.ToByteArray();
                Console.Write($"After {j:D3}-bit ASHR: ");
                for (int i = 0; i < bytes.Length; i++)
                  Console.Write($"{bytes[i]:X2}{(i + 1 == bytes.Length ? "" : ", ")}");
                Console.WriteLine($" (sign {bi2.Sign})");
              }
              Console.ReadKey();
        } } }

Output:

-79228162514264337593543950335
Before:             01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, FF (sign -1)
After 000-bit ASHR: 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, FF (sign -1)
After 001-bit ASHR: 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 80 (sign -1)
After 002-bit ASHR: 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, C0 (sign -1)
...
After 031-bit ASHR: 00, 00, 00, 00, 00, 00, 00, 00, FE (sign -1)
After 032-bit ASHR: 00 (sign 0) <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
After 033-bit ASHR: 00, 00, 00, 00, 00, 00, 00, 80 (sign -1)
After 063-bit ASHR: 00, 00, 00, 00, FE (sign -1)
After 064-bit ASHR: 00 (sign 0) <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
After 065-bit ASHR: 00, 00, 00, 80 (sign -1)
@aklette
Copy link
Author

aklette commented Sep 11, 2018

Workaround:

// instead of...
var newVar = bigInt >> n;

// this seems to work correctly (for positives, negatives and zero)...
var newVar = bigInt.Sign == -1 ? ~(~bigInt >> n) : bigInt >> n;

E&OE: Based on limited understanding of theory plus assumptions about BigInteger's internal mechanics. I don't have the bandwidth to get into it rigorously but this has worked for me so far.

@danmoseley danmoseley changed the title Bug: Incorrect result for 32-bit right shift of specific negative values Bug: Incorrect result for 32-bit right shift of specific negative values in BigInteger Sep 12, 2018
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020
@tannergooding tannergooding added Cost:S Work that requires one engineer up to 1 week help wanted [up-for-grabs] Good issue for external contributors labels Jan 15, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 13, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 24, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Numerics bug Cost:S Work that requires one engineer up to 1 week help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants