805421 - crash in js::ion::Cannon

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Martijn Wargers (dead)]

This bug was filed from the Socorro interface and is 
report bp-0aebea27-d8ea-4bf2-a56c-d6fee2121025 .
============================================================= 
0 		@0x5cdab000 	
1 	libxul.so 	js::ion::Cannon 	Ion.cpp:1370
2 	libxul.so 	js::RunScript 	jsinterp.cpp:301
3 	libxul.so 	js::InvokeGetterOrSetter 	jsinterp.cpp:379
4 	libxul.so 	js::GetPropertyHelper 	jsscopeinlines.h:295
5 	libxul.so 	js::ion::GetPropertyCache 	IonCaches.cpp:764
6 		@0x5c81a652

Comment 1

[Martijn Wargers (dead)]

This happens for in current trunk build on the Samsung Galaxy Nexus, just by loading http://people.mozilla.org/~mwargers/tests/layout/Morphing%20Clock.htm

Comment 2

[Martijn Wargers (dead)]

Seems to be wfm in current trunk build.

Comment 3

[Scoobidiver (away)]

More reports at:
https://crash-stats.mozilla.com/report/list?signature=js%3A%3Aion%3A%3ACannon%28JSContext*%2C+js%3A%3AStackFrame*%29

Comment 4

[Marty Rosenberg [:mjrosenb]]

just looking at the backtrace, and the address that we crash at, I think I know what is wrong here.  Patch should be coming soon.

Comment 5

[Marty Rosenberg [:mjrosenb]]

Attachment: /home/mrosenberg/patches/moreFlushes-r0.patch

I discovered two more ways that we can execute code without going through a flusher.  This fixes both of them.  It is pretty simple, since the number of transitions from interpreter -> Ion is small

I should also apply a similar fix in the code that transitions to JM, since we can have the exact same issue on an Interp -> JM -> ion sequence

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 685547 [details] [diff] [review]
/home/mrosenberg/patches/moreFlushes-r0.patch

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

::: js/src/ion/Ion.cpp
@@ +1450,5 @@
>          JSAutoResolveFlags rf(cx, RESOLVE_INFER);
> +        AutoFlushCache *afc = cx->compartment->ionCompartment()->flusher();
> +        cx->compartment->ionCompartment()->setFlusher(NULL);
> +        if (afc)
> +            afc->flushAnyway();

Do you also need this pattern around EnterMethodJIT, which can enter Ion code? Or is it only EnterIon?

(If it's needed at both places, I'd recommend folding this logic into an AutoForceFlushCache object.)

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/99c2b45b6668