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

RESOLVED FIXED in mozilla7

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: gkw, Assigned: Waldo)

Tracking

(Blocks: 3 bugs, {regression, testcase})

Trunk
mozilla7
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
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

Updated

6 years ago
Assignee: general → jandemooij
Status: NEW → ASSIGNED
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
Gary would you mind auto-bisecting this test case? I still have to learn how to do this myself..
Assignee: jandemooij → general
Status: ASSIGNED → NEW
(Reporter)

Comment 3

6 years ago
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
Blocks: 465479, 645468
No longer blocks: 349611, 624298
(Reporter)

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
Sigh, forgot the imacros encode the old/wrong default-value semantics.
Assignee: general → jwalden+bmo
Created attachment 536372 [details] [diff] [review]
Patch and tests
Attachment #536372 - Flags: review?(luke)

Comment 6

6 years ago
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?
Created attachment 536960 [details] [diff] [review]
Tweaked
Attachment #536372 - Attachment is obsolete: true
Attachment #536372 - Flags: review?(luke)
Attachment #536960 - Flags: review?(luke)

Comment 8

6 years ago
Comment on attachment 536960 [details] [diff] [review]
Tweaked

Weee.
Attachment #536960 - Flags: review?(luke) → review+
http://hg.mozilla.org/tracemonkey/rev/84b333a052ad
Status: NEW → ASSIGNED
Whiteboard: fixed-in-tracemonkey
Target Milestone: --- → mozilla7
cdleary-bot mozilla-central merge info:
http://hg.mozilla.org/mozilla-central/rev/84b333a052ad
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Reporter)

Updated

6 years ago
Blocks: 349611
Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.