Closed Bug 636394 Opened 9 years ago Closed 9 years ago

"Assertion failure: canHaveMethodBarrier(),"

Categories

(Core :: JavaScript Engine, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
blocking2.0 --- .x+

People

(Reporter: gkw, Assigned: jorendorff)

References

Details

(Keywords: assertion, regression, testcase, Whiteboard: [fixed-in-tracemonkey])

Attachments

(3 files)

a = /f/
Boolean.prototype.p0 = function () {}
a.__proto__ = Boolean.prototype;
a.watch("p0", function () {})
a.p0 = function () {}

asserts js debug shell on TM changeset 9d8a96a61d71 without -m nor -j at Assertion failure: canHaveMethodBarrier()

Hoping for .x
OS: Windows 7 → All
Hardware: x86 → All
Attached file stack
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   48599:80382d88b92c
user:        Brendan Eich
date:        Fri Jul 23 14:41:56 2010 -0700
summary:     Arguments.callee.caller does not work in FF 4 under certain circumstances (577648, r=jwalden).
Blocks: 577648
I think the testcase in comment #0 needs to be passed in as a CLI argument..
Assignee: general → jorendorff
blocking2.0: ? → .x+
Group: core-security
Attached patch fixSplinter Review
The barrier tries to access the a variable, but can't because it's no object/function/primitive or date, it's a regexp. The patch adds regexp to the list of things that can have an Method barrier.
Attachment #514875 - Flags: review?(jorendorff)
That's not the root cause of the bug though. You could then trigger the same assertion by changing the first line so that `a` is a Date object or anything else.

Thanks for giving this some thought, kosver. Please take a second to see how tests can be added. Every patch for a bug like this needs to add a test.
The issue is that setting a watchpoint can "Clone the prototype property so we can watch the right object". We're cloning a method property onto an object that can't have methods. Cloning a method should trigger the read barrier on the prototype first.

Patching...
Attached patch v1Splinter Review
Attachment #515156 - Flags: review?(brendan)
Attachment #514875 - Flags: review?(jorendorff)
Attachment #515156 - Flags: review?(brendan) → review+
Why is this bug s-s? It's at worst a pigeon-hole problem in compile-and-go code where people collide on the identity and mutability of one joined function object when they expect unjoined clones.

I see no issues in an opt shell.

Gary, I will let you open it.

/be
(In reply to comment #7)
> I see no issues in an opt shell.
> 
> Gary, I will let you open it.

No errors in valgrind using opt shell.

$ valgrind ./js-opt-32-tm-linux ~/Desktop/636394.js 
==7431== Memcheck, a memory error detector
==7431== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==7431== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==7431== Command: ./js-opt-32-tm-linux /home/fuzz1/Desktop/636394.js
==7431== 
==7431== 
==7431== HEAP SUMMARY:
==7431==     in use at exit: 0 bytes in 0 blocks
==7431==   total heap usage: 769 allocs, 769 frees, 627,513 bytes allocated
==7431== 
==7431== All heap blocks were freed -- no leaks are possible
==7431== 
==7431== For counts of detected and suppressed errors, rerun with: -v
==7431== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 22 from 7)
Group: core-security
Comment on attachment 515156 [details] [diff] [review]
v1

This is not a blocker, but it's one of a series of watchpoint bugs. I'd like to get all the fixes into FF4.

This particular patch is especially straightforward. It fixes an assertion.
Attachment #515156 - Flags: approval2.0?
Attachment #515156 - Flags: approval2.0? → approval2.0+
Missed the train. Will fix for .x. Removing approval.
Attachment #515156 - Flags: approval2.0+
http://hg.mozilla.org/tracemonkey/rev/542cd23169c2
Whiteboard: [fixed-in-tracemonkey]
http://hg.mozilla.org/mozilla-central/rev/542cd23169c2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/regress/regress-636394.js.
Flags: in-testsuite+
Testcases have been landed by virtue of being marked in-testsuite+ -> VERIFIED as well.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.