652305 - TI+JM: Kraken crypto-aes regression

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Reduced

$ ./js -n -m test.js
test.js:19: Error: Assertion failed: got -658505728, expected 1667457891

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Reduced further

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Reduced

Oops, interpreter-based reducer did not handle nested operators.

Comment 3

[Brian Hackett [Laid off!]]

Before branching to an opcode we ensure that things known to be doubles at the target are in fact doubles.  They stayed as doubles in the fallthrough, however, and when we then branched to a point they were known to be integers we didn't fix them again and were left interpreting a double value as an integer.

This fixes things to check both double and integer types before branching, but I'm not sure it's the right fix (would like to get kraken working again, while I think some more).  There are a couple other places we fix int-to-double but not double-to-int and it might be better to just not tolerate the compiler treating things as doubles which were inferred as ints, and get a more tightly focused fix for this situation.

http://hg.mozilla.org/projects/jaegermonkey/rev/e044a9a69132

Comment 4

[Brian Hackett [Laid off!]]

More targeted fix.  For branches with fallthroughs where we need to fix any double entries before branching, restore those fixed doubles before the next opcode.  We should maintain an invariant that any entries which were inferred as integers are not maintained as doubles by the compiler.  (Would be nice to maintain a stronger invariant that types in the compiler are always in sync with inferred types, but this may lose precision within inlined frames).

http://hg.mozilla.org/projects/jaegermonkey/rev/f394ef228e61

Comment 5

[Brian Hackett [Laid off!]]

Fix to the last fix's fix for the first fix (bug that will not die).  When restoring doubles back to integers, we stored raw frame entries in vectors rather than getting them from the FrameState itself, and ended up accessing entries that weren't being tracked by the FrameState.

http://hg.mozilla.org/projects/jaegermonkey/rev/591e2ce89668