Closed Bug 541191 Opened 14 years ago Closed 14 years ago

Regression on the r-tree benchmark

Categories

(Core :: JavaScript Engine, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta8+

People

(Reporter: renaud.durlin, Assigned: dvander)

References

()

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20100120 Minefield/3.7a1pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20100120 Minefield/3.7a1pre

Error when running a javascript benchmark.

This is a dup of Bug 539553 (sorry). I filled a new bug because the other bug is private.
Something land on 2010-01-14 (6c9304d3aa15), I don't know if this was supposed to fix this problem, but apparently it doesn't.

Reproducible: Always

Steps to Reproduce:
1. go to http://stackulator.com/rtree/numbers.html

Actual Results:  
Error: b is undefined
Source File: http://stackulator.com/rtree/rtree.js
Line: 689

Expected Results:  
No error!

Same problem with Firefox 3.6 with a new profile.
Blocks: 539553
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → general
Component: General → JavaScript Engine
Product: Firefox → Core
QA Contact: general → general
Version: unspecified → Trunk
Attached patch fix (obsolete) — Splinter Review
fun_apply isn't using the correct length. doesn't matter if we were to trace this test case with the correct length, we'd bail out every time in the imacro's GETELEM.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #438312 - Flags: review?(brendan)
this is a correctness issue and affects 1.9.2
blocking1.9.2: --- → ?
Comment on attachment 438312 [details] [diff] [review]
fix

And we don't need to guard on trace because the apply-invoked call is already guarding on argc, right?

Thanks,

/be
Attachment #438312 - Flags: review?(brendan) → review+
Attached patch better fix (obsolete) — Splinter Review
as brendan rightly questioned, we do have to guard at runtime because in some cases the upcoming GETELEMs won't actually abort the trace, and there is no guard on the argc.

let's just trace this instead. we'll actually stay on trace in some cases (see third test case).
Attachment #438312 - Attachment is obsolete: true
Attachment #438317 - Flags: review?(brendan)
Comment on attachment 438317 [details] [diff] [review]
better fix

And of course, this is not right either, because of:

// Trigger length override
arguments.length = 1;
// Delete lenght property
delete arguments.length;
// Re-define it with a getter that wreaks havoc on trace
arguments.__defineGetter__('length', function () { return eval('1'); });
Attachment #438317 - Flags: review?(brendan)
This is too late for 1.9.2.10. Please nominate for a later update if you feel it is still needed at that time.
blocking1.9.2: ? → ---
David, why isn't any nastiness such as comment 5 a deep-bail from builtin situation?

/be
Above I mean too late for 1.9.2.4.
blocking1.9.2: --- → needed
FYI, I no longer see the bug with Firefox 3.6.9.
So what's the story here?  Are we planning to fix this on 1.9.2?
And this is a problem on m-c too, right?
blocking2.0: --- → ?
blocking2.0: ? → beta8+
Attached patch fixSplinter Review
I feel better about not tracing this now. We didn't trace it anyway.
Attachment #438317 - Attachment is obsolete: true
Attachment #480815 - Flags: review?(dmandelin)
Attachment #480815 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/mozilla-central/rev/71a6cebbb7c7
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
blocking1.9.2: needed → ---
You need to log in before you can comment on or make changes to this bug.