781214 - IonMonkey: Assertion failure: !cx->compartment->activeAnalysis, at jsinterp.cpp:334 or Crash [@ js::ion::IonCompile]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on ionmonkey revision 15db586f9a12 (run with --ion -n -m --ion-eager -a):


p = Proxy.create({
  has: function() {}
})
Object.prototype.__proto__ = p
function testIn() {
    var obj = { "-1": 5, "1.7": 3, "foo": 5, "1": 7 };
}
testIn()

Comment 1

[Christian Holler (:decoder)]

Valgrind shows several errors here that look like this:


==18507== Invalid read of size 4
==18507==    at 0x80D9A12: js::types::TypeSet::getKnownTypeTag(JSContext*) (jsinfer.cpp:1470)
==18507==    by 0x8394E82: js::ion::TypeInferenceOracle::elementWriteIsDenseArray(JSScript*, unsigned char*) (TypeOracle.cpp:389)
==18507==    by 0x834A1FB: js::ion::IonBuilder::jsop_initelem() (IonBuilder.cpp:3902)
==18507==    by 0x8354508: js::ion::IonBuilder::inspectOpcode(JSOp) (IonBuilder.cpp:909)
==18507==    by 0x8354AAC: js::ion::IonBuilder::traverseBytecode() (IonBuilder.cpp:663)
==18507==    by 0x8356722: js::ion::IonBuilder::build() (IonBuilder.cpp:344)
==18507==    by 0x83306A5: js::ion::BuildMIR(js::ion::IonBuilder&, js::ion::MIRGraph&) (Ion.cpp:691)
==18507==    by 0x833558A: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Ion.cpp:833)
==18507==    by 0x8335C0B: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:986)
==18507==    by 0x80F6EC2: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2491)
==18507==    by 0x8CFA770: ???
==18507==  Address 0x8d2cd70 is 16 bytes before a block of size 64 alloc'd
==18507==    at 0x779C82F: malloc (vg_replace_malloc.c:236)
==18507==    by 0x80CE1C6: js::Vector<js::types::CompilerOutput, 0u, js::TempAllocPolicy>::growStorageBy(unsigned int) (Utility.h:157)
==18507==    by 0x83356D4: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Vector.h:772)
==18507==    by 0x8335C0B: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:986)
==18507==    by 0x80F6EC2: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2491)
==18507==    by 0x8CFA770: ???
==18507== 
==18507== Invalid read of size 4
==18507==    at 0x83355BC: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (jsscript.h:627)
==18507==    by 0x8335C0B: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:986)
==18507==    by 0x80F6EC2: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2491)
==18507==    by 0x8CFA770: ???
==18507==  Address 0x8d2cd70 is 16 bytes before a block of size 64 alloc'd
==18507==    at 0x779C82F: malloc (vg_replace_malloc.c:236)
==18507==    by 0x80CE1C6: js::Vector<js::types::CompilerOutput, 0u, js::TempAllocPolicy>::growStorageBy(unsigned int) (Utility.h:157)
==18507==    by 0x83356D4: bool js::ion::IonCompile<&(js::ion::TestCompiler(js::ion::IonBuilder&, js::ion::MIRGraph&))>(JSContext*, JSScript*, JSFunction*, unsigned char*, bool) (Vector.h:772)
==18507==    by 0x8335C0B: js::ion::CanEnter(JSContext*, JSScript*, js::StackFrame*, bool) (Ion.cpp:986)
==18507==    by 0x80F6EC2: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2491)
==18507==    by 0x8CFA770: ???


Memory corruption => S-s

Comment 2

[Kannan Vijayan [:djvj]]

Attachment: Fix.

The issue here was that IonBuilder, when compiling jsop_initprop, called |LookupPropertyWithFlags| on the template object, which can call |lookupGeneric| to do its work.

In this test case, a proxy object is installed as the base __proto__ for all regularly constructed objects.  So the templateObject lookup hits a proxy with a |has| trap, and that runs into an ASSERT on InvokeKernel about not being called in the middle of analysis.

Other parts of the IonBuilder which call lookupGeneric ensure that the object they're calling it on is native and has no hooks lookup hooks installed (on either it or one of its prototypes).

Fix just refactors that check into a separate, cumbersomely named function, and adds the check to the jsop_initprop compiler.

Comment 3

[Kannan Vijayan [:djvj]]

Attachment: Fix with review.

See previous comment.  Same patch.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 650280 [details] [diff] [review]
Fix with review.

djvj meant to give this review to mjrosenb, which previously failed because mjrosenb was not on cc. Fixed.

Comment 5

[Marty Rosenberg [:mjrosenb]]

Comment on attachment 650280 [details] [diff] [review]
Fix with review.

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

::: js/src/ion/IonBuilder.cpp
@@ +3939,5 @@
>  
> +static bool
> +CanEffectlesslyCallLookupGenericOnObject(JSObject *obj)
> +{
> +    JSObject *pobj = obj;

minor nit, you don't need a temp here (what does the 'p' stand for?)

@@ -4277,1 @@
>  

ahh, it was copied verbatim from here :)

Comment 6

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/projects/ionmonkey/rev/d142f2087917
https://hg.mozilla.org/projects/ionmonkey/rev/723b214b3512 (fix nit)

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.