IonMonkey: We Invalidate before we Invalidate

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mjrosenb, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

6 years ago
Created attachment 603993 [details]
bits and pieces of the v8 benchmarking suite

A backtrace on ARM, since that's what I observed this segfault on:

#0  js::ion::IonActivationIterator::IonActivationIterator (this=0xbeb80220, cx=0x7bff00) at /home/mrosenberg/src/ionmonkey/ionmonkey-build/js/src/ion/IonFrames.cpp:286
#1  0x0048f4e4 in js::ion::Invalidate (cx=0x7bff00, invalid=..., resetUses=true) at /home/mrosenberg/src/ionmonkey/ionmonkey-build/js/src/ion/Ion.cpp:1121
#2  0x001427a8 in js::types::TypeCompartment::processPendingRecompiles (this=0x7c0754, cx=0x7bff00) at /home/mrosenberg/src/ionmonkey/ionmonkey-build/js/src/jsinfer.cpp:2148
#3  0x00098e04 in js::types::AutoEnterTypeInference::~AutoEnterTypeInference (this=0xbeb802d4, __in_chrg=<optimized out>) at ../../src/jsinferinlines.h:235
#4  0x00149130 in js::types::TypeMonitorResult (cx=0x7bff00, script=0x40b0f550, pc=0x7e4ba4 "5", rval=...) at /home/mrosenberg/src/ionmonkey/ionmonkey-build/js/src/jsinfer.cpp:5170
#5  0x00193f5c in js::types::TypeScript::Monitor (cx=0x7bff00, script=0x40b0f550, pc=0x7e4ba4 "5", rval=...) at ../../src/jsinferinlines.h:575
#6  0x005e9998 in js::ion::InvalidationBailout (sp=0xbeb80388, frameSizeOut=0xbeb80380) at /home/mrosenberg/src/ionmonkey/ionmonkey-build/js/src/ion/Bailouts.cpp:461
#7  0x408982a0 in ?? ()

since we attempt to walk the stack from within InvalidationBailout, ionTop hasn't been set, and we end up reading garbage values.
dvander says that we simply shouldn't call invalidate from within InvalidationBailout, which sounds reasonable.
(Reporter)

Comment 1

6 years ago
Created attachment 603996 [details] [diff] [review]
/home/mrosenberg/patches/bailoutIssues-r1.patch
Attachment #603996 - Flags: review?(jdemooij)
Comment on attachment 603996 [details] [diff] [review]
/home/mrosenberg/patches/bailoutIssues-r1.patch

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

::: js/src/ion/Bailouts.cpp
@@ +463,5 @@
> +        // monitor the result, since the bailout happens before the MMonitorTypes
> +        // instruction is executed.
> +        jsbytecode *pc = activation->bailout()->bailoutPc();
> +        if (js_CodeSpec[*pc].format & JOF_TYPESET)
> +            return BAILOUT_RETURN_MONITOR;

Nit: add a JS_ASSERT(retval == BAILOUT_RETURN_OK); before the return, so that if "retval" can have another value in the future, we won't ignore it.
Attachment #603996 - Flags: review?(jdemooij) → review+

Updated

6 years ago
Duplicate of this bug: 732850
Duplicate of this bug: 732854
(Reporter)

Comment 5

6 years ago
landed: http://hg.mozilla.org/projects/ionmonkey/rev/0af68c0ec9a8
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.