780288 - "Assertion failure: false (compartment mismatched)," or "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

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

s = newGlobal()
try {
  evalcx("\
	Object.defineProperty(this,\"i\",{enumerable:true,get:function(){t}});\
	for each(y in this)true\
  ", s)
} catch (e) {}
try {
  evalcx("\
	for(z=0,(7).watch(\"\",eval);;g){\
	  if(z=1){({t:function(){}})\
	}\
	", s)
} catch (e) {}
try {
  evalcx("\
	Object.defineProperty(this,\"g2\",{get:function(){return this}});\
	g2.y()\
  ", s)
} catch (e) {}

asserts js debug shell on m-c changeset 030feb4315fc without any CLI arguments at Assertion failure: false (compartment mismatched), while a variant asserts at Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)) :


s = newGlobal()
try {
  evalcx("\
	Object.defineProperty(this,\"i\",{enumerable:true,get:function(){t}});\
	for each(y in this)true\
  ", s)
} catch (e) {}
try {
  evalcx("\
	for(z=0,(7).watch(\"\",eval);;g){\
	  if(z=1){({t:function(){}})\
	}\
	", s)
} catch (e) {}
try {
  evalcx("\
	Object.defineProperty(this,\"g2\",{get:function(){return this}});\
	g2.y(\"\")\
  ", s)
} catch (e) {}

Comment 1

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

Locking s-s because compartment mismatched bugs are usually bad.

Comment 2

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

(not sure which one is correct)

Regression window of the first testcase in comment 0:

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   97470:68c396f305f4
user:        Luke Wagner
date:        Wed Jun 20 08:57:29 2012 -0700
summary:     Bug 755186 - rm JS_NewGlobalObject (r=jorendorff)

Regression window of the second testcase in comment 0:

The first bad revision is:
changeset:   101160:a91040f69ea3
user:        Steve Fink
date:        Mon Jul 23 13:37:31 2012 -0700
summary:     Bug 777219 - Prepare SpiderMonkey for a fully rooted API; r=bhackett

Comment 3

[Luke Wagner [:luke]]

Does this reproduce for you on anything other than 32-bit windows?  I can't repro on 64-bit debug linux.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

By the way, it's really useful to have a stack trace along with bugs like this.

Comment 5

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

Attachment: stack

Stack for "Assertion failure: false (compartment mismatched),"

Sorry for missing out the stack, also repros in m-c changeset a7fadfbad932

Comment 6

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

Attachment: stack

Stack for "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"

Comment 7

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

(In reply to Luke Wagner [:luke] from comment #3)
> Does this reproduce for you on anything other than 32-bit windows?  I can't
> repro on 64-bit debug linux.

I don't think I can check this till next week.

Comment 8

[Luke Wagner [:luke]]

These both seem related to the Help() shell function...

Comment 9

[Luke Wagner [:luke]]

Oh, heh:
 - for comment 5, I suspect the problem is that JS_GetGlobalObject(cx) can be in a different compartment than cx, so we need to AutoEnterCompartment
 - for comment 7, I think we need to check JSVAL_IS_PRIMITIVE before calling JSVAL_TO_OBJECT

Comment 10

[Daniel Veditz [:dveditz]]

I'm going to guess at sec-moderate because comment 8 seems to say this specific testcase isn't a problem in release builds, but comment 9 is saying there might be a more generic issue underlying it.

Comment 11

[Luke Wagner [:luke]]

Oh, no, comment 9 is just commenting on two bugs in the shell function Help().

Comment 12

[Andrew McCreight [:mccr8]]

I'm going to mark it sec-critical then due to the compartment mismatch.

Comment 13

[Luke Wagner [:luke]]

The compartment mismatch is caused by the shell function, so I don't think it's s-s.  I'll just write a patch and see if I'm right; I was just eye-balling it in comment 9.

Comment 14

[Luke Wagner [:luke]]

Attachment: maybe fix

I can't repro the crash so I can't verify the assert, but this patch fixes the two flagrant bugs.  Gary: can you verify?

Comment 15

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

Comment on attachment 650669 [details] [diff] [review]
maybe fix

I verify that the 2 asserts are indeed fixed by this patch, thanks Luke!

Comment 16

[Luke Wagner [:luke]]

Comment on attachment 650669 [details] [diff] [review]
maybe fix

(Assume I add the test-cases)

Comment 17

[Luke Wagner [:luke]]

comment 15 confirms comment 13, so removing s-s.

Comment 18

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb4c98f406c0

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bb4c98f406c0

Comment 20

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

Testcases have been added to the testsuite -> VERIFIED.