1247381 - Replace a ! that got dropped in bug 1105069 part 7

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Terrence Cole [:terrence]]

Attachment: fix_dropped_not-v0.diff

Andrew discovered in bug 1246720 that I had accidentally dropped a ! when converting weak tracing to GCCellPtrs as part of bug 1105069, part 7.

http://hg.mozilla.org/mozilla-central/rev/aa2a54fffd77

I think that this means we will simply not visit weakmap values, so we will not see cycles if they are behind a weakmap and not held live elsewhere. If the thing is not held live elsewhere, then I think it is probably not participating in cycles unless the thing is also the key of the weakmap elsewhere? It's hard to tell what the severity is here, so I am going to file as sec-high and suggest uplift to all branches.

Comment 1

[Andrew McCreight [:mccr8]]

This only affects weak map values that are not AddToCCKind(), but that hold onto CCed things. I don't know how common that is. Looking at the definition of TraceKind, it seems like only Symbol is something that could end up there, at least for content JS:

https://dxr.mozilla.org/mozilla-central/source/js/public/TraceKind.h?case=true&from=TraceKind#34

I guess debugger stuff uses C++ weak maps, and might put scripts or something in there.

Comment 2

[Terrence Cole [:terrence]]

After discussion on IRC: Value can only hold JSObject, JSString, and JS::Symbol, so there is no security issue here, just a performance issue.

Comment 3

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d563945a38a8c97d834d45434830db26f2065f61
Bug 1247381 - Restore a CC optimization dropped in bug 1105069 part 7; r=mccr8

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d563945a38a8