Last Comment Bug 780288 - "Assertion failure: false (compartment mismatched)," or "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"
: "Assertion failure: false (compartment mismatched)," or "Assertion failure: J...
Status: VERIFIED FIXED
[shell only]
: assertion, regression, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows 7
: -- minor (vote)
: mozilla17
Assigned To: Luke Wagner [:luke]
: general
Mentors:
Depends on:
Blocks: jsfunfuzz
  Show dependency treegraph
 
Reported: 2012-08-03 14:40 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-09-17 01:23 PDT (History)
11 users (show)
gary: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
stack (12.49 KB, text/plain)
2012-08-03 16:19 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
stack (11.34 KB, text/plain)
2012-08-03 16:20 PDT, Gary Kwong [:gkw] [:nth10sd]
no flags Details
maybe fix (1.62 KB, patch)
2012-08-09 13:54 PDT, Luke Wagner [:luke]
wmccloskey: review+
gary: feedback+
Details | Diff | Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-08-03 14:40:06 PDT
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] 2012-08-03 14:43:10 PDT
Locking s-s because compartment mismatched bugs are usually bad.
Comment 2 Gary Kwong [:gkw] [:nth10sd] 2012-08-03 15:35:12 PDT
(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] 2012-08-03 15:58:32 PDT
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 (:billm) 2012-08-03 15:59:38 PDT
By the way, it's really useful to have a stack trace along with bugs like this.
Comment 5 Gary Kwong [:gkw] [:nth10sd] 2012-08-03 16:19:12 PDT
Created attachment 648894 [details]
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] 2012-08-03 16:20:14 PDT
Created attachment 648895 [details]
stack

Stack for "Assertion failure: JSVAL_IS_OBJECT_OR_NULL_IMPL(JSVAL_TO_IMPL(v)),"
Comment 7 Gary Kwong [:gkw] [:nth10sd] 2012-08-03 16:20:45 PDT
(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] 2012-08-03 16:33:19 PDT
These both seem related to the Help() shell function...
Comment 9 Luke Wagner [:luke] 2012-08-03 16:35:48 PDT
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] 2012-08-08 10:45:38 PDT
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] 2012-08-08 11:03:31 PDT
Oh, no, comment 9 is just commenting on two bugs in the shell function Help().
Comment 12 Andrew McCreight [:mccr8] 2012-08-08 11:08:23 PDT
I'm going to mark it sec-critical then due to the compartment mismatch.
Comment 13 Luke Wagner [:luke] 2012-08-09 13:46:12 PDT
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] 2012-08-09 13:54:39 PDT
Created attachment 650669 [details] [diff] [review]
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] 2012-08-09 15:05:52 PDT
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] 2012-08-09 16:00:18 PDT
Comment on attachment 650669 [details] [diff] [review]
maybe fix

(Assume I add the test-cases)
Comment 17 Luke Wagner [:luke] 2012-08-09 16:01:00 PDT
comment 15 confirms comment 13, so removing s-s.
Comment 19 Ed Morley [:emorley] 2012-08-10 01:58:05 PDT
https://hg.mozilla.org/mozilla-central/rev/bb4c98f406c0
Comment 20 Gary Kwong [:gkw] [:nth10sd] 2012-08-13 05:50:42 PDT
Testcases have been added to the testsuite -> VERIFIED.

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