Closed Bug 895774 Opened 12 years ago Closed 12 years ago

Permaorange fx-team devtools/debugger/test/browser_dbg_blackboxing-02.js | application crashed [@ js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*)]

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Version:
25 Branch
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla25
Tracking Flags:
Tracking Status
firefox24 --- fixed
firefox25 --- fixed

People

(Reporter: cbook, Unassigned)

References

(
URL
)

Details

(Keywords: crash, intermittent-failure)

Crash Data

Attachments

(1 file, 1 obsolete file)

fix and test
4.11 KB, patch
jandem
: review+
fitzgen
: feedback+
Details | Diff | Splinter Review
Ubuntu VM 12.04 fx-team opt test mochitest-browser-chrome on 2013-07-18 20:20:15 PDT for push c98c47341be6 slave: tst-linux32-ec2-360 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_blackboxing-02.js | Exited with code 11 during test run PROCESS-CRASH | chrome://mochitests/content/browser/browser/devtools/debugger/test/browser_dbg_blackboxing-02.js | application crashed [@ js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*)] 20:40:21 INFO - Crash dump filename: /tmp/tmpHhFX68/minidumps/223dc7e6-240d-c2c9-3199061b-4d5b7df9.dmp 20:40:21 INFO - Operating system: Linux 20:40:21 INFO - 0.0.0 Linux 3.2.0-23-generic-pae #36-Ubuntu SMP Tue Apr 10 22:19:09 UTC 2012 i686 20:40:21 INFO - CPU: x86 20:40:21 INFO - GenuineIntel family 6 model 45 stepping 7 20:40:21 INFO - 1 CPU 20:40:21 INFO - Crash reason: SIGSEGV 20:40:21 INFO - Crash address: 0x18 20:40:21 INFO - Thread 0 (crashed) 20:40:21 INFO - 0 libxul.so!js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*) [Interpreter.cpp:c98c47341be6 : 119 + 0x3] 20:40:21 INFO - eip = 0xb5db9d63 esp = 0xbfb21280 ebp = 0xbfb212a8 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xbfb212d7 edi = 0xbfb212d8 eax = 0x00000000 ecx = 0xaea5e780 20:40:21 INFO - edx = 0x9b52d980 efl = 0x00010246 20:40:21 INFO - Found by: given as instruction pointer in context 20:40:21 INFO - 1 libxul.so!js::ScriptFrameIter::computeThis() const [Interpreter-inl.h:c98c47341be6 : 53 + 0x15] 20:40:21 INFO - eip = 0xb5e0c03a esp = 0xbfb212b0 ebp = 0xbfb212e8 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xaea5e781 edi = 0x00000001 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 2 libxul.so!DebuggerFrame_getThis [Debugger.cpp:c98c47341be6 : 3822 + 0x7] 20:40:21 INFO - eip = 0xb5daf0a5 esp = 0xbfb212f0 ebp = 0xbfb21488 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xbfb21338 edi = 0xb724aa60 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 3 libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [jscntxtinlines.h:c98c47341be6 : 225 + 0x12] 20:40:21 INFO - eip = 0xb5dbfb5c esp = 0xbfb21490 ebp = 0xbfb216b8 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0x8ac0b5b0 edi = 0xbfb2170c 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 4 libxul.so!js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) [Interpreter.cpp:c98c47341be6 : 527 + 0x1e] 20:40:21 INFO - eip = 0xb5dcfe7b esp = 0xbfb216c0 ebp = 0xbfb21768 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xb724aa60 edi = 0x00000000 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 5 libxul.so!js::InvokeGetterOrSetter(JSContext*, JSObject*, JS::Value, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) [Interpreter.cpp:c98c47341be6 : 598 + 0x27] 20:40:21 INFO - eip = 0xb5dd0023 esp = 0xbfb21770 ebp = 0xbfb217b8 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xb724aa60 edi = 0xbfb21ae0 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 6 libxul.so!js::Shape::get(JSContext*, JS::Handle<JSObject*>, JSObject*, JSObject*, JS::MutableHandle<JS::Value>) [Shape-inl.h:c98c47341be6 : 261 + 0x28] 20:40:21 INFO - eip = 0xb5f07418 esp = 0xbfb217c0 ebp = 0xbfb21828 ebx = 0xb6e1e8bc 20:40:21 INFO - esi = 0xb724aa60 edi = 0xbfb21ae0 20:40:21 INFO - Found by: call frame info 20:40:21 INFO - 7 libxul.so!js::GetPropertyHelper(JSContext*, JS::Handle<JSObject*>, JS::Handle<int>, unsigned int, JS::MutableHandle<JS::Value>) [jsobj.cpp:c98c47341be6 : 4026 + 0x29]
Summary: Intermittent TEST-UNEXPECTED-FAIL | /devtools/debugger/test/browser_dbg_blackboxing-02.js | Exited with code 11 during test run |application crashed [@ js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*)] → Permaorange fx-team devtools/debugger/test/browser_dbg_blackboxing-02.js | application crashed [@ js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*)]
this might been cause by: -> https://hg.mozilla.org/integration/fx-team/rev/a58e072e2ae7 -> it then relanded again as 9c0275e6303e ->https://hg.mozilla.org/integration/fx-team/log/a4e8ace8a4a9/browser/devtools/debugger/test/browser_dbg_blackboxing-02.js Nick, could you take a look, since this is also now perma-organge on fx-team i close that tree
Flags: needinfo?(nfitzgerald)
This is probably a SpiderMonkey bug that Nick's otherwise benign front-end change seems to trigger. CCing Jim and Eddy who may be able to help figure this one out.
Potentially related to bug 880538?
(In reply to Dave Camp (:dcamp) from comment #6) Highly doubtful unless you've got any "use asm" in this test suite.
Also bug 894948 was the last one to touch the files containing the top stack frames in the crash dump.
I don't see how bug 894948 could cause this: it didn't land anything, effectively.
The original push which landed that test was all green: https://tbpl.mozilla.org/?tree=Fx-Team&rev=9c0275e6303e So it has to be something that landed after, right?
Flags: needinfo?(nfitzgerald)
How come the crash only happens in opt builds, while debug builds run fine? What do we do differently there?
cx->global() dereferences cx->compartment() so this would have been crashing already.
I can reproduce the crash in my old Mac, if anyone wants me to test anything.
Oh, so it could have been caused by bug 894948, after all: the backout apparently wasn't pushed with the original patch. RyanVM pushed it just now.
(In reply to Till Schneidereit [:till] from comment #17) > Oh, so it could have been caused by bug 894948, after all: the backout > apparently wasn't pushed with the original patch. RyanVM pushed it just now. (In reply to TinderboxPushlog Robot from comment #18) > RyanVM > https://tbpl.mozilla.org/php/getParsedLog.php?id=25498688&tree=Fx-Team > Rev5 MacOSX Mountain Lion 10.8 fx-team opt test mochitest-browser-chrome on > 2013-07-19 11:50:10 > revision: 20848adc9980 > slave: talos-mtnlion-r5-022 > > 12:01:32 WARNING - TEST-UNEXPECTED-FAIL | > chrome://mochitests/content/browser/browser/devtools/debugger/test/ > browser_dbg_blackboxing-02.js | Exited with code 1 during test run > 12:02:02 WARNING - PROCESS-CRASH | > chrome://mochitests/content/browser/browser/devtools/debugger/test/ > browser_dbg_blackboxing-02.js | application crashed [@ > js::BoxNonStrictThis(JSContext*, JS::MutableHandle<JS::Value>, bool*)] > 12:02:03 ERROR - Return code: 256 Guess not.
backed out my patch until we can figure out who the offender is: https://hg.mozilla.org/integration/fx-team/rev/24b00dfb20de
Bisecting pointed to bug 887334 as the culprit. I'm now bisecting through the many changesets in that bug to figure out which one it is.
The first bad revision is: https://hg.mozilla.org/integration/fx-team/rev/5427f4d376c4 Bug 887334 - Stop setting the compartment to defaultCompartmentObject_->compartment(). r=luke
bholley: I landed a patch (bug 877686) on fx-team that introduced a new test, the test results for the whole commit were green, then we merged m-c into fx-team, and this new test crashed the browser every time. Note that my patch doesn't touch any platform code, and is only some JS for the debugger frontend. After bisecting, your commit for bug 887334 was revealed to be the changeset that introduced this crashing behavior. Can you take a look at this? I can't land my patch until this is fixed. Thanks!
Flags: needinfo?(bobbyholley+bmo)
This only reproduces on opt builds, so I've been fighting a bit with garbled stacks. But the basic situation seems to be that we call DebuggerFrame_getThis, which enters a compartment here: http://mxr.mozilla.org/mozilla-central/source/js/src/vm/Debugger.cpp#3822 But that ends up invoking ScriptFrameIter::computeThis, which pull what seems to be a different cx out of data_, and passes it to ComputeThis: http://mxr.mozilla.org/mozilla-central/source/js/src/vm/Stack.cpp#1107 That cx has a null compartment_, presumably meaning that nobody ever entered a compartment (at least since the last call to saveFrameChain). In the old days, we'd have been in the default compartment for the cx. But now we crash when we try to call ->global(), since that assumes we're in a compartment. I don't know a ton about this ScriptFrameIter stuff, so it's hard for me to tell what's going wrong, and gdb-ing this stuff isn't productive given the opt-ness. Maybe luke can tell me more, or suggest somebody who knows this stuff better to dig in?
Flags: needinfo?(bobbyholley+bmo) → needinfo?(luke)
Given bug 868437, jandem might be able to help.
Flags: needinfo?(jdemooij)
Attached patch maybe the fix? (obsolete) — — Splinter Review
Ah hah, iiuc, this is might be a pre-existing cross-compartment bug: The ScriptFrameIter walks down a callstack where each frame can be in a different compartment, therefore no assumption can be made about the compartment of a frame (as canonically determined by its fp->scopeChain->compartment) and cx->compartment. Thus, it is a potential cross-compartment violation for ScriptFrameIter::computeThis to call ComputeThis without having AutoCompartment'd fp->scopeChain (in particular this could allocate objects (via js_PrimitiveToObject) in the wrong compartment and then store these to fp->thisv! bholley's patch just made the disconnect between cx->compartment and fp->scopeChain->compartment more evident :) Nick: could you apply this patch and test whether it fixes the problem you're seeing? In the meantime I'll try to cook up a shell testcase...
Attachment #779417 - Flags: review?(jdemooij)
Attachment #779417 - Flags: feedback?(nfitzgerald)
Flags: needinfo?(luke)
Attachment #779417 - Attachment is patch: true
Severity: normal → critical
Attached patch fix and test — — Splinter Review
Getting shell testcases is always informative; I was able to get a compartment violation assert to trigger, but it requires not just cross-compartment calls but also multiple JSContexts to be used. It turns out, callers of computeThis are already entering the right compartment (they're about to touch the thisValue after all and we have compartment asserts a'plenty); the issue is that we use the JSContext of the StackFrame which may be different than the JSContext of the debugger. Thus, this patch just adds a JSContext *cx parameter (which is good form anyhow, for fallible methods). So this patch definitely fixes *a* bug; waiting for Nick to hear whether the patch fixes the above crash.
Attachment #779417 - Attachment is obsolete: true
Attachment #779417 - Flags: review?(jdemooij)
Attachment #779417 - Flags: feedback?(nfitzgerald)
Attachment #779472 - Flags: review?(jdemooij)
Attachment #779472 - Flags: feedback?(nfitzgerald)
Flags: needinfo?(jdemooij)
Applying that patch fixed it for me locally, but here is a try push to be sure: https://tbpl.mozilla.org/?tree=Try&rev=79bce0f13e3d
Ok with the new patch, it isn't crashing locally, and here is a try push: https://tbpl.mozilla.org/?tree=Try&rev=cf1b337067ab
I think that try push is good; I'm assuming f+ Nick?
Comment on attachment 779472 [details] [diff] [review] fix and test Yeah looks good to me.
Attachment #779472 - Flags: feedback?(nfitzgerald) → feedback+
Comment on attachment 779472 [details] [diff] [review] fix and test Review of attachment 779472 [details] [diff] [review]: ----------------------------------------------------------------- Good catch.
Attachment #779472 - Flags: review?(jdemooij) → review+
Oops, it marked me as the author... big enough deal to backout and re-commit?
(In reply to Nick Fitzgerald [:fitzgen] from comment #41) > Oops, it marked me as the author... big enough deal to backout and re-commit? If someone hasn't set the author in their global hgrc, I see it as finders keepers hehe ;-)
That's strange, I do have username set in hgrc; no big deal though. Just to make sure I understand, fx-team will eventually merge to m-c?
Probably my fault somewhere along the qimport, qref -m "...", qfinish sequence. Yes, fx-team will merge into m-c.
https://hg.mozilla.org/mozilla-central/rev/978b2693a7a4
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → mozilla25
(In reply to Bobby Holley (:bholley) from comment #46) > https://hg.mozilla.org/releases/mozilla-aurora/ > pushloghtml?changeset=84b828b63115 Backed out from Aurora for possibly causing xpcshell crashes along with the other changes from bholley's push in https://hg.mozilla.org/releases/mozilla-aurora/rev/659b0d61fbc6
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: