643829 - TI: Incorrect results with compiled zlib

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Alon Zakai (:azakai)]

Attachment: 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.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: 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..)

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Patch

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!

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Patch v2

This also pins lhs in the "target" case.

Comment 4

[Brian Hackett [Laid off!]]

Oops.  jsop_equality has the same problem in the obj == obj case, do you mind fixing that too?

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 6

[Jan de Mooij [:jandem]]

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.

Comment 7

[Jan de Mooij [:jandem]]

Never mind, syncForBranch shuffles registers so we have to do it this way.

Comment 8

[Brian Hackett [Laid off!]]

http://hg.mozilla.org/projects/jaegermonkey/rev/3d7188751917

Comment 9

[Alon Zakai (:azakai)]

The 1_1 version in the attached archive segfaults on latest jaegermonkey with |-m -n| (while |-m| is ok).

Comment 10

[Brian Hackett [Laid off!]]

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