Closed
Bug 599761
Opened 14 years ago
Closed 14 years ago
TM: set right compartment in jsd.
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: gwagner, Assigned: jst)
References
Details
(Whiteboard: [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css][suspect-regress-dromaeo_jslib])
Attachments
(2 files, 1 obsolete file)
1.23 KB,
patch
|
Details | Diff | Splinter Review | |
11.37 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
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
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?
Comment 2•14 years ago
|
||
Wait, if scopeobj is null we end up in the default compartment.
Comment 3•14 years ago
|
||
The asserts are getting pretty obscure at this point. This might be the end of the tunnel.
Assignee: general → gal
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?
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.
Comment 6•14 years ago
|
||
Attachment #478707 -
Attachment is obsolete: true
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.
Comment 8•14 years ago
|
||
jst is looking at a related test case
Assignee | ||
Updated•14 years ago
|
Summary: TM: set right compartment in _newJSDContext → TM: set right compartment in jsd.
Assignee | ||
Updated•14 years ago
|
Assignee: gal → jst
Assignee | ||
Comment 9•14 years ago
|
||
I believe with this change we pass mochitest as far as jsd usage goes.
Attachment #479234 -
Flags: review?(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).
Attachment #479234 -
Flags: review?(gal) → review+
Assignee | ||
Comment 11•14 years ago
|
||
Fixed in tracemonkey.
http://hg.mozilla.org/tracemonkey/rev/571523d28b41
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-tracemonkey]
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
Comment 14•14 years ago
|
||
Looking.
Comment 15•14 years ago
|
||
Yeah, we forgot that part.
Comment 16•14 years ago
|
||
Gregor, want to transplant that into a new bug and land it if it tryservers? Thanks for catching this.
Updated•14 years ago
|
Whiteboard: [fixed-in-tracemonkey] → [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med]
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.
Comment 18•14 years ago
|
||
The bot seems to be marking every changeset in the m-c merge.
Updated•14 years ago
|
Whiteboard: [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med] → [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css]
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.
Updated•14 years ago
|
Whiteboard: [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css] → [fixed-in-tracemonkey][suspect-regress-ts_cold_generated_med][suspect-regress-dromaeo_css][suspect-regress-dromaeo_jslib]
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.
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.
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.
You need to log in
before you can comment on or make changes to this bug.
Description
•