Last Comment Bug 734022 - IonMonkey: We Invalidate before we Invalidate
: IonMonkey: We Invalidate before we Invalidate
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
: 732850 732854 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-08 01:34 PST by Marty Rosenberg [:mjrosenb]
Modified: 2012-03-16 16:12 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
bits and pieces of the v8 benchmarking suite (13.01 KB, application/javascript)
2012-03-08 01:34 PST, Marty Rosenberg [:mjrosenb]
no flags Details
/home/mrosenberg/patches/bailoutIssues-r1.patch (1.87 KB, patch)
2012-03-08 01:56 PST, Marty Rosenberg [:mjrosenb]
jdemooij: review+
Details | Diff | Splinter Review

Description Marty Rosenberg [:mjrosenb] 2012-03-08 01:34:58 PST
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.
Comment 1 Marty Rosenberg [:mjrosenb] 2012-03-08 01:56:52 PST
Created attachment 603996 [details] [diff] [review]
/home/mrosenberg/patches/bailoutIssues-r1.patch
Comment 2 Jan de Mooij [:jandem] 2012-03-08 02:19:43 PST
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.
Comment 3 Jan de Mooij [:jandem] 2012-03-08 07:14:02 PST
*** Bug 732850 has been marked as a duplicate of this bug. ***
Comment 4 David Anderson [:dvander] 2012-03-12 13:15:04 PDT
*** Bug 732854 has been marked as a duplicate of this bug. ***
Comment 5 Marty Rosenberg [:mjrosenb] 2012-03-16 16:12:57 PDT
landed: http://hg.mozilla.org/projects/ionmonkey/rev/0af68c0ec9a8

Note You need to log in before you can comment on or make changes to this bug.