Last Comment Bug 652305 - TI+JM: Kraken crypto-aes regression
: TI+JM: Kraken crypto-aes regression
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: ---
Assigned To: general
: Jason Orendorff [:jorendorff]
Depends on:
Blocks: infer-regress
  Show dependency treegraph
Reported: 2011-04-23 00:06 PDT by Jan de Mooij [:jandem]
Modified: 2011-04-24 08:56 PDT (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Reduced (2.76 KB, application/x-javascript)
2011-04-23 00:06 PDT, Jan de Mooij [:jandem]
no flags Details
Reduced further (1.59 KB, application/x-javascript)
2011-04-23 00:15 PDT, Jan de Mooij [:jandem]
no flags Details
Reduced (1.42 KB, application/x-javascript)
2011-04-23 00:38 PDT, Jan de Mooij [:jandem]
no flags Details

Description User image Jan de Mooij [:jandem] 2011-04-23 00:06:46 PDT
Created attachment 527916 [details]

$ ./js -n -m test.js
test.js:19: Error: Assertion failed: got -658505728, expected 1667457891
Comment 1 User image Jan de Mooij [:jandem] 2011-04-23 00:15:32 PDT
Created attachment 527918 [details]
Reduced further
Comment 2 User image Jan de Mooij [:jandem] 2011-04-23 00:38:21 PDT
Created attachment 527920 [details]

Oops, interpreter-based reducer did not handle nested operators.
Comment 3 User image Brian Hackett (:bhackett) 2011-04-23 22:50:02 PDT
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.
Comment 4 User image Brian Hackett (:bhackett) 2011-04-24 07:08:01 PDT
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).
Comment 5 User image Brian Hackett (:bhackett) 2011-04-24 08:56:44 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.