D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 16264 - BigInt multiplication crashes on 64-bit (biguintnoasm.d(276): Range violation)
Summary: BigInt multiplication crashes on 64-bit (biguintnoasm.d(276): Range violation)
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: x86_64 Windows
: P1 major
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-11 04:26 UTC by Kirill Kryukov
Modified: 2017-11-15 15:11 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Kirill Kryukov 2016-07-11 04:26:37 UTC
This code crashes on multiplication (does not even reach the assert):

======= a.d start =======
import std.bigint;

void main()
{
    auto a = BigInt(
`335690982744637013564796917901053301979460129353374296317539383938630086938` ~
`465898213033510992292836631752875403891802201862860531801760096359705447768` ~
`957432600293361240407059207520920532482429912948952142341440301429494694368` ~
`264560802292927144211230021750155988283029753927847924288850436812178022006` ~
`408597793414273953252832688620479083497367463977081627995406363446761896298` ~
`967177607401918269561385622811274398143647535024987050366350585544531063531` ~
`7118554808325723941557169427279911052268935775`);

    auto b = BigInt(
`207672245542926038535480439528441949928508406405023044025560363701392340829` ~
`852529131306106648201340460604257466180580583656068555417076345439694125326` ~
`843947164365500055567495554645796102453565953360564114634705366335703491527` ~
`429426780005741168078089657359833601261803592920462081364401456331489106355` ~
`199133982282631108670436696758342051198891939367812305559960349479160308314` ~
`068518200681530999860641597181672463704794566473241690395901768680673716414` ~
`243691584391572899147223065906633310537507956952626106509069491302359792769` ~
`378934570685117202046921464019396759638376362935855896435623442486036961070` ~
`534574698959398017332214518246531363445309522357827985468581166065335726996` ~
`711467464306784543112544076165391268106101754253962102479935962248302404638` ~
`21737237102628470475027851189594709504`);

    BigInt c = a * b;  // Crashes

    assert(c == BigInt(
`697137001950904057507249234183127244116872349433141878383548259425589716813` ~
`135440660252012378417669596912108637127036044977634382385990472429604619344` ~
`738746224291111527200379708978133071390303850450970292020176369525401803474` ~
`998613408923490273129022167907826017408385746675184651576154302536663744109` ~
`111018961065316024005076097634601030334948684412785487182572502394847587887` ~
`507385831062796361152176364659197432600147716058873232435238712648552844428` ~
`058885217631715287816333209463171932255049134340904981280717725999710525214` ~
`161541960645335744430049558161514565159449390036287489478108344584188898872` ~
`434914159748515512161981956372737022393466624249130107254611846175580584736` ~
`276213025837422102290580044755202968610542057651282410252208599309841499843` ~
`672251048622223867183370008181364966502137725166782667358559333222947265344` ~
`524195551978394625568228658697170315141077913403482061673401937141405425042` ~
`283546509102861986303306729882186190883772633960389974665467972016939172303` ~
`653623175801495207204880400522581834672918935651426160175413277309985678579` ~
`830872397214091472424064274864210953551447463312267310436493480881235642109` ~
`668498742629676513172286703948381906930297135997498416573231570483993847269` ~
`479552708416124555462530834668011570929850407031109157206202741051573633443` ~
`58105600`
    ));
}
======= a.d end =======

Error message: 
core.exception.RangeError@std\internal\math\biguintnoasm.d(276): Range violation

Tested with DMD 2.071.0 and DMD 2.071.1 on Windows 7 64-bit. 32-bit build works fine, only 64-bit build crashes.

Tested both release and debug mode, no change in crash or error message. Command lines:

dmd a.d -ofar.exe -w -release -O -inline -boundscheck=off -m64

dmd a.d -ofad.exe -w -inline -boundscheck=on -unittest -m64

Isolated from an actual project. I'll appreciate any fix or workaround.
Comment 1 Kirill Kryukov 2016-07-11 07:59:40 UTC
Slightly reduced (same behavior: 32-bit compile works, 64-bit one - crashes).

===== a.d begin =====
import std.internal.math.biguintcore;

void main()
{
    BigUint a, b;

    a.fromDecimalString(
`335690982744637013564796917901053301979460129353374296317539383938630086938` ~
`465898213033510992292836631752875403891802201862860531801760096359705447768` ~
`957432600293361240407059207520920532482429912948952142341440301429494694368` ~
`264560802292927144211230021750155988283029753927847924288850436812178022006` ~
`408597793414273953252832688620479083497367463977081627995406363446761896298` ~
`967177607401918269561385622811274398143647535024987050366350585544531063531` ~
`7118554808325723941557169427279911052268935775`);

    b.fromDecimalString(
`207672245542926038535480439528441949928508406405023044025560363701392340829` ~
`852529131306106648201340460604257466180580583656068555417076345439694125326` ~
`843947164365500055567495554645796102453565953360564114634705366335703491527` ~
`429426780005741168078089657359833601261803592920462081364401456331489106355` ~
`199133982282631108670436696758342051198891939367812305559960349479160308314` ~
`068518200681530999860641597181672463704794566473241690395901768680673716414` ~
`243691584391572899147223065906633310537507956952626106509069491302359792769` ~
`378934570685117202046921464019396759638376362935855896435623442486036961070` ~
`534574698959398017332214518246531363445309522357827985468581166065335726996` ~
`711467464306784543112544076165391268106101754253962102479935962248302404638` ~
`21737237102628470475027851189594709504`);

    BigUint c = BigUint.mul(a,b);  // Crashes

    BigUint ce;
    ce.fromDecimalString(
`697137001950904057507249234183127244116872349433141878383548259425589716813` ~
`135440660252012378417669596912108637127036044977634382385990472429604619344` ~
`738746224291111527200379708978133071390303850450970292020176369525401803474` ~
`998613408923490273129022167907826017408385746675184651576154302536663744109` ~
`111018961065316024005076097634601030334948684412785487182572502394847587887` ~
`507385831062796361152176364659197432600147716058873232435238712648552844428` ~
`058885217631715287816333209463171932255049134340904981280717725999710525214` ~
`161541960645335744430049558161514565159449390036287489478108344584188898872` ~
`434914159748515512161981956372737022393466624249130107254611846175580584736` ~
`276213025837422102290580044755202968610542057651282410252208599309841499843` ~
`672251048622223867183370008181364966502137725166782667358559333222947265344` ~
`524195551978394625568228658697170315141077913403482061673401937141405425042` ~
`283546509102861986303306729882186190883772633960389974665467972016939172303` ~
`653623175801495207204880400522581834672918935651426160175413277309985678579` ~
`830872397214091472424064274864210953551447463312267310436493480881235642109` ~
`668498742629676513172286703948381906930297135997498416573231570483993847269` ~
`479552708416124555462530834668011570929850407031109157206202741051573633443` ~
`58105600`);

    assert(c == ce);
}
===== a.d end =====
Comment 2 Kirill Kryukov 2016-07-11 14:33:44 UTC
Since this crash occurs with particularly large operands, I took a look at the dependence of this bug upon operand sizes. D's BigInt (and BigUint) is represented internally as an array of uint (regardless of whether it's a 32-bit and 64-bit compile).

The initial bug-report is with operand of 52 and 82 uints. I took a look at other nearby sizes, and noticed that this bug lives in a narrow sector starting around operand sizes 48 and 74 (in uints). The sector starts there and grows wider as it goes:

http://kirr.homeunix.org/temp/d-bug-16264-operand-size-table.png

Code used to fill this table (compile a.d into a.exe, then run test.pl):

===== a.d start =====
import std.conv: to;
import std.internal.math.biguintcore;

void main(string[] args)
{
    uint alen = to!uint(args[1]);
    uint blen = to!uint(args[2]);
    string unitstr = args[3];

    string astr = ``, bstr = ``;
    for (int i = 0; i < alen; i++) astr ~= unitstr;
    for (int i = 0; i < blen; i++) bstr ~= unitstr;

    BigUint a, b;
    a.fromHexString(astr);
    b.fromHexString(bstr);

    assert(a.uintLength() == alen);
    assert(b.uintLength() == blen);

    BigUint c = BigUint.mul(a,b);  // Crashes
}
===== a.d end =====

===== test.pl start =====
my ($mina,$maxa,$minb,$maxb) = (40,80,70,120);
print '    ';
for (my $b = $minb; $b <= $maxb; $b++) { printf " %3d", $b; }
print "\n";
for (my $a = $mina; $a <= $maxa; $a++)
{
    printf " %3d", $a;
    for (my $b = $minb; $b <= $maxb; $b++)
    {
        my $e = system("a.exe $a $b FFFFFFFF 2>nul");
        print '   ', ($e ? 1 : 0);
    }
    print "\n";
}
===== test.pl end =====

I also checked 32-bit compiles - no crashes with any of these operand sizes.
Comment 3 Kirill Kryukov 2016-07-12 02:15:54 UTC
Here is the reason 32-bit BigUint multiplication works: std/internal/math/biguintcore.d has the following:

version(D_InlineAsm_X86)
{
    import std.internal.math.biguintx86;
}
else
{
    import std.internal.math.biguintnoasm;
}

So, 32-bit compile uses the seemingly correct std.internal.math.biguintx86. On the other hand, 64-bit compile uses std.internal.math.biguintnoasm - buggy, untested and apparently never used in the real world.
Comment 4 Kirill Kryukov 2016-07-12 03:36:18 UTC
Actually I've no idea why the 32-bit compile works with my initial test. If I add the following unittest into biguintcore.d, and compile in 32-bit (dmd.exe -main -unittest biguintcore.d), it crashes too.

unittest
{
    BigUint a, b;

    a.fromDecimalString(
`207672245542926038535480439528441949928508406405023044025560363701392340829` ~
`852529131306106648201340460604257466180580583656068555417076345439694125326` ~
`843947164365500055567495554645796102453565953360564114634705366335703491527` ~
`429426780005741168078089657359833601261803592920462081364401456331489106355` ~
`199133982282631108670436696758342051198891939367812305559960349479160308314` ~
`068518200681530999860641597181672463704794566473241690395901768680673716414` ~
`243691584391572899147223065906633310537507956952626106509069491302359792769` ~
`378934570685117202046921464019396759638376362935855896435623442486036961070` ~
`534574698959398017332214518246531363445309522357827985468581166065335726996` ~
`711467464306784543112544076165391268106101754253962102479935962248302404638` ~
`21737237102628470475027851189594709504`);

    b.fromDecimalString(
`335690982744637013564796917901053301979460129353374296317539383938630086938` ~
`465898213033510992292836631752875403891802201862860531801760096359705447768` ~
`957432600293361240407059207520920532482429912948952142341440301429494694368` ~
`264560802292927144211230021750155988283029753927847924288850436812178022006` ~
`408597793414273953252832688620479083497367463977081627995406363446761896298` ~
`967177607401918269561385622811274398143647535024987050366350585544531063531` ~
`7118554808325723941557169427279911052268935775`);

    BigDigit[] cdata = new BigDigit[a.uintLength() + b.uintLength()];

    mulInternal(cdata, a.data, b.data);  // Crashes
}

Also, this test is a bit further reduction.

Note that the crash disappears if you change the operand sizes (by adding or removing a few lines in either a or b), so it's specific to particular length of the operands.

The crash message is now more specific: 

"core.exception.AssertError@m\biguintcore.d(1190): Bigint Internal Error: Asymmetric Karatsuba"

So I think it's related to how mulInternal divides the operands before calling mulKaratsuba.

However, this bug is now probably applies to both 32-bit and 64-bit!

This blocks D's BigUint (and be extension BigInt) use in any non-toy calculation.
Comment 5 Kirill Kryukov 2016-07-12 04:48:44 UTC
Looks like it might be the same issue with #11599 ( https://issues.dlang.org/show_bug.cgi?id=11599 )

If so, "Asymmetric Karatsuba" bug is known since 2013, and stays unfixed. Which practically means that no one uses D's BigInt in large calculations.
Comment 6 github-bugzilla 2017-10-28 13:22:14 UTC
Commits pushed to stable at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/93ee606fba69390b2f9ef7d77e43abc02b82cd19
Fix Issue 16264(+11599) - BigInt multiplication crashes on 64-bit

https://github.com/dlang/phobos/commit/18c19d8527ac8ff2bf688df2ebdae304f1b5d7b6
Merge pull request #5715 from yosupo06/Issue_16264

Fix Issue 16264(+11599) - BigInt multiplication crashes on 64-bit
Comment 7 github-bugzilla 2017-11-15 15:11:21 UTC
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/93ee606fba69390b2f9ef7d77e43abc02b82cd19
Fix Issue 16264(+11599) - BigInt multiplication crashes on 64-bit

https://github.com/dlang/phobos/commit/18c19d8527ac8ff2bf688df2ebdae304f1b5d7b6
Merge pull request #5715 from yosupo06/Issue_16264