Closed Bug 734022 Opened 12 years ago Closed 12 years ago

IonMonkey: We Invalidate before we Invalidate

Categories

(Core :: JavaScript Engine, defect)

All
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mjrosenb, Unassigned)

References

Details

Attachments

(2 files)

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.
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+
landed: http://hg.mozilla.org/projects/ionmonkey/rev/0af68c0ec9a8
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.