Null crash when logging weak maps

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mccr8, Assigned: peterv)

Tracking

({crash, regression})

Trunk
mozilla48
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed, firefox48 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

3 years ago
Peter hit this crash the other day, and Kris Maglione hit the same thing, so we should fix it. Peter, do you have your patch handy that adds the null check? The paste bin expired.
Reporter

Comment 1

3 years ago
I think this is a regression that landed in 47 so we should uplift it.
Flags: needinfo?(peterv)
Reporter

Updated

3 years ago
Assignee: nobody → continuation
Blocks: 1247679
Reporter

Updated

3 years ago
Keywords: regression
Reporter

Comment 2

3 years ago
This is the crash stack:

Assertion failure: asCell(), at /home/kris/code/mozilla-central/obj-debug/dist/include/js/HeapAPI.h:216
#01: JS::GCCellPtr::unsafeAsUIntPtr() const (/home/kris/code/mozilla-central/obj-debug/dist/include/js/HeapAPI.h:216 (discriminator 4))
#02: JS::GCCellPtr::unsafeAsInteger() const (/home/kris/code/mozilla-central/obj-debug/dist/include/js/HeapAPI.h:212)
#03: CCGraphBuilder::NoteWeakMapping(JSObject*, JS::GCCellPtr, JSObject*, JS::GCCellPtr) (/home/kris/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:2475)
#04: non-virtual thunk to CCGraphBuilder::NoteWeakMapping(JSObject*, JS::GCCellPtr, JSObject*, JS::GCCellPtr) (/home/kris/code/mozilla-central/xpcom/base/nsCycleCollector.cpp:2463)
#05: NoteWeakMapsTracer::trace(JSObject*, JS::GCCellPtr, JS::GCCellPtr) (/home/kris/code/mozilla-central/xpcom/base/CycleCollectedJSRuntime.cpp:220)
Assignee

Comment 3

3 years ago
Posted patch v1Splinter Review
Sorry, didn't have time to file this yet.
Assignee: continuation → peterv
Status: NEW → ASSIGNED
Flags: needinfo?(peterv)
Assignee

Comment 4

3 years ago
Comment on attachment 8728866 [details] [diff] [review]
v1

Review of attachment 8728866 [details] [diff] [review]:
-----------------------------------------------------------------

I already had an r=mccr8 on irc, so I'll just land this.
Attachment #8728866 - Flags: review+

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3667b972ff45
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reporter

Comment 7

3 years ago
Comment on attachment 8728866 [details] [diff] [review]
v1

Approval Request Comment
[Feature/regressing bug #]: bug 1247679
[User impact if declined]: crashes when Firefox developers are debugging leaks
[Describe test coverage new/current, TreeHerder]: no test coverage
[Risks and why]: very low: this adds a null check to code that only runs when a Firefox developer is debugging a leak
[String/UUID change made/needed]: none
Attachment #8728866 - Flags: approval-mozilla-aurora?
Comment on attachment 8728866 [details] [diff] [review]
v1

Simple enough, crash fix. Aurora47+
Attachment #8728866 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: crash
You need to log in before you can comment on or make changes to this bug.