Open Bug 1458209 Opened 7 years ago Updated 4 years ago

Off-thread XDR decoding can report spurious OOMs

Categories

(Core :: JavaScript Engine, enhancement, P3)

Product:
Core
Component:
JavaScript Engine
Type:
enhancement

Tracking

()

Tracking Flags:
Tracking Status
firefox61 --- verified
firefox62 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- fix-optional
firefox66 --- affected

People

(Reporter: jandem, Unassigned)

References

(Blocks 1 open bug)

Details

See bug 1452114 and bug 1457475. If XDR decoding fails with one of the TranscodeResults here: https://searchfox.org/mozilla-central/rev/8837610b6c999451435695e800f38d4acbc0a644/js/src/jsapi.h#6018-6025 Then we report an OOM here (and in the multi-scripts-decode version): https://searchfox.org/mozilla-central/rev/08df4e6e11284186d477d7e5b0ae48483ecc979c/js/src/vm/HelperThreads.cpp#1669-1675 (That MOZ_DIAGNOSTIC_ASSERT is a MOZ_ASSERT now, bug 1457475.) It would be great if we could add a JS shell function to corrupt a cache entry's XDR version number or something, so we could write tests for this.
I can't needinfo nbp so I'll CC + assign.
Assignee: nobody → nicolas.b.pierron
Status: NEW → ASSIGNED
Priority: -- → P1
Assignee: nicolas.b.pierron → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(nicolas.b.pierron)
status-firefox61: --- → fix-optional
status-firefox62: --- → affected
Flags: needinfo?(nicolas.b.pierron)
Flags: needinfo?(nicolas.b.pierron)
Argh.
Assignee: nobody → jorendorff
Flags: needinfo?(nicolas.b.pierron) → needinfo?(jorendorff)
This code is odd: https://searchfox.org/mozilla-central/rev/1ce4e8a5601da8e744ca6eda69e782318afab54d/js/src/vm/HelperThreads.cpp#1859-1861 Raising errors doesn't do anything particularly useful... I guess when `parseTask->errors` has more than 1 element, we expect all but 0 or 1 of them to be warnings? Hmm.
I don't actually have time to work on this immediately. It's small but not that small. Next release, I hope.
status-firefox63: affected → fix-optional
status-firefox64: --- → affected
Wontfix for 64. Jason, given that this has had a few wontfixes, should we still be calling it P1? Or should we bring more help in for this issue?
status-firefox64: fix-optional → wontfix
status-firefox65: affected → fix-optional
status-firefox66: --- → affected

Yeah, this is P3. Backlog.

Priority: P1 → P3
Assignee: jorendorff → nobody
Blocks: stencil
Severity: normal → N/A
Flags: needinfo?(jorendorff)
Blocks: stencil-backlog
No longer blocks: stencil
You need to log in before you can comment on or make changes to this bug.