660438 - TM: Different output with testcase with and without -m, -a and -j

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

try {
    function x() {}
    x.valueOf = this
    for (let a = 0; a < 8; a++) {
        print(x < x)
    }
} catch (e) {}

on JM changeset 96fae421af85, tested on 64-bit Mac 10.6.

outputs: (without CLI parameters)

false
false
false
false
false
false
false
false

with -m -a -j :

false
false
false
false
false
false
false

(one less false)

Not sure if this is correct:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   69936:200fb5d19aa6
user:        Jan de Mooij
date:        Fri May 27 13:56:11 2011 +0200
summary:     Bug 624298 - Add an IC for JSOP_CALLNAME. r=dvander

Comment 1

[Jan de Mooij [:jandem]]

The CALLNAME IC exposed this problem, but I think it's a pre-existing tracer bug. This modified test case:
--
function x() {}
x.valueOf = this
for (var a = 0; a < 9; a++) {
    x < 1;
}
--
Fails with -j:
TypeError: #1={x:function x() {}, a:7} is not a function

Comment 2

[Jan de Mooij [:jandem]]

Gary would you mind auto-bisecting this test case? I still have to learn how to do this myself..

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   64402:0906d9490eaf
user:        Jeff Walden
date:        Mon Mar 28 20:01:53 2011 -0700
summary:     Bug 645468 - Remove js_TryMethod: its semantics aren't what most of its users want, and its utility is limited.  r=luke

Comment 4

[Jeff Walden [:Waldo]]

Sigh, forgot the imacros encode the old/wrong default-value semantics.

Comment 5

[Jeff Walden [:Waldo]]

Attachment: Patch and tests

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 536372 [details] [diff] [review]
Patch and tests

Review of attachment 536372 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsopcode.tbl
@@ +505,5 @@
>  OPDEF(JSOP_ENTERBLOCK,    201,"enterblock",  NULL,    3,  0, -1,  0,  JOF_OBJECT)
>  OPDEF(JSOP_LEAVEBLOCK,    202,"leaveblock",  NULL,    5, -1,  0,  0,  JOF_UINT16)
>  
> +/* Jump to target if top of stack value isn't callable. */
> +OPDEF(JSOP_IFCANTCALLTOP, 203,"ifcantcalltop",NULL,   3,  1,  1,  0,  JOF_JUMP|JOF_DETECTING)

Are you sure you don't want to use something more in the terse tradition of opcodes.  JSOP_IFCNTCLTP perhaps? ;-)

::: js/src/jstracer.cpp
@@ +15642,5 @@
> +        return ARECORD_CONTINUE;
> +
> +    // The tricky part: callable objects that aren't also functions.
> +    LIns* args[] = { get(&top) };
> +    guard(!top.toObject().isCallable(), w.eqi0(w.call(&IsCallable_tn_ci, args)), MISMATCH_EXIT);

IIUC, this will let us trace calls to non-function callables.  Can we just abort recording in such a case instead of trying to make it work?

Comment 7

[Jeff Walden [:Waldo]]

Attachment: Tweaked

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 536960 [details] [diff] [review]
Tweaked

Weee.

Comment 9

[Jeff Walden [:Waldo]]

http://hg.mozilla.org/tracemonkey/rev/84b333a052ad

Comment 10

[Chris Leary [:cdleary] (not checking bugmail)]

cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/84b333a052ad

Comment 11

[Christian Holler (:decoder)]

Bug in removed tracer code, setting in-testsuite- flag.