1347486 - Crash in GetCacheIRExpectedInputType

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-09dc0cae-869b-4e85-800f-b6ffa2170315.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	GetCacheIRExpectedInputType 	js/src/jit/BaselineInspector.cpp:1118
1 	xul.dll 	js::jit::BaselineInspector::expectedPropertyAccessInputType(unsigned char*) 	js/src/jit/BaselineInspector.cpp:1147
2 	xul.dll 	js::jit::IonBuilder::maybeUnboxForPropertyAccess(js::jit::MDefinition*) 	js/src/jit/IonBuilder.cpp:9941
3 	xul.dll 	js::jit::IonBuilder::jsop_getprop(js::PropertyName*) 	js/src/jit/IonBuilder.cpp:9998
4 	xul.dll 	js::jit::IonBuilder::jsop_length() 	js/src/jit/IonBuilder.cpp:9186
5 	xul.dll 	js::jit::IonBuilder::inspectOpcode(JSOp) 	js/src/jit/IonBuilder.cpp:2134
6 	xul.dll 	js::jit::IonBuilder::traverseBytecode() 	js/src/jit/IonBuilder.cpp:1446
7 	xul.dll 	js::jit::IonBuilder::buildInline(js::jit::IonBuilder*, js::jit::MResumePoint*, js::jit::CallInfo&) 	js/src/jit/IonBuilder.cpp:1006
8 	xul.dll 	js::jit::IonBuilder::IonBuilder(JSContext*, js::jit::CompileCompartment*, js::jit::JitCompileOptions const&, js::jit::TempAllocator*, js::jit::MIRGraph*, js::CompilerConstraintList*, js::jit::BaselineInspector*, js::jit::CompileInfo*, js::jit::OptimizationInfo const*, js::jit::BaselineFrameInspector*, unsigned int, unsigned int) 	js/src/jit/IonBuilder.cpp:169
9 	xul.dll 	js::jit::IonBuilder::inlineScriptedCall(js::jit::CallInfo&, JSFunction*) 	js/src/jit/IonBuilder.cpp:3646
10 	mozglue.dll 	arena_dalloc_small 	memory/mozjemalloc/jemalloc.c:4589
11 	mozglue.dll 	je_free 	memory/mozjemalloc/jemalloc.c:6393
12 	xul.dll 	js::jit::ControlFlowGenerator::~ControlFlowGenerator() 	
13 	xul.dll 	GetOrCreateControlFlowGraph 	js/src/jit/IonBuilder.cpp:1365
14 	xul.dll 	js::jit::IonBuilder::initEnvironmentChain(js::jit::MDefinition*) 	js/src/jit/IonBuilder.cpp:1153
15 	xul.dll 	js::jit::IonBuilder::initEnvironmentChain(js::jit::MDefinition*) 	js/src/jit/IonBuilder.cpp:1180
16 	xul.dll 	js::detail::InlineTable<js::InlineMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, 24, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>::InlineEntry, js::InlineMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, 24, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>::Entry, js::HashMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy, 24>::InlineTable<js::InlineMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, 24, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>::InlineEntry, js::InlineMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, 24, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>::Entry, js::HashMap<JSAtom*, js::frontend::RecyclableAtomMapValueWrapper<unsigned int>, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy>, js::DefaultHasher<JSAtom*>, js::SystemAllocPolicy, 24>(js::SystemAllocPolicy) 	js/src/ds/InlineTable.h:108
17 	xul.dll 	js::jit::IonBuilder::buildInline(js::jit::IonBuilder*, js::jit::MResumePoint*, js::jit::CallInfo&) 	js/src/jit/IonBuilder.cpp:1006
18 	xul.dll 	js::jit::IonBuilder::IonBuilder(JSContext*, js::jit::CompileCompartment*, js::jit::JitCompileOptions const&, js::jit::TempAllocator*, js::jit::MIRGraph*, js::CompilerConstraintList*, js::jit::BaselineInspector*, js::jit::CompileInfo*, js::jit::OptimizationInfo const*, js::jit::BaselineFrameInspector*, unsigned int, unsigned int) 	js/src/jit/IonBuilder.cpp:169
19 	xul.dll 	js::jit::IonBuilder::inlineScriptedCall(js::jit::CallInfo&, JSFunction*) 	js/src/jit/IonBuilder.cpp:3646
20 	xul.dll 	mozilla::Vector<js::jit::CFGBlock, 4, js::jit::JitAllocPolicy>::convertToHeapStorage(unsigned int) 	obj-firefox/dist/include/mozilla/Vector.h:863
21 		@0x44a15b 	
22 	xul.dll 	js::NativeObject::addPropertyInternal(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, JS::Handle<jsid>, bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>), bool (*)(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>, JS::ObjectOpResult&), unsigned int, unsigned int, unsigned int, js::ShapeTable::Entry*, bool, js::AutoKeepShapeTables const&) 	js/src/vm/Shape.cpp:657

there are single occurrences of this signature before but its volume is rising on 53 now that this is beta. it's crashing in MOZ_CRASH("Unexpected instruction") that got added in bug 1267163 and apparently bug 1324521 took a stab at fixing this kind of crash already but it's still ongoing.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Patch

I haven't been able to reproduce this, but it's possible we're seeing GuardMagicValue there (this seems to be JSOP_LENGTH so arguments.length makes sense). Type information probably has to be exactly right for this to trigger.

I think we can remove the MOZ_CRASH and return MIRType::Value as it's just an optimization.

Comment 2

[Hannes Verschore [:h4writer]]

Comment on attachment 8849594 [details] [diff] [review]
Patch

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

Sad that we won't get information anymore if we forget to optimize it?
Should we put MOZ_CRASH in debug builds here and return MIRType_Value in opt builds?

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/47b5e2e4dce6
Replace MOZ_CRASH in GetCacheIRExpectedInputType with MOZ_ASSERT_UNREACHABLE. r=h4writer

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8849594 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1267163.
[User impact if declined]: Crashes.
[Is this code covered by automated tests?]: No, this likely depends on GC timing and type information, I haven't been able to reproduce this locally.
[Has the fix been verified in Nightly?]: It landed on m-i.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Safe fix, only affects a very unlikely corner case.
[String changes made/needed]: None.

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/47b5e2e4dce6

Comment 6

[Gerry Chang [:gchang]]

Comment on attachment 8849594 [details] [diff] [review]
Patch

Fix a crash. Aurora54+ & Beta53+.

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0d59b5163d27

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0ed8ba3fbaa1

Comment 9

[Julien Cristau [:jcristau]]

Comment on attachment 8849594 [details] [diff] [review]
Patch

this doesn't seem to affect 52.x nearly as much as 53.0b, but in the interest of avoiding a crash, esr52+

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr52/rev/15dbaf157058