Last Comment Bug 660438 - TM: Different output with testcase with and without -m, -a and -j
: TM: Different output with testcase with and without -m, -a and -j
Status: RESOLVED FIXED
fixed-in-tracemonkey
: regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on:
Blocks: jsfunfuzz js-differential-test infer-regress 645468
  Show dependency treegraph
 
Reported: 2011-05-28 03:47 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-02-25 16:51 PST (History)
8 users (show)
choller: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch and tests (14.75 KB, patch)
2011-05-31 12:26 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
Tweaked (14.49 KB, patch)
2011-06-02 13:11 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
luke: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2011-05-28 03:47:41 PDT
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] 2011-05-28 04:49:39 PDT
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] 2011-05-28 04:51:38 PDT
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] 2011-05-28 07:21:11 PDT
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] (remove +bmo to email) 2011-05-29 19:16:24 PDT
Sigh, forgot the imacros encode the old/wrong default-value semantics.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2011-05-31 12:26:22 PDT
Created attachment 536372 [details] [diff] [review]
Patch and tests
Comment 6 Luke Wagner [:luke] 2011-05-31 16:28:51 PDT
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] (remove +bmo to email) 2011-06-02 13:11:00 PDT
Created attachment 536960 [details] [diff] [review]
Tweaked
Comment 8 Luke Wagner [:luke] 2011-06-02 15:31:27 PDT
Comment on attachment 536960 [details] [diff] [review]
Tweaked

Weee.
Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2011-06-13 18:18:54 PDT
http://hg.mozilla.org/tracemonkey/rev/84b333a052ad
Comment 10 Chris Leary [:cdleary] (not checking bugmail) 2011-06-20 17:02:49 PDT
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/84b333a052ad
Comment 11 Christian Holler (:decoder) 2013-02-25 16:51:41 PST
Bug in removed tracer code, setting in-testsuite- flag.

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