1618131 - Improve compacting GC assertions and poisoning

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P1)

Description

[Jon Coppeard (:jonco)]

These patches were originally written for bug 1600895.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1618131 - Assert that compacting GC only ever updates unmoved GC things or new copies of moved things and never the old copy r=jandem

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1618131 - Poison moved GC thing contents sooner r=jandem

This makes sure we will detect use of the contents of moved GC things during the update phase of the GC, not just when we return to the mutator as previously. Annoyingly we need to preserve contents for native objects with fixed elements because the elements flags are stored there and these may be accessed from other objects if they are COW elements.

This caught a use of unforwarded scripts during invalidation caused by OOM during sweeping type information.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/86e126a20395
Assert that compacting GC only ever updates unmoved GC things or new copies of moved things and never the old copy r=jandem
https://hg.mozilla.org/integration/autoland/rev/e3c661a8798f
Poison moved GC thing contents sooner r=jandem

Comment 4

[Cristian Brindusan [:cbrindusan]]

Backed out 2 changesets (Bug 1618131) for linux build bustages at Poison.h.
https://hg.mozilla.org/integration/autoland/rev/2fb440de0e0c5d104296309fd1cf207ddff14aec

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=e3c661a8798f59ba94714a3795dbd4293e0bb624&selectedJob=290535374

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290535374&repo=autoland&lineNumber=64706

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9a9dd047d64
Assert that compacting GC only ever updates unmoved GC things or new copies of moved things and never the old copy r=jandem
https://hg.mozilla.org/integration/autoland/rev/ea0a05b19edb
Poison moved GC thing contents sooner r=jandem

Comment 6

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/d9a9dd047d64
https://hg.mozilla.org/mozilla-central/rev/ea0a05b19edb

Comment 7

[Jon Coppeard (:jonco)]

Comment on attachment 9129086 [details]
Bug 1618131 - Poison moved GC thing contents sooner r=jandem

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Required for bug 1618880. This is a possible GC crash / security vulnerability.
• User impact if declined: Possible GC crash / security vulnerability.
• Fix Landed on Version: 75
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small patch that adds some debug-only poisoning and forwards some GC pointers in a couple of places.
• String or UUID changes made by this patch: None

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9129086 [details]
Bug 1618131 - Poison moved GC thing contents sooner r=jandem

Prereq patch needed for bug 1618880. Approved for 68.7esr.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/b98350145456