The default bug view has changed. See this FAQ.

"Assertion failure: sharedBlock," with e4x

VERIFIED FIXED in Firefox 8

Status

()

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

People

(Reporter: gkw, Assigned: jorendorff)

Tracking

(Blocks: 1 bug, {assertion, regression, testcase})

Trunk
mozilla8
x86
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox8+ fixed)

Details

(Whiteboard: js-triage-needed [qa-])

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 547348 [details]
stack

for (let w in [0]) {
    let([] = ([
        <x/>
    ]), r = <x/>) {}
}

asserts js debug shell on MI changeset 99d121a0f799 without any CLI arguments at Assertion failure: sharedBlock,

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   72780:69dfa4008531
user:        Jason Orendorff
date:        Thu Jul 14 08:54:17 2011 -0500
summary:     Bug 646968 - Fix name lookups in let scopes. r=brendan.
Whiteboard: js-triage-needed
tracking-8 because this is a regression from something landed in 8.
Assignee: general → jorendorff
status-firefox8: --- → affected
tracking-firefox8: --- → +
(Assignee)

Comment 2

6 years ago
Whew.

There are two JSOP_TOXML opcodes in this, at offsets 45 and 59. JSOP_TOXML indirectly calls js::GetScopeChain.

The problem is that the first toxml is (incorrectly) dynamically inside the innermost let scope. In short, everything goes nuts from there.

dis();
00000:  callgname "dis"
00003:  call 0
00006:  popv

for (let w in [0]) {
00007:  enterblock depth 0 {w: 0}
00010:  nop
00011:  nullblockchain
00012:  object [0]
00015:  nop
00016:  blockchain depth 0 {w: 0}
00019:  iter 1
00021:  goto 73 (+52)

    w <- next value
00024:  trace 0
00027:  iternext 1
00029:  setlocal 0
00032:  pop

    let ([] = ([<x/>]),
00033:  enterblock depth 2 {r: 1, : 0}
00036:  newarray 1
00040:  zero
00041:  startxml
00042:  string "<x/>"
00045:  toxml
00046:  initelem
00047:  endinit
00048:  dup
00049:  pop
00050:  pop

        r = <x/>)
00051:  nop
00052:  blockchain depth 0 {w: 0}
00055:  startxml
00056:  string "<x/>"
00059:  toxml
00060:  nop
00061:  blockchain depth 2 {r: 1, : 0}
00064:  setlocal 3
00067:  pop

    {}
00068:  leaveblock 2

}
00073:  moreiter
00074:  ifne 24 (-50)
00077:  enditer
00078:  leaveblock 1
00083:  stop

Might be a straightforward bytecode emitter bug. jsemit is supposed to be using TempPopScope every time it emits code in a let-head, but here there's destructuring assignment and we're not. I must have missed a case.
(Assignee)

Comment 3

6 years ago
Created attachment 548361 [details] [diff] [review]
v1

...I'm particularly proud of the comments, which almost make it sound like this really isn't that precarious at all.
Attachment #548361 - Flags: review?(brendan)
Comment on attachment 548361 [details] [diff] [review]
v1

Argh, I shoulda grepped when reviewing the last patch.

/be
Attachment #548361 - Flags: review?(brendan) → review+
http://hg.mozilla.org/mozilla-central/rev/a73864715b24
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Marking fixed on firefox8 based on checkin two weeks before migration
status-firefox8: affected → fixed
qa- as nothing to do for QA fix verification -- please reset to qa+ if there is something for QA to verify.
Whiteboard: js-triage-needed → js-triage-needed [qa-]
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/regress/regress-673070-3.js.
Flags: in-testsuite+
(Reporter)

Comment 9

4 years ago
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.