1445955 - Disable MemoryProtectionExceptionHandler function if any JSRuntime outlive the ProtectedRegionTree

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Nicolas B. Pierron [:nbp]]

In case of JSRuntime leaks (Bug 1445619), we might assert that we still have some registered content as of the pages of the JSRuntime might still be protected.

This bug is about weakening this assertion to only ensure that the tree of protected pages is empty iff we have no leaked JSRuntime.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Disable memory protection in case of leaked JSRuntime.

This patch adds an Atomic which is used to ensure if the tree of memory ranges
can be mutated. It is set/reset by the constructor (assuming we would only have
this static instance).

The Atomic is kept out-side the ProtectionPages class, as it indicates if the
content within the class is dirty or clean. (moving it inside should be fine in
practice, but semantically wrong)

Comment 2

[Emanuel Hoogeveen [:ehoogeveen]]

Comment on attachment 8959142 [details] [diff] [review]
Disable memory protection in case of leaked JSRuntime.

Review of attachment 8959142 [details] [diff] [review]:
-----------------------------------------------------------------

Annoying, but worth it to get this working with LifoAlloc.

::: js/src/ds/MemoryProtectionExceptionHandler.cpp
@@ +75,4 @@
>  
> +    ~ProtectedRegionTree() {
> +        // See Bug 1445619: Currently many users of the JS engine are leaking
> +        // the world, unfortunately LifoAllpoc owned by JSRuntimes have

Typo: s/LifoAllpoc/LifoAlloc/

@@ +194,5 @@
>              uintptr_t address = uintptr_t(ExceptionRecord->ExceptionInformation[1]);
>  
>              // If the faulting address is in one of our protected regions, we
>              // want to annotate the crash to make it stand out from the crowd.
> +            if (sProtectedRegionsInit && sProtectedRegions.isProtected(address)) {

Hmm, this doesn't seem strictly necessary but I guess it's better to avoid weird edge cases.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff6ecf81af7
Disable memory protection in case of leaked JSRuntime. r=ehoogeveen

Comment 4

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/0ff6ecf81af7