Perma runner.py | application crashed [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()]

RESOLVED FIXED in Firefox 68

Status

()

defect
P5
critical
RESOLVED FIXED
3 months ago
2 months ago

People

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

Tracking

(Regression, {crash, regression})

unspecified
mozilla68
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox67 unaffected, firefox68 fixed)

Details

(Whiteboard: [stockwell fixed:patch], crash signature)

Attachments

(7 attachments)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review

#[markdown(off)]
Filed by: nbeleuzu [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=243094038&repo=mozilla-central

https://queue.taskcluster.net/v1/task/OISyivfzT9ypajCBESmmsw/runs/0/artifacts/public/logs/live_backing.log

12:24:09 INFO - raptor-control-server shutting down browser (pid: 5220)
12:24:09 INFO - raptor-control-server received webext_status: Removed tab: 2
12:24:25 INFO - mozcrash Downloading symbols from: https://queue.taskcluster.net/v1/task/VRdZAaqNSc-ils686f6Qfg/artifacts/public/build/target.crashreporter-symbols.zip
12:24:28 INFO - mozcrash Copy/paste: C:\Users\task_1556366894\build\win32-minidump_stackwalk.exe c:\users\task_1556366894\appdata\local\temp\tmpyrcm87.mozrunner\minidumps\bc4c63a8-fd98-4968-9d1d-dfd93a25f2d6.dmp c:\users\task_1556366894\appdata\local\temp\tmpwr9tha
12:24:32 INFO - mozcrash Saved minidump as C:\Users\task_1556366894\build\blobber_upload_dir\bc4c63a8-fd98-4968-9d1d-dfd93a25f2d6.dmp
12:24:32 INFO - mozcrash Saved app info as C:\Users\task_1556366894\build\blobber_upload_dir\bc4c63a8-fd98-4968-9d1d-dfd93a25f2d6.extra
12:24:32 INFO - PROCESS-CRASH | runner.py | application crashed [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()]
12:24:32 INFO - Crash dump filename: c:\users\task_1556366894\appdata\local\temp\tmpyrcm87.mozrunner\minidumps\bc4c63a8-fd98-4968-9d1d-dfd93a25f2d6.dmp
12:24:32 INFO - Operating system: Windows NT
12:24:32 INFO - 10.0.15063
12:24:32 INFO - CPU: amd64
12:24:32 INFO - family 6 model 94 stepping 3
12:24:32 INFO - 8 CPUs
12:24:32 INFO - GPU: UNKNOWN
12:24:32 INFO - Crash reason: EXCEPTION_BREAKPOINT
12:24:32 INFO - Crash address: 0x7ff90f93fa9b
12:24:32 INFO - Assertion: Unknown assertion type 0x00000000
12:24:32 INFO - Process uptime: 38 seconds
12:24:32 INFO - Thread 7 (crashed)
12:24:32 INFO - 0 xul.dll!class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()() [JSScript.cpp:7bdb46cc5ec6221c8df10f0a239dc4faf457488d : 0 + 0x67]
12:24:32 INFO - rax = 0x00007ff91119a199 rdx = 0x0000020c1aa2a340
12:24:32 INFO - rcx = 0x00007ff9289b4aa8 rbx = 0x0000020c19400005
12:24:32 INFO - rsi = 0x0000000000087b1e rdi = 0x0000020c19487b23
12:24:32 INFO - rbp = 0x0000006a6b5fed80 rsp = 0x0000006a6b5fed30
12:24:32 INFO - r8 = 0x0000000021ab218c r9 = 0x0000020c1aa7c000
12:24:32 INFO - r10 = 0x0000000000001b7a r11 = 0x0000020c15b017d8
12:24:32 INFO - r12 = 0x0000006a6b5fee18 r13 = 0x0000006a6b5feed0
12:24:32 INFO - r14 = 0x000000000000003c r15 = 0x00000000000879e4
12:24:32 INFO - rip = 0x00007ff90f93fa9b
12:24:32 INFO - Found by: given as instruction pointer in context

Duplicate of this bug: 1547527
Crash Signature: [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()] → [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()] [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::XDRScript<js::XDR_DECODE>(class js::XDRState<js::XDR_DECODE> *,…

Yeah, it would be. Will look tomorrow.

Crash Signature: [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()] [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::XDRScript<js::XDR_DECODE>(class js::XDRState<js::XDR_DECODE> *,… → [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::ScriptSource::xdrData<js::XDR_DECODE>::<unnamed-tag>::operator()()] [@ class mozilla::Result<mozilla::Ok,JS::TranscodeResult> js::XDRScript<js::XDR_DECODE>(class js::XDRState<js::XDR_DECODE> *…

Great, thank you!

That stack doesn't give a useful line number for the crashing code, possibly because it's in a generic lambda inside a template. I'm going to split those lambdas out into static class functions to see if I can get something better from that. (Incidentally, I was already somewhat considering making that change to avoid the "pass a value of the right type purely for generic-overload-selection purposes" thing done now, so this is not actually fuglifying.)

thanks for looking at the :waldo, ping :rwood or :davehunt if this is work needed in the raptor harness or the binast test. It is odd that this binast test is failing- hopefully it is fixable soon.

I think the issue here is I used the wrong sense in an mode == XDR_ENCODE test in the BinAST XDR code. Testing out the one-line patch now (atop patches to move all the lambdas to separate functions)...

Flags: needinfo?(jwalden)

Mm, https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac2a125ecaa47182e0c6078d2b1651be46c5e453&selectedJob=243420699 suggests that final patch is not actually the fix -- although it look right in its own right to me. (Maybe it's just not a complete fix? ¯_(ツ)_/¯ ) Looking into that further, tho unfortunately it looks like we XDR wrong and then the actual error happens when we use that XDR stuff much further down the line...

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0494a9f3230c
Make the |CodeUncompressedData| generic lambda inside |ScriptSource::xdrData| instead be a static member function in |ScriptSource|, avoiding the need to pass an ignored |Unit| argument to it purely for overload selection.  r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fe6df92def0
Make the |CodeCompressedData| generic lambda inside |ScriptSource::xdrData| instead be a static member function in |ScriptSource|, avoiding the need to pass an ignored |Unit| argument to it purely for overload selection.  r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/b96f067ff117
Make the |CodeBinASTData| generic lambda inside |ScriptSource::xdrData| instead be a static member function in |ScriptSource|, consistent with the other code-particular-type-of-data functions.  r=tcampbell

Okay, I audited the changes line by line, and I think there are two problems here, and the failure here could be either one, but if you don't fix both you will have problems.

First, the ternary assigned to sourceBase in the BinAST XDR code has its arms flipped -- trying to read from freshly-allocated metadata and write into already-installed (in ss->binASTMetadata_) metadata. The latter is going to write into oddspace. The former is going to read a bunch of nulls. It's not clear to me that this definitely explains a crash.

Second, the UniquePtr used to hold a BinASTSourceMetadata when filling in a ScriptSource needs to be rooted against GCs during the process of filling in the metadata -- because I changed it from creating and immediately installing the metadata, to installing it last as a sort of flag-day on the meaning of ss->data and ss->binASTMetadata_. So when it's created but before it's installed, it needs to be known to the GC. This involved two things: first, freshMetadata has to be a Rooted, and second, it has to be passed as MutableHandle to ss->initializeBinAST.

I am not certain these are the causes of the problem here, but I'm 60-70% confident. We'll see:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a395d00bfcbedface9dc9106a130f23c7381e26d&selectedJob=243516401

If those are it, I'll post them for review either tonight or tomorrow. If these aren't it, well, not sure what at that point. Maybe worth bringing in fresh eyes then, since running this one weird test locally hasn't worked for me yet.

Assignee: nobody → jwalden
Component: Raptor → JavaScript Engine
Product: Testing → Core
Version: Version 3 → unspecified
Duplicate of this bug: 1547490
Crash Signature: ,JS::TranscodeResult> js::XDRScript<js::XDR_DECODE>(class js::XDRState<js::XDR_DECODE> *, class JS::Handle<js::Scope *>, class JS::Handle<js::ScriptSourceObject *>, class JS::Handle<JSFunction *>, class JS::MutableHandle<JSScript *>)] → ,JS::TranscodeResult> js::XDRScript<js::XDR_DECODE>(class js::XDRState<js::XDR_DECODE> *, class JS::Handle<js::Scope *>, class JS::Handle<js::ScriptSourceObject *>, class JS::Handle<JSFunction *>, class JS::MutableHandle<JSScript *>)] [@ mozilla::Result<m…
Crash Signature: , class JS::MutableHandle<JSScript *>)] [@ mozilla::Result<mozilla::Ok, JS::TranscodeResult> js::ScriptSource::xdrData<(js::XDRMode)1>(js::XDRState<(js::XDRMode)1>*, js::ScriptSource*)::{lambda()#1}::operator()() const] [@ mozilla::Result<mozilla::Ok → , class JS::MutableHandle<JSScript *>)] [@ mozilla::Result<mozilla::Ok, JS::TranscodeResult> js::ScriptSource::xdrData<(js::XDRMode)1>(js::XDRState<(js::XDRMode)1>*, js::ScriptSource*)::{lambda()#1}::operator()() const] [@ mozilla::Result<mozilla::Ok
Keywords: leave-open

(In reply to Jeff Walden [:Waldo] from comment #16)

I am not certain these are the causes of the problem here, but I'm 60-70% confident. We'll see:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a395d00bfcbedface9dc9106a130f23c7381e26d&selectedJob=243516401

Woop woop, two greens are here! Will post the remaining patches now, just in case undeserved magic ends up appearing before I'd have posted it tomorrow morning. (But there really is no rush, I've got other distractions to occupy me...)

Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/116b2213c426
Whoops, stylemageddon style patrol nitfixes.  r=me, DONTBUILD
Attachment #9061495 - Attachment description: Bug 1547478 - Flip a a |mode == XDR_ENCODE| to |mode == XDR_DECODE| so that BinAST source data is properly XDR'd. r=tcampbell → Bug 1547478 - Flip the sense of a |mode == XDR_ENCODE| so that BinAST source data is properly XDR'd. r=tcampbell

(In reply to Jeff Walden [:Waldo] from comment #18)

Woop woop, two greens are here! Will post the remaining patches now, just in case undeserved magic ends up appearing before I'd have posted it tomorrow morning. (But there really is no rush, I've got other distractions to occupy me...)

...spoke too soon, the first two greens were followed by a succession of orange. Hrm.

Okay, I understand this now.

The previous code would allocate bytes of BinAST data, then initialize a ScriptSource as BinAST containing those bytes. Such initialization includes a deduplication step, so the bytes you hand in are not necessarily the bytes in the ultimate ScriptSource.

The previous code then would walk through the metadata serializing atoms and CharSlices. The former don't require special handling other than making sure the GC always knows about them -- which bug 1544882 neglected to do, but which I have fixed in the one diff posted here for review. The latter, however...they contain pointers into the bytes of BinAST data.

Before, we would update with pointers into the deduplicated data. But with the current landed changes, we update with pointers into un-deduplicated data. So if deduplication happens and the bytes you pass in are not the shared-immutable bytes actually stored in the ScriptSource, hilarity will ensue.

So basically what's now called ScriptSource::initializeBinAST is going to have to take not a UniquePtr<char[], JS::FreePolicy> bytes, but a deduplicated SharedImmutableString (or maybe a reference to one?) instead.

Eeeeeeugh.

I'm out of time today. Patch tomorrow.

Incidentally, https://treeherder.mozilla.org/#/jobs?repo=try&revision=5802d5415adfab60022c977bb9768d898f38a8d3 is these patches showing up green. (Or at least as green as anything ever is.)

Crash Signature: , JS::TranscodeResult> js::XDRScript<(js::XDRMode)1>(js::XDRState<(js::XDRMode)1>*, JS::Handle<js::Scope*>, JS::Handle<js::ScriptSourceObject*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>)] → , JS::TranscodeResult> js::XDRScript<(js::XDRMode)1>(js::XDRState<(js::XDRMode)1>*, JS::Handle<js::Scope*>, JS::Handle<js::ScriptSourceObject*>, JS::Handle<JSFunction*>, JS::MutableHandle<JSScript*>)] [@ mozilla::Result<mozilla::Ok, JS::TranscodeResult> …
Pushed by jwalden@mit.edu:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e8514d344b
Root fresh BinAST metadata created during XDR decoding until it's transferred into its ScriptSource.  r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/878d133f6a4c
Add more assertions verifying that ScriptSources contain expected types of data during the XDR process.  r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc57aed491f5
Flip the sense of a |mode == XDR_ENCODE| so that BinAST source data is properly XDR'd.  r=tcampbell

Sure, without the last patch here waiting for review that's more or less expected.

Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Whiteboard: [stockwell disable-recommended] → [stockwell needswork:owner]
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/99ad8ab7c1fd
XDR BinAST metadata using deduplicated BinAST data so that pointers in the metadata will correctly point into the deduplicated data, not into user-provided data that hasn't been deduplicated yet.  r=tcampbell
Whiteboard: [stockwell disable-recommended] → [stockwell fixed:patch]
Status: NEW → RESOLVED
Closed: 3 months ago
Keywords: leave-open
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.