The default bug view has changed. See this FAQ.

"Assertion failure: &obj->getSlotRef(slot) == this,"

VERIFIED FIXED in Firefox 15

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: billm)

Tracking

(Blocks: 1 bug, 5 keywords)

Trunk
mozilla17
assertion, regression, sec-critical, testcase, valgrind
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox13 unaffected, firefox14 unaffected, firefox15+ verified, firefox16+ verified, firefox17+ verified, firefox-esr1015+ verified)

Details

(Whiteboard: [js:inv:p1][advisory-tracking+])

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
Created attachment 626983 [details]
stack (including Valgrind stack)

function f(y) {
  x = ["", "", "", "", "", "", "", "", "", "", "", "", "", ""]
  for (var i = 0; i < x.length; ++i) {
    y[i] = []
  }
  return y
}
evalcx("undefined.function::e", f(evalcx("lazy")))

asserts js debug shell on m-c changeset 6ad95e65d74d without any CLI arguments at Assertion failure: &obj->getSlotRef(slot) == this,

s-s because Valgrind shows invalid reads in an opt build.

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   93639:fbff86190de6
user:        Brian Hackett
date:        Wed May 09 14:14:10 2012 -0700
summary:     Don't throw away JIT code for compartments in web pages displaying animations, bug 750834. r=billm
I doubt fbff86190de6 is involved (though it's not impossible), as the method JIT isn't enabled here and the GC related changes in fbff86190de6 are mainly gated in the shell on gcPreserveCode() being called, which isn't the case here.  This looks like a issue with barriers and e4x?
Whiteboard: js-triage-needed → [js:inv:p1]
(Reporter)

Comment 2

5 years ago
sec-critical based on invalid reads from Valgrind.
Keywords: sec-critical
(Reporter)

Comment 3

5 years ago
WFM on m-c changeset bf8f2961d0cc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 4

5 years ago
Actually, not true:

options('allow_xml');
function f(y) {
  x = ["", "", "", "", "", "", "", "", "", "", "", "", "", ""]
  for (var i = 0; i < x.length; ++i) {
    y[i] = []
  }
  return y
}
evalcx("undefined.function::e", f(evalcx("lazy")));


still asserts identically.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
(Reporter)

Comment 5

5 years ago
I could reproduce on 64-bit Mac js shells.
Status: REOPENED → NEW
OS: Linux → All
Hardware: x86 → All
(Reporter)

Comment 6

5 years ago
Tested on m-c rev bf8f2961d0cc.
(Reporter)

Updated

5 years ago
status-firefox-esr10: --- → unaffected
status-firefox13: --- → unaffected
status-firefox14: --- → unaffected
status-firefox15: --- → affected
status-firefox16: --- → affected
(Assignee)

Comment 7

5 years ago
I'll take this, but I doubt it's related to incremental GC. The testcase isn't running any incremental GCs or any barrier verification, so the write barriers shouldn't be triggering.
Assignee: general → wmccloskey
(Assignee)

Comment 8

5 years ago
Created attachment 639505 [details] [diff] [review]
patch

We call getSlotRef. Later we call NewXMLNamespace, which causes the slots vector to be reallocated at a different place. Finally we try to use the result of getSlotRef again. It asserts the location it returned is no longer the right one.

I think this regressed in bug 650369, which landed in FF6.
Attachment #639505 - Flags: review?(jwalden+bmo)
tracking-firefox15: --- → +
tracking-firefox16: --- → +
Attachment #639505 - Flags: review?(jwalden+bmo) → review+
(Reporter)

Comment 9

5 years ago
Bill, I wanted to help check this in but it bitrotted a little.

$ hg qpush
applying bug758408.patch
patching file js/src/jsxml.cpp
Hunk #1 FAILED at 7495
Hunk #2 FAILED at 7514
2 out of 2 hunks FAILED -- saving rejects to file js/src/jsxml.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug758408.patch

Updated

5 years ago
status-firefox17: --- → affected
tracking-firefox17: --- → +
(Assignee)

Comment 10

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2fe4ea4a614e
Target Milestone: --- → mozilla17
(Assignee)

Comment 11

5 years ago
Comment on attachment 639505 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 650369
User impact if declined: Possible crashes, exploitation.
Testing completed (on m-c, etc.): On m-c.
Risk to taking this patch (and alternatives if risky): Very low.
String or UUID changes made by this patch: None.
Attachment #639505 - Flags: approval-mozilla-esr10?
Attachment #639505 - Flags: approval-mozilla-beta?
Attachment #639505 - Flags: approval-mozilla-aurora?
(Reporter)

Comment 12

5 years ago
Testcase to be landed when mozilla-beta makes it out to release in 6 weeks' time, setting in-testsuite? first.
Flags: in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/2fe4ea4a614e
Status: NEW → RESOLVED
Last Resolved: 5 years ago5 years ago
status-firefox17: affected → fixed
Resolution: --- → FIXED
If bug 650369 is the regressing bug, this affects the ESR as well.
status-firefox-esr10: unaffected → affected
tracking-firefox-esr10: --- → 15+
Comment on attachment 639505 [details] [diff] [review]
patch

[Triage Comment]
Low risk sg:crit fix, approving for all branches.
Attachment #639505 - Flags: approval-mozilla-esr10?
Attachment #639505 - Flags: approval-mozilla-esr10+
Attachment #639505 - Flags: approval-mozilla-beta?
Attachment #639505 - Flags: approval-mozilla-beta+
Attachment #639505 - Flags: approval-mozilla-aurora?
Attachment #639505 - Flags: approval-mozilla-aurora+
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/81c6465d31b6
https://hg.mozilla.org/releases/mozilla-beta/rev/5c6e3097670e
https://hg.mozilla.org/releases/mozilla-esr10/rev/fc115a7cc14a
status-firefox-esr10: affected → fixed
status-firefox15: affected → fixed
status-firefox16: affected → fixed
Whiteboard: [js:inv:p1] → [js:inv:p1][advisory-tracking+]
Assigning to myself to resolve in-testsuite? flag to serve as verification. Gary, would you care to take some time to mentor me?
QA Contact: anthony.s.hughes
Whiteboard: [js:inv:p1][advisory-tracking+] → [js:inv:p1][advisory-tracking+][qa+]
(Reporter)

Comment 18

5 years ago
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #17)
> Assigning to myself to resolve in-testsuite? flag to serve as verification.
> Gary, would you care to take some time to mentor me?

No problem - try saving comment 4 into a file (758408-c4.js for example and run ./js 758408-c4.js where the js binary corresponds to the binaries according to the changesets below):

Do you see this assertion on mozilla-central changeset 2fe4ea4a614e ?

Do you see this assertion on its parent, mozilla-central changeset b56cc7740240 ?

If the first one is no, and the second one is yes, then you're successfully verified that mozilla-central changeset 2fe4ea4a614e would have fixed this.
Keywords: verifyme
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #18)
> Do you see this assertion on mozilla-central changeset 2fe4ea4a614e ?

I see the following:
$ ./objdir-ff-debug/js/src/js ../758408-c4.js 
../758408-c4.js:9: TypeError: undefined has no properties
(Reporter)

Comment 20

5 years ago
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #19)
> (In reply to Gary Kwong [:gkw, :nth10sd] from comment #18)
> > Do you see this assertion on mozilla-central changeset 2fe4ea4a614e ?
> 
> I see the following:
> $ ./objdir-ff-debug/js/src/js ../758408-c4.js 
> ../758408-c4.js:9: TypeError: undefined has no properties

This likely means we need a comment at the top of the .js test:

// |jit-test| TypeError

to show that the TypeError is expected.
Confirmed testcase reproducible with 2012-05-24 mozilla-central debug JS shell.

Verified fixed with:
 * 2012-08-24 mozilla-central debug JS shell
 * 2012-08-24 mozilla-aurora debug JS shell
 * 2012-08-24 mozilla-beta debug JS shell
 * 2012-08-24 mozilla-esr10 debug JS shell
Status: RESOLVED → VERIFIED
status-firefox-esr10: fixed → verified
status-firefox15: fixed → verified
status-firefox16: fixed → verified
status-firefox17: fixed → verified
Keywords: verifyme
Whiteboard: [js:inv:p1][advisory-tracking+][qa+] → [js:inv:p1][advisory-tracking+]
Group: core-security
E4X has been removed, so we won't add the test.
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.