grondilu / libdigest-raku

Raku implementation of various digests
Artistic License 2.0
27 stars 42 forks source link

Digest test failing on Rakudo 2017.04.3-235-gdc5eece built on MoarVM version 2017.04-64-g6d5ea04 #18

Closed CurtTilmes closed 7 years ago

CurtTilmes commented 7 years ago
$ perl6 --version
This is Rakudo version 2017.04.3-235-gdc5eece built on MoarVM version 2017.04-64-g6d5ea04

===> Testing: Digest:ver('0.3.4'):auth('Lucien Grondin')
t/digest.t .. ok
t/ripemd.t .. ok
# Failed test 'sha256'
# at t/sha.t line 12
t/sha.t .....1/2# expected: 'fbc1a9f858ea9e177916964bd88c3d37b91a1e84412765e29950777f265c4b75'
#      got: 'd7d04233b3a8427b7798817eae45cb70c7c24cfcc6740c2b44fb7aa823662036'
# Looks like you failed 1 test of 2
t/sha.t ..... Dubious, test returned 1
Failed 1/2 subtests 

It failed in my travis build here

CurtTilmes commented 7 years ago

Works with 2017.04.3-47-gf0414c4, fails with 2017.04.3-235-gdc5eece

{master} ~/git/libdigest-perl6$ perl6 -v
This is Rakudo version 2017.04.3-47-gf0414c4 built on MoarVM version 2017.04-44-gf0db882
implementing Perl 6.c.
{master} ~/git/libdigest-perl6$ perl6 t/sha.t 
1..2
ok 1 - sha1
ok 2 - sha256
{master} ~/git/libdigest-perl6$ ~/git/rakudo/install/bin/perl6 -v
This is Rakudo version 2017.04.3-235-gdc5eece built on MoarVM version 2017.04-64-g6d5ea04
implementing Perl 6.c.
{master} ~/git/libdigest-perl6$ ~/git/rakudo/install/bin/perl6 t/sha.t 
1..2
ok 1 - sha1
not ok 2 - sha256

# Failed test 'sha256'
# at t/sha.t line 12
# expected: 'fbc1a9f858ea9e177916964bd88c3d37b91a1e84412765e29950777f265c4b75'
#      got: 'd7d04233b3a8427b7798817eae45cb70c7c24cfcc6740c2b44fb7aa823662036'
# Looks like you failed 1 test of 2
CurtTilmes commented 7 years ago

Narrowed down some more:

Works with 2017.04.3-210-g6435933, fails with 2017.04.3-211-gef29bb9

This is the commit that broke it:

https://github.com/rakudo/rakudo/commit/ef29bb9f41aa5d50638544db1ff966459c888f68

It affects the +> bit shift operator.

CurtTilmes commented 7 years ago

I don't know why it broke, but if you change sub rotr(uint32 $n, uint32 $b) { to sub rotr($n, $b) {

the test passes...

CurtTilmes commented 7 years ago

The two cases differ when rotr is called with $n = 1652322944 and $b = 18.

In Rakudo 2017.04.3-210 rotr() returns 27071659120799 (correct). In Rakudo 2017.04.3-211, rotr() returns 27071659114496 (incorrect).

Isolating those values and calling the exact same subroutine with the two versions of Rakudo both give the same (correct) answer.

Rakudo broke that code, but only in some weird circumstance. The bug is in Rakudo, related to that commit, but I don't understand it and can't isolate it. It does seem to be related to converting Int to uint32. All is fine when I remove those.

zoffixznet commented 7 years ago

I don't know why it broke, but if you change sub rotr(uint32 $n, uint32 $b) { to sub rotr($n, $b) { the test passes...

@MasterDuke17 would you know if that's caused by the glitches with uint32 types not being real uints or something?

In Rakudo 2017.04.3-210 rotr() returns 27071659120799 (correct). In Rakudo 2017.04.3-211, rotr() returns 27071659114496 (incorrect).

I can repro the test failure as well as test fixing itself if uint32 type constraints are removed, but not any issues with the shift ops or the rotr giving wrong value there ^ I can't reproduce that, at least via the bot:

<Zoffix_> c: ef29bb9f41aa5d sub rotr(uint32 $n, uint32 $b) { $n +> $b +| $n +< (32 - $b) }; dd rotr 1652322944, 18
<committable6> Zoffix_, ¦ef29bb9: «27071659120799»

ATM, my suggestion would be to report this bug (by emailing to rakudobug@perl.org) with some test case that repros the issue and to remove the uint32 type constraints in the routine, as the native candidates are currently disabled, so you're not getting any perf boost ATM anyway.

zoffixznet commented 7 years ago

Disregard my comments above. Found it to be a SPESH issue. Filed as https://rt.perl.org/Ticket/Display.html?id=131306

SPESH stuff is beyond my current skill level, so I'm unsure when it'll get fixed.

zoffixznet commented 7 years ago

This Issue can be closed.

I added a kludge that works around the core issue in https://github.com/rakudo/rakudo/commit/2f22b701d4cf44fa43500edcc3450718fa7fd468 as well as tests to ensure it doesn't re-appear in https://github.com/perl6/roast/commit/c1d62112f3