Closed
Bug 788419
Opened 12 years ago
Closed 12 years ago
crash in js::DebugScopeProxy::handleUnaliasedAccess
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: Honza, Assigned: luke)
Details
(Keywords: crash, Whiteboard: [firebug-p1])
Crash Data
Attachments
(1 file)
1.68 KB,
patch
|
jimb
:
review+
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-2c8c7d52-2297-4a48-9bd6-f0c752120905 . ============================================================= According to Firebug automated test suite, this crash appeared around 3rd of September. STR: 1) Install Firebug from here: https://getfirebug.com/releases/firebug/1.11/firebug-1.11.0a2.xpi 2) Install FBTest extension from here: https://getfirebug.com/releases/fbtest/1.11/fbTest-1.11b1.xpi 3) Open Firebug 4) open FBTest, Firebug (icon) menu -> Open Test Console 5) Press 'Run All' button at the top of the test console window Doesn't happen all the time Honza
Reporter | ||
Updated•12 years ago
|
Whiteboard: [firebug-p1]
Comment 1•12 years ago
|
||
I also already experienced this today: https://crash-stats.mozilla.com/report/index/bp-24040e56-d0aa-4a4b-a010-131962120905 Sebastian
Comment 2•12 years ago
|
||
More precise STR: - Install Firebug, and open https://getfirebug.com/tests/head/script/callstack/4845/issue4845.html - Reload with script panel enabled - execution should stop at a debugger; statement in secondLevelFunction() - Press Shift-F8 (= re-run) -> sometimes it crashes https://crash-stats.mozilla.com/report/index/4bd04342-cd1e-48fd-aa69-3a2c82120905 I tried a debug build from https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-central-linux/1346844012/ but didn't see any assertion failures. Here's a (symbol-less) stack from that: https://crash-stats.mozilla.com/report/index/55bb26a0-d404-4623-945d-6ea0e2120905
Assignee | ||
Comment 3•12 years ago
|
||
Oh gosh, that loop is flagrantly wrong. Pretty hard to exercise since has() shields handleUnaliasedAccess from random name lookups; you have to add a new property with frame.eval and then access it.
Assignee | ||
Updated•12 years ago
|
status-firefox17:
--- → affected
tracking-firefox17:
--- → ?
Comment 4•12 years ago
|
||
Comment on attachment 658557 [details] [diff] [review] fix and test Review of attachment 658557 [details] [diff] [review]: ----------------------------------------------------------------- The fix to the loop looks obviously correct, but I wanted to understand why the test case trips the bug. I take it the situation here is: - f is not heavyweight, and has no locals, so its bindings list is just the empty shape. - When we call GetDebugScope we create an ersatz CallObject for it. - When we evaluate the 'var blah=43': - The 'has' method's loop over the bindings is properly written, so defineProperty creates a property on the CallObject that has no counterpart in the script's Bindings list. - When we store the value from the 'var' statement's initializer, the 'set' method gives handleUnaliasedAccess a shot first. The bogus loop then trips over the empty bindings list, instead of simply failing, returning false, and letting 'set' write the value to the ersatz CallObject's property. Is that right?
Attachment #658557 -
Flags: review?(jimb) → review+
Comment 5•12 years ago
|
||
... and the 'blah = 44' test is just there to ensure that ordinary stores, not associated with any definition, also work.
Assignee | ||
Comment 6•12 years ago
|
||
You are correct sir!
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f2ba5257ab22
Assignee | ||
Comment 8•12 years ago
|
||
Comment on attachment 658557 [details] [diff] [review] fix and test [Approval Request Comment] Bug caused by (feature/regressing bug #): 780647 User impact if declined: debugger crashes Testing completed (on m-c, etc.): m-c
Attachment #658557 -
Flags: approval-mozilla-aurora?
Updated•12 years ago
|
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/6bc6c8a55543
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f2ba5257ab22
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Reporter | ||
Comment 11•12 years ago
|
||
Thanks for quick fix! What is the easiest way to find out whether the path is already part of Nightly? honza
Comment 12•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #8) > [Approval Request Comment] > Bug caused by (feature/regressing bug #): 780647 > User impact if declined: debugger crashes > Testing completed (on m-c, etc.): m-c Luke - what's the risk profile here? I ask because if anything but low, we may want to point QA at this bug specifically for exploratory regression testing, or discuss alternatives.
Comment 13•12 years ago
|
||
(In reply to Alex Keybl [:akeybl] from comment #12) > Luke - what's the risk profile here? I ask because if anything but low, we > may want to point QA at this bug specifically for exploratory regression > testing, or discuss alternatives. I would say the risk profile for this patch is low: the loop simply fails to check its conditions in the right order, and fails when there are no iterations to perform. The fix is the obvious rearrangement. It's the conditions for running into the bug, as given in the test case, that are subtle.
Comment 14•12 years ago
|
||
Comment on attachment 658557 [details] [diff] [review] fix and test [Triage Comment] Thanks Jim. Given where we are in the cycle, and the low risk profile, we'll take this in support of Firebug workflows working correctly.
Attachment #658557 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•