Gregor Wagner [:gwagner] Reporter
	Description • 14 years ago

I found another defaultCompartment mixup in jsd.

2  libmozjs.so!JS_NewGlobalObject [jsapi.cpp:ef1f16975962 : 2942 + 0x31]
    eip = 0x0013efb1   esp = 0xbfd15f40   ebp = 0xbfd15f88   ebx = 0x004a568c
    Found by: call frame info
 3  libxul.so!_newJSDContext [jsd_high.c:ef1f16975962 : 144 + 0x17]
    eip = 0x020e0989   esp = 0xbfd15f90   ebp = 0xbfd15fb8   ebx = 0x02f06a60
    Found by: call frame info
 4  libxul.so!jsd_DebuggerOnForUser [jsd_high.c:ef1f16975962 : 209 + 0x1f]
    eip = 0x020e0c2e   esp = 0xbfd15fc0   ebp = 0xbfd15fe8   ebx = 0x02f06a60
    Found by: call frame info


http://tinderbox.mozilla.org/showlog.cgi?tree=MozillaTry&errorparser=unittest&logfile=1285526795.1285526948.15829.gz&buildtime=1285526795&buildname=Rev3%20Fedora%2012%20tryserver%20debug%20test%20mochitests-4%2f5&fulltext=1#err0

	Andreas Gal :gal
	Comment 1 • 14 years ago

    if( scopeobj )
        compartment = js_SwitchToObjectCompartment(jsdc->dumbContext, scopeobj);
    jsdc->glob = JS_NewGlobalObject(jsdc->dumbContext, &global_class);
    if( scopeobj )
        js_SwitchToCompartment(jsdc->dumbContext, compartment);

We are switching to the compartment right before that. Wth?

	Andreas Gal :gal
	Comment 2 • 14 years ago

Wait, if scopeobj is null we end up in the default compartment.

	Andreas Gal :gal
	Comment 3 • 14 years ago

The asserts are getting pretty obscure at this point. This might be the end of the tunnel.

	Gregor Wagner [:gwagner] Reporter
	Comment 4 • 14 years ago

I still get an assertion: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1285575562.1285575751.19920.gz&fulltext=1#err0

It's for the same test but at a different location. 
We assert now in JS_InitStandardClasses instead of JS_NewGlobalObject. Probably the next line in jsd_high.c. Missing JSAutoCrossCompartmentCall?

	Jason Orendorff [:jorendorff]
	Comment 5 • 14 years ago

I think making a separate compartment for JSD is a mistake. All the debugger stuff should be in the same compartment as the JSObject for the debugger service, which is chrome. Otherwise every object the service tries to return is actually from the debugger compartment.

Let me whip up a counterproposal here.

	Andreas Gal :gal
	Comment 7 • 14 years ago

Jason, want to file a bug to improve this? Until then a separate compartment seems the best way to make this stop crashing.

	Andreas Gal :gal
	Comment 8 • 14 years ago

jst is looking at a related test case

	Johnny Stenback (:jst) Assignee
	Comment 9 • 14 years ago

I believe with this change we pass mochitest as far as jsd usage goes.

	Andreas Gal :gal
	Comment 10 • 14 years ago

Comment on attachment 479234 [details] [diff] [review]
Fix jsd_val.c

We should be able to land this on TM (no need to push it into blake's pile).

	Johnny Stenback (:jst) Assignee
	Comment 11 • 14 years ago

Fixed in tracemonkey.

http://hg.mozilla.org/tracemonkey/rev/571523d28b41

	Robert Sayre
	Comment 12 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/571523d28b41

	Gregor Wagner [:gwagner] Reporter
	Comment 13 • 14 years ago

Do we still need Andreas' patch to land? I see cx->compartment == defaultCompartment in JS_NewGlobalObject with this patch.

http://tinderbox.mozilla.org/showlog.cgi?tree=MozillaTry&errorparser=unittest&logfile=1285737362.1285737801.6537.gz&buildtime=1285737362&buildname=WINNT%205.2%20tryserver%20debug%20test%20mochitests-4%2f5&fulltext=1#err0

	Andreas Gal :gal
	Comment 14 • 14 years ago

Looking.

	Andreas Gal :gal
	Comment 15 • 14 years ago

Yeah, we forgot that part.

	Andreas Gal :gal
	Comment 16 • 14 years ago

Gregor, want to transplant that into a new bug and land it if it tryservers? Thanks for catching this.

	Mozilla RelEng Bot
	Comment 17 • 14 years ago

A changeset from this bug was associated with a XP Ts, Cold MED Dirty Profile regression on Firefox. boo-urns :(

  Previous: avg 420.356 stddev 3.679 of 30 runs up to a9d1ad0bc386
  New     : avg 430.832 stddev 1.905 of 5 runs since a60414d076b5
  Change  : +10.475 (2.49% / z=2.847)
  Graph   : http://mzl.la/bZFaB3

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-ts_cold_generated_med] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

	Andreas Gal :gal
	Comment 18 • 14 years ago

The bot seems to be marking every changeset in the m-c merge.

	Mozilla RelEng Bot
	Comment 19 • 14 years ago

A changeset from this bug was associated with a Win7 Dromaeo (CSS) regression on Firefox. boo-urns :(

  Previous: avg 2014.419 stddev 40.480 of 30 runs up to a9d1ad0bc386
  New     : avg 1901.610 stddev 12.432 of 5 runs since a60414d076b5
  Change  : -112.809 (-5.6% / z=2.787)
  Graph   : http://mzl.la/9gFu4n

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

	Mozilla RelEng Bot
	Comment 20 • 14 years ago

A changeset from this bug was associated with a Win7 Dromaeo (jslib) regression on Firefox. boo-urns :(

  Previous: avg 127.610 stddev 4.222 of 30 runs up to a9d1ad0bc386
  New     : avg 116.384 stddev 0.751 of 5 runs since a60414d076b5
  Change  : -11.226 (-8.8% / z=2.659)
  Graph   : http://mzl.la/bZu5UP

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

	Mozilla RelEng Bot
	Comment 21 • 14 years ago

A changeset from this bug was associated with a XP Dromaeo (CSS) regression on Firefox. boo-urns :(

  Previous: avg 2045.275 stddev 49.676 of 30 runs up to a9d1ad0bc386
  New     : avg 1936.120 stddev 13.937 of 5 runs since a60414d076b5
  Change  : -109.155 (-5.34% / z=2.197)
  Graph   : http://mzl.la/b0dlou

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_css] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.

	Mozilla RelEng Bot
	Comment 22 • 14 years ago

A changeset from this bug was associated with a XP Dromaeo (jslib) regression on Firefox. boo-urns :(

  Previous: avg 129.703 stddev 4.099 of 30 runs up to a9d1ad0bc386
  New     : avg 117.954 stddev 0.660 of 5 runs since a60414d076b5
  Change  : -11.749 (-9.06% / z=2.866)
  Graph   : http://mzl.la/avZij4

The regression occurred from changesets in the following range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=a9d1ad0bc386&tochange=a60414d076b5

The tag [suspect-regress-dromaeo_jslib] has been added to the status whiteboard;
please remove it only once you have confirmed this bug is not the cause
of the regression.