923183 - GenerationalGC: xpcshell test test_install_strictcompat.js crashes in browser build [@ js::CompartmentChecker::check(JS::Value const&)]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

From https://tbpl.mozilla.org/php/getParsedLog.php?id=28670885&tree=Try

06:52:45  WARNING -  PROCESS-CRASH | /builds/slave/test/build/tests/xpcshell/tests/toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js | application crashed [@ js::CompartmentChecker::check(JS::Value const&)]
06:52:45     INFO -  Crash dump filename: /tmp/tmp1i6T5h/13c1e218-a925-d6ae-55e7f2c6-1e6a4785.dmp
06:52:45     INFO -  Operating system: Linux
06:52:45     INFO -                    0.0.0 Linux 3.2.0-23-generic-pae #36-Ubuntu SMP Tue Apr 10 22:19:09 UTC 2012 i686
06:52:45     INFO -  CPU: x86
06:52:45     INFO -       GenuineIntel family 6 model 45 stepping 7
06:52:45     INFO -       1 CPU
06:52:45     INFO -  Crash reason:  SIGSEGV
06:52:45     INFO -  Crash address: 0x2b2b2b2b
06:52:45     INFO -  Thread 0 (crashed)
06:52:45     INFO -   0  libxul.so!js::CompartmentChecker::check(JS::Value const&) [Interpreter.cpp:fc7e2207a52b : 3857 + 0x0]
06:52:45     INFO -      eip = 0xb505d498   esp = 0xbfba98e0   ebp = 0xbfba98f8   ebx = 0xb716de40
06:52:45     INFO -      esi = 0x09354640   edi = 0x09354640   eax = 0xbfba992c   ecx = 0x092caf18
06:52:45     INFO -      edx = 0x2b2b2b2b   efl = 0x00210206
06:52:45     INFO -      Found by: given as instruction pointer in context
06:52:45     INFO -   1  libxul.so!js::assertSameCompartmentDebugOnly<JS::Value> [Runtime.h:fc7e2207a52b : 803 + 0x47]
06:52:45     INFO -      eip = 0xb505d588   esp = 0xbfba9900   ebp = 0xbfba9948   ebx = 0xb716de40
06:52:45     INFO -      esi = 0x09354640   edi = 0x09354640
06:52:45     INFO -      Found by: call frame info
06:52:45     INFO -   2  libxul.so!Interpret [jscntxtinlines.h:fc7e2207a52b : 153 + 0x10]
06:52:45     INFO -      eip = 0xb50713a2   esp = 0xbfba9950   ebp = 0xbfba9d98   ebx = 0xb716de40
06:52:45     INFO -      esi = 0x09440330   edi = 0x09354640
06:52:45     INFO -      Found by: call frame info
06:52:45     INFO -   3  libxul.so!js::RunScript [Interpreter.cpp:fc7e2207a52b : 417 + 0x6]
06:52:45     INFO -      eip = 0xb5077548   esp = 0xbfba9da0   ebp = 0xbfba9dd8   ebx = 0xb716de40
06:52:45     INFO -      esi = 0x09354640   edi = 0xbfba9dc4
06:52:45     INFO -      Found by: call frame info
06:52:45     INFO -   4  libxul.so!js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*) [Interpreter.cpp:fc7e2207a52b : 386 + 0xe]
06:52:45     INFO -      eip = 0xb50777c7   esp = 0xbfba9de0   ebp = 0xbfba9e98   ebx = 0xb716de40
06:52:45     INFO -      esi = 0x09354640   edi = 0xbfba9e30
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -   5  libxul.so!js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) [Interpreter.cpp:fc7e2207a52b : 644 + 0x8]
06:52:46     INFO -      eip = 0xb5077bda   esp = 0xbfba9ea0   ebp = 0xbfba9f08   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x09354640   edi = 0x00000001
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -   6  libxul.so!JS_ExecuteScript(JSContext*, JSObject*, JSScript*, JS::Value*) [jsapi.cpp:fc7e2207a52b : 4850 + 0x16]
06:52:46     INFO -      eip = 0xb519ca23   esp = 0xbfba9f10   ebp = 0xbfba9f88   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x09354640   edi = 0xa5e9ea80
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -   7  libxul.so!JS_ExecuteScriptVersion(JSContext*, JSObject*, JSScript*, JS::Value*, JSVersion) [jsapi.cpp:fc7e2207a52b : 4858 + 0x19]
06:52:46     INFO -      eip = 0xb519cc36   esp = 0xbfba9f90   ebp = 0xbfba9fc8   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x09354640   edi = 0xbfba9fa0
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -   8  libxul.so!mozJSSubScriptLoader::LoadSubScript(nsAString_internal const&, JS::Value const&, nsAString_internal const&, JSContext*, JS::Value*) [mozJSSubScriptLoader.cpp:fc7e2207a52b : 317 + 0x1f]
06:52:46     INFO -      eip = 0xb423c007   esp = 0xbfba9fd0   ebp = 0xbfbaa1b8   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x00000000   edi = 0xbfbaa14c
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -   9  libxul.so + 0x18cc0a7
06:52:46     INFO -      eip = 0xb49f70a8   esp = 0xbfbaa1c0   ebp = 0xbfbaa1f8   ebx = 0xb716de40
06:52:46     INFO -      esi = 0xbfbaa288   edi = 0xbfbaa220
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -  10  libxul.so!CallMethodHelper::Call() [XPCWrappedNative.cpp:fc7e2207a52b : 2803 + 0xf]
06:52:46     INFO -      eip = 0xb4215bcd   esp = 0xbfbaa200   ebp = 0xbfbaa248
06:52:46     INFO -      Found by: previous frame's frame pointer
06:52:46     INFO -  11  libxul.so!XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) [XPCWrappedNative.cpp:fc7e2207a52b : 2109 + 0x16]
06:52:46     INFO -      eip = 0xb42163bd   esp = 0xbfbaa250   ebp = 0xbfbaa358   ebx = 0xb716de40
06:52:46     INFO -      esi = 0xbfbaa288   edi = 0x0931c818
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -  12  libxul.so!XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) [XPCWrappedNativeJSOps.cpp:fc7e2207a52b : 1308 + 0x9]
06:52:46     INFO -      eip = 0xb421bd18   esp = 0xbfbaa360   ebp = 0xbfbaa458   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x094c4250   edi = 0xbfbaa3a8
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -  13  libxul.so!js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) [jscntxtinlines.h:fc7e2207a52b : 218 + 0x16]
06:52:46     INFO -      eip = 0xb506662b   esp = 0xbfbaa460   ebp = 0xbfbaa4a8   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x09354640   edi = 0xbfbaa734
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -  14  libxul.so!js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct) [Interpreter.cpp:fc7e2207a52b : 460 + 0x18]
06:52:46     INFO -      eip = 0xb506ca69   esp = 0xbfbaa4b0   ebp = 0xbfbaa728   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x00000000   edi = 0xb7152fa0
06:52:46     INFO -      Found by: call frame info
06:52:46     INFO -  15  libxul.so!Interpret [Interpreter.cpp:fc7e2207a52b : 2463 + 0x2b]
06:52:46     INFO -      eip = 0xb50758ff   esp = 0xbfbaa730   ebp = 0xbfbaab78   ebx = 0xb716de40
06:52:46     INFO -      esi = 0x00000000   edi = 0x09354640
06:52:46     INFO -      Found by: call frame info

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug923183-xpcshell-crash

The problem is that some stack frames store the this and callee values before the stack frame, and we don't currently mark them.  With GGC this means that if they get moved then these are left pointing at the wrong locations.

I guess these must always be referred to from other locations, otherwise this would have shown up before.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 818415 [details] [diff] [review]
bug923183-xpcshell-crash

The interpreter stack was recently rewritten by Jan, so probably he should review this.

But, I think you're right, this case only occurs for eval/global code, in which case callee/this are marked via the RootedValue in ExecuteKernel which is why we didn't see this before.

Since hasArgs() == !(flags & (EVAL | GLOBAL)), I think you could slightly simplify the patch to be:

  if (hasArgs()) {
    ...
  } else {
    gc::MarkValueRootRange(trc, 2, ...)
  }

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 818415 [details] [diff] [review]
bug923183-xpcshell-crash

Review of attachment 818415 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch! r=me with luke's nit addressed.

Comment 4

[Jon Coppeard (:jonco)]

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

Comment 5

[Carsten Book [:Tomcat]]

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