926778 - Prevent executable allocator from handing out poisoned pointers with JSGC_ROOT_ANALYSIS

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steve Fink [:sfink] [:s:]]

I was getting disturbingly frequent but intermittent failures with rooting analysis, and thought I'd dig in a little.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Prevent executable allocator from handing out poisoned pointers with JSGC_ROOT_ANALYSIS

I know we're going to be ripping out the dynamic rooting analysis soon, but this gets rid of a lot of intermittent orange for me.

But certainly not all -- I have a lingering very-intermittent:

Assertion failure: masm.framePushed() == initialStack, at /home/sfink/src/MI-GC/js/src/jit/IonCaches.cpp:975

even with this patch applied.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 816972 [details] [diff] [review]
Prevent executable allocator from handing out poisoned pointers with JSGC_ROOT_ANALYSIS

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

r=me

::: js/src/assembler/jit/ExecutableAllocatorPosix.cpp
@@ +46,5 @@
> +    void* allocation;
> +#ifdef JSGC_ROOT_ANALYSIS
> +    do {
> +#endif
> +    allocation = mmap(NULL, n, INITIAL_PROTECTION_FLAGS, MAP_PRIVATE | MAP_ANON, VM_TAG_FOR_EXECUTABLEALLOCATOR_MEMORY, 0);

The indentation here looks wrong to me. I guess the style guide doesn't really cover this case, but I would suggest indenting this line. It doesn't really matter though.

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8a18721cdd8d