Closed Bug 1658070 Opened 4 years ago Closed 4 years ago

ProxyObject expando slot is invalid after JSObject::swap

Categories

(Core :: JavaScript: GC, defect)

Product:
Core
Component:
JavaScript: GC
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
82 Branch
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox80 --- unaffected
firefox81 + fixed
firefox82 + fixed

People

(Reporter: intermittent-bug-filer, Assigned: mgaudet)

References

(Regression)

Details

(Keywords: crash, intermittent-failure, regression)

Crash Data

Attachments

(2 files)

Bug1658070Debug.patch
(deleted), patch
Details | Diff | Splinter Review
Bug 1658070 - Save and restore expando slot on proxies during JSObject::swap r?jonco
(deleted), text/x-phabricator-request
RyanVM
: approval-mozilla-beta+
Details

Filed by: apavel [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=312439839&repo=mozilla-beta
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/TUqaZWhFQFqq1l1k_oXDeQ/runs/0/artifacts/public/logs/live_backing.log
Reftest URL: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/TUqaZWhFQFqq1l1k_oXDeQ/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

[task 2020-08-07T21:52:33.960Z] 21:52:33 INFO - TEST-START | /webvtt/rendering/cues-with-video/processing-model/embedded_style_media_queries.html
[task 2020-08-07T21:52:33.961Z] 21:52:33 INFO - PID 2144 | 1596837153958 Marionette INFO Testing http://web-platform.test:8000/webvtt/rendering/cues-with-video/processing-model/embedded_style_media_queries.html == http://web-platform.test:8000/webvtt/rendering/cues-with-video/processing-model/embedded_style_media_queries-ref.html
[task 2020-08-07T21:52:34.078Z] 21:52:34 INFO - PID 2144 | 1596837154061 Marionette INFO [43] Emitted TestRendered event
[task 2020-08-07T21:52:34.609Z] 21:52:34 INFO - PID 2144 | A content process crashed and MOZ_CRASHREPORTER_SHUTDOWN is set, shutting down
[task 2020-08-07T21:52:35.103Z] 21:52:35 INFO - mozcrash Downloading symbols from: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/HC3zXi4sSIWfmeNHIPjZsw/artifacts/public/build/target.crashreporter-symbols.zip
[task 2020-08-07T21:52:35.232Z] 21:52:35 INFO - PID 2144 | 1596837155218 Marionette INFO Stopped listening on port 53562
[task 2020-08-07T21:52:39.192Z] 21:52:39 INFO - mozcrash Copy/paste: /Users/cltbld/tasks/task_1596832991/fetches/minidump_stackwalk/minidump_stackwalk /var/folders/f7/3h7xxhj96mv05tf1xp7gx_s0000017/T/tmpTlU49W/minidumps/162F7FD9-71B8-4844-BFE5-28C5AA8AE08F.dmp /var/folders/f7/3h7xxhj96mv05tf1xp7gx_s0000017/T/tmpJDRsGt
[task 2020-08-07T21:52:44.924Z] 21:52:44 INFO - mozcrash Saved minidump as /Users/cltbld/tasks/task_1596832991/build/blobber_upload_dir/162F7FD9-71B8-4844-BFE5-28C5AA8AE08F.dmp
[task 2020-08-07T21:52:44.924Z] 21:52:44 INFO - mozcrash Saved app info as /Users/cltbld/tasks/task_1596832991/build/blobber_upload_dir/162F7FD9-71B8-4844-BFE5-28C5AA8AE08F.extra
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - PROCESS-CRASH | /webvtt/rendering/cues-with-video/processing-model/embedded_style_media_queries.html | application crashed [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)]
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - Mozilla crash reason: MOZ_CRASH(no missing return)
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - Crash dump filename: /var/folders/f7/3h7xxhj96mv05tf1xp7gx_s0000017/T/tmpTlU49W/minidumps/162F7FD9-71B8-4844-BFE5-28C5AA8AE08F.dmp
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - Operating system: Mac OS X
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - 10.14.5 18F132
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - CPU: amd64
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - family 6 model 69 stepping 1
[task 2020-08-07T21:52:44.988Z] 21:52:44 INFO - 4 CPUs
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO -
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - GPU: UNKNOWN
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO -
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Crash reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Crash address: 0x0
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Process uptime: 11 seconds
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO -
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Thread 0 (crashed)
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - 0 XUL!bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*) [Marking.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 701 + 0x1a]
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rax = 0x000000010ddd9c07 rdx = 0x000000000001ff0f
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rcx = 0x000000010e927120 rbx = 0x000000011f36b020
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rsi = 0xfff80000ffffffff rdi = 0x00007ffee8db9930
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db9790
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - r8 = 0x000000011f8aca38 r9 = 0x000000003c768b54
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - r10 = 0x000017264dd2f5c0 r11 = 0xfffff61e67ad2528
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - r12 = 0x00002acd614e3760 r13 = 0x00002acd614e3760
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - r14 = 0xffffffffffffffff r15 = 0x00007ffee8db9930
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rip = 0x000000010b6199d6
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Found by: given as instruction pointer in context
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - 1 XUL!js::ProxyObject::trace(JSTracer*, JSObject*) [Proxy.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 720 + 0x12]
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db97f0
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - rip = 0x000000010b16fd22
[task 2020-08-07T21:52:44.989Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - 2 XUL + 0x40c1bd0
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db97f8
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rip = 0x000000010b16fbd0
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - 3 XUL!js::TenuringTracer::traceObject(JSObject*) [Marking.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 3347 + 0x5]
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db9830
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rip = 0x000000010b62d335
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - 4 XUL!js::gc::ArenaCellSet::trace(js::TenuringTracer&) [Marking.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 3273 + 0x40]
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db9890
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rip = 0x000000010b62cc93
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - 5 XUL!js::gc::StoreBuffer::WholeCellBuffer::trace(js::TenuringTracer&) [Marking.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 3308 + 0x8]
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db98e0
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - rip = 0x000000010b62cf38
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.990Z] 21:52:44 INFO - 6 XUL!js::Nursery::doCollection(JS::GCReason, js::gc::TenureCountCache&) [Nursery.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 1141 + 0x11]
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db9900
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rip = 0x000000010b634770
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - 7 XUL!mozilla::CycleCollectedJSRuntime::GCNurseryCollectionCallback(JSContext*, JS::GCNurseryProgress, JS::GCReason) [CycleCollectedJSRuntime.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 1097 + 0xb]
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db99a0
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rip = 0x0000000107463fdb
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - 8 XUL!nsTArray_Impl<RefPtr<nsIDocShell>, nsTArrayInfallibleAllocator>::~nsTArray_Impl() [nsTArray.h:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 993 + 0x3a]
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db99c0
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rip = 0x00000001086f9301
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - 9 libmozglue.dylib!free [malloc_decls.h:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 54 + 0x13d]
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db99e0
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - rip = 0x000000010e8c214d
[task 2020-08-07T21:52:44.991Z] 21:52:44 INFO - Found by: stack scanning
[task 2020-08-07T21:52:44.992Z] 21:52:44 INFO - 10 XUL!js::Nursery::collect(JSGCInvocationKind, JS::GCReason) [Nursery.cpp:6fa6624abc9cee1610dc4ececdd23115edcddd22 : 1012 + 0x8]
[task 2020-08-07T21:52:44.992Z] 21:52:44 INFO - rbp = 0xfffb000000000000 rsp = 0x00007ffee8db9a20
[task 2020-08-07T21:52:44.992Z] 21:52:44 INFO - rip = 0x000000010b634283
[task 2020-08-07T21:52:44.992Z] 21:52:44 INFO - Found by: stack scanning

Component: Audio/Video: Playback → JavaScript: GC

The timing suggested this might be caused by bug 1657122, but there's nothing in the backtrace that relates to that change.

The whole cell store buffer is use a lot by JIT code but I couldn't find any JIT changes in the relevant time window that look related either.

Looking at recent crash reports in bug 1658496 suggest this is happening when tracing the proxy's expando slot. This was added fairly recently in bug 1644160.

Matthew is it possible that this is related to private fields? Could this slot be getting corrupted somehow?

Flags: needinfo?(mgaudet)
See Also: → https://bugzilla.mozilla.org/show_bug.cgi?id=1660310
Attached patch Bug1658070Debug.patch (deleted) — — Splinter Review

I can confirm the slot is being corrupted. With the attached patch, running ./mach jit-test -G auto-regress/bug1538542-1.js we see a failure.

Investigating the failure shows the corruption happens as part of JSObject::swap. The expando slot is uninitialized memory after ProxyObject::initExternalValueArrayAfterSwap runs.

I tried writing a simple fix, where I'd stash the expando object and try to restore it. What confuses me though is how to handle the case where you're not swapping two proxies. i.e.

diff --git a/js/src/vm/JSObject.cpp b/js/src/vm/JSObject.cpp
--- a/js/src/vm/JSObject.cpp
+++ b/js/src/vm/JSObject.cpp
@@ -1739,6 +1739,10 @@ void JSObject::swap(JSContext* cx, Handl
       }
     }
 
+    RootedObject proxyExpandoA(
+        cx, proxyA ? proxyA->expando().toObjectOrNull() : nullptr);
+    RootedObject proxyExpandoB(
+        cx, proxyB ? proxyB->expando().toObjectOrNull() : nullptr);
     // Swap the main fields of the objects, whether they are native objects or
     // proxies.
     char tmp[sizeof(JSObject_Slots0)];
@@ -1771,6 +1775,14 @@ void JSObject::swap(JSContext* cx, Handl
         oomUnsafe.crash("initExternalValueArray");
       }
     }
+
+    // Swap proxy expando objects
+    if (a->is<ProxyObject>()) {
+      b->as<ProxyObject>().setExpando(proxyExpandoA);
+    }
+    if (b->is<ProxyObject>()) {
+      a->as<ProxyObject>().setExpando(proxyExpandoB);
+    }
   }
 
   // Swapping the contents of two objects invalidates type sets which contain

Doesn't work, because it assumes that a and b are either both proxies or not; which doesn't seem to hold for this test case,

Flags: needinfo?(mgaudet) → needinfo?(jcoppeard)
Blocks: private-fields
Regressed by: 1644160
Summary: Intermittent /webvtt/rendering/cues-with-video/processing-model/embedded_style_media_queries.html | application crashed [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)] → ProxyObject expando slot is invalid after JSObject::swap
Has Regression Range: --- → yes
Keywords: regression

I have a preliminary patch for this that simply assumes we never swap proxies with expandos, and ensures the slot is correctly initialized.

Assignee: nobody → mgaudet
Flags: needinfo?(jcoppeard)
Crash Signature: [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)] → [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)] [@ js::MapGCThingTyped<`lambda at /builds/worker/checkouts/gecko/js/src/gc/Marking.cpp:3093:43'>(JS::Value const&, js::TenuringTracer::traverse<JS::Value>::<unnamed-tag>&&)]
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/428a932226db Save and restore expando slot on proxies during JSObject::swap r=jonco

[Tracking Requested - why for this release]:

This is a potential crash in 81 caused by incorrectly initialized data.

Crash Signature: [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)] [@ js::MapGCThingTyped<`lambda at /builds/worker/checkouts/gecko/js/src/gc/Marking.cpp:3093:43'>(JS::Value const& → [@ bool js::gc::TraceEdgeInternal<JS::Value>(JSTracer*, JS::Value*, char const*)] [@ js::MapGCThingTyped<`lambda at /builds/worker/checkouts/gecko/js/src/gc/Marking.cpp:3093:43'>(JS::Value const&
status-firefox81: --- → affected
tracking-firefox81: --- → ?

FTR based on bug 1660965 the following got logged earlier today before the patch for this bug landed:

0  XUL!js::ReportBadValueTypeAndCrash(JS::Value const&) [Value.cpp:c6bb371adb375636bbe68b954cc2d87748afadba : 39 + 0x1b]
   rax = 0x0000000110dc9910   rdx = 0x0000000000000008
   rcx = 0x0000000110dc92d0   rbx = 0x000000012ffae020
   rsi = 0x000000000000002e   rdi = 0x0000000000000000
   rbp = 0xfffb000000000000   rsp = 0x00007ffee6a33740
    r8 = 0x00007ffee6a33498    r9 = 0x0000000000000010
   r10 = 0x00007ffee6a33710   r11 = 0xffff80022a39666e
   r12 = 0x00003c83bb2f3520   r13 = 0x0000000111f973d8
   r14 = 0x00007ffee6a33920   r15 = 0xffffffffffffffff
   rip = 0x000000010daf161b
   Found by: given as instruction pointer in context

Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314473906&repo=try&lineNumber=13907-13917

https://hg.mozilla.org/mozilla-central/rev/428a932226db

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox82: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(mgaudet)

Comment on attachment 9172794 [details]
Bug 1658070 - Save and restore expando slot on proxies during JSObject::swap r?jonco

Beta/Release Uplift Approval Request

  • User impact if declined: Intermittent crashes
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): A fairly simple fix converting a relatively clearly invalid initialization into one that is simple and corrected.
  • String changes made/needed:
Flags: needinfo?(mgaudet)
Attachment #9172794 - Flags: approval-mozilla-beta?

Comment on attachment 9172794 [details]
Bug 1658070 - Save and restore expando slot on proxies during JSObject::swap r?jonco

Approved for 81.0b5.

Attachment #9172794 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: