Closed Bug 643829 Opened 9 years ago Closed 9 years ago

TI: Incorrect results with compiled zlib

Categories

(Core :: JavaScript Engine, defect)

Other Branch
x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: azakai, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

Attached file zlib
The attachment is zlib compiled to JavaScript (two versions - one with
optimizations, one without). Running it in jaegermonkey with -m gives incorrect results (an error in one version, an infinite loop in the other). The output without -m is valid (it does some zlib automatic tests and says they passed).

This is similar and perhaps related to bug 643635.
Attached file Reduced
This should print "undefined" and then 0-29. With -m -a it prints nothing. With -m it prints about 16 lines until it enters JM.

(FWIW I hacked the interpreter to replace expressions with their result. Works quite well to automatically reduce the last ~200 lines. We still need a syntax-based reducer to remove superfluous block statements..)
Attached patch Patch (obsolete) — Splinter Review
The lhs register was clobbered in jsop_relational_int. With this patch zlib works perfectly with all combinations of -m, -a and -n:

./js -m -a zlib.js
zlib version 1.2.5 = 4688, compile flags = 85
uncompress(): hello, hello!
inflate(): hello, hello!
large_inflate(): OK
after inflateSync(): hello, hello!
inflate with dictionary: hello, hello!
Assignee: general → jandemooij
Status: NEW → ASSIGNED
Attachment #521765 - Flags: review?(bhackett1024)
Attached patch Patch v2 (obsolete) — Splinter Review
This also pins lhs in the "target" case.
Attachment #521765 - Attachment is obsolete: true
Attachment #521765 - Flags: review?(bhackett1024)
Attachment #521771 - Flags: review?(bhackett1024)
Oops.  jsop_equality has the same problem in the obj == obj case, do you mind fixing that too?
Attached patch PatchSplinter Review
Attachment #521771 - Attachment is obsolete: true
Attachment #521771 - Flags: review?(bhackett1024)
Attachment #521902 - Flags: review?(bhackett1024)
I considered moving the regalloc outside the |if| but it added more complexity and I think this is safer considering the fixDoubleTypes/syncForBranch in the target case. Let me know if you think it's safe or better to move it.
Never mind, syncForBranch shuffles registers so we have to do it this way.
Attachment #521902 - Flags: review?(bhackett1024) → review+
http://hg.mozilla.org/projects/jaegermonkey/rev/3d7188751917
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
The 1_1 version in the attached archive segfaults on latest jaegermonkey with |-m -n| (while |-m| is ok).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The loop analysis added yesterday with the bounds hoisting broke on triply-nested loops with 'continue' statements going to the outermost loop.  This WFM now with -m -n.

http://hg.mozilla.org/projects/jaegermonkey/rev/6474999c14c6
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.