Bug 90053 – Javascript SHA-512 gives wrong hash on second and subsequent runs unless Web Inspector Javascript Debugging is on

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[C Dan]

Created attachment 149698 [details]
Peasy test code and two Javasc SHA-512 implems

(Test file enc.)

If I run Javasc SHA-512 twice or more on a Webkit browser, only the first hash is correct. Ironically, this means the usual hash method of an initial test won't work because that passes but the real one then mismatches.

The test file runs two different SHA-512 implems, hashing the word "test" separately eight times for each. On non-Webk browsers (eg, Firefox) all 16 hashes are identical and match the expected result. On Webk browsers (eg, iCab) the first in each set are correct, but the remaining 14 are wrong -- but match each other.

Tiny Webkit (eg, Mobile Safari) is fine.

The beardy-weirdy thing is that I discovered by chance the tests work if you switch on Javasc Debugging in the Web Inspector. (My wild guess is that the non-debug Javasc is optimised to the point of reusing some info it should have cleared instead. This would explain why it's the same incorrect hash each time rather than random results.)

If the test file doesn't work, you can also see the effect at http://jssha.sourceforge.net/ -- input "test" in the Hashing Demo section and select SHA-512. Now delete and retype "test" and the hash changes. Switch on Javasc Debugging and try again and the hash is solid.

[Filip Pizlo]

(In reply to comment #0)
> Created an attachment (id=149698) [details]
> Peasy test code and two Javasc SHA-512 implems
> 
> (Test file enc.)
> 
> If I run Javasc SHA-512 twice or more on a Webkit browser, only the first hash is correct. Ironically, this means the usual hash method of an initial test won't work because that passes but the real one then mismatches.
> 
> The test file runs two different SHA-512 implems, hashing the word "test" separately eight times for each. On non-Webk browsers (eg, Firefox) all 16 hashes are identical and match the expected result. On Webk browsers (eg, iCab) the first in each set are correct, but the remaining 14 are wrong -- but match each other.
> 
> Tiny Webkit (eg, Mobile Safari) is fine.
> 
> The beardy-weirdy thing is that I discovered by chance the tests work if you switch on Javasc Debugging in the Web Inspector. (My wild guess is that the non-debug Javasc is optimised to the point of reusing some info it should have cleared instead. This would explain why it's the same incorrect hash each time rather than random results.)
> 
> If the test file doesn't work, you can also see the effect at http://jssha.sourceforge.net/ -- input "test" in the Hashing Demo section and select SHA-512. Now delete and retype "test" and the hash changes. Switch on Javasc Debugging and try again and the hash is solid.

Thanks for finding this!

JavaScriptCore has roughly speaking two ways of executing JS code: the baseline way (unoptimized) and the DFG way (DFG = Data Flow Graph JIT, i.e. optimized).  We switch between baseline and DFG on the fly.  They are *supposed* to execute code identically modulo that DFG is faster.  What you found is almost certainly a DFG bug - it is failing to obey correct JS semantics, and hence appears to execute code differently than baseline.

Because the DFG kicks in only after your code has executed for some time, it is often the case that the first execution will run in baseline and subsequent executions run in DFG.  Hence your observation that the first run is right but the others are wrong.

Currently, turning on the debugger turns off the DFG.  Hence your observation that turning on the debugger "fixes" the problem.

I'll try to fix this ASAP.  And again, thanks for the detailed bug report.

[Filip Pizlo]

Found the problem: it appears that our logic for recovering unsigned 32-bit integers on OSR exit is totally broken. :-/  I'm in the process of testing a patch to unbreak it, and return it to its former glory.

[Filip Pizlo]

<rdar://problem/11764613>

[Filip Pizlo]

Created attachment 149818 [details]
the patch

[Mark Hahnenberg]

Comment on attachment 149818 [details]
the patch

r=me

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/121391

[C Dan]

Hurrah! Trem exc. Thanks for the fix and the interesting explan.

(By coincidence the Blowfish Javasc I was playing with alongside SHA-512 turned out to have a slight bug in the implem -- nothing to do with Webk -- so neither popular well-established lib worked and I thought I'd gone bonkers.)