Stack overflow crash with failed Ci.nsINamed instances
Categories
(Core :: XPConnect, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox66 | --- | unaffected |
firefox67 | --- | wontfix |
firefox68 | --- | wontfix |
People
(Reporter: Oriol, Unassigned)
References
(Regression)
Details
(Keywords: crash, regression)
Open the browser console and enter one of these:
Cc["@mozilla.org/profile/migrator;1?app=browser&type=firefox"].createInstance(Ci.nsINamed);
Cc["@mozilla.org/remote-web-navigation;1"].createInstance(Ci.nsINamed);
Result: stack overflow crash
https://crash-stats.mozilla.org/report/index/02ee7a91-774a-4abf-ad50-a8e9a0190411
https://crash-stats.mozilla.org/report/index/a5d8bde0-d12d-46bc-8e20-35e240190411
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=e8ad5619116c&tochange=c999e7890f8b
I guess other classes changed in bug 1524688 are also affected, but I haven't tested them all.
Firefox ASan shows
==2348==ERROR: AddressSanitizer: stack-overflow on address 0x7ffe1022fc38 (pc 0x56278664f9f1 bp 0x7ffe10230490 sp 0x7ffe1022fc40 T0)
#0 0x56278664f9f0 in __asan_memset /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cc:27:3
#1 0x7fbacd089451 in PodAssign<mozilla::detail::HashTable<const JS::PropertyKey, mozilla::HashSet<JS::PropertyKey, mozilla::DefaultHasher<jsid>, js::TempAllocPolicy>::SetHashPolicy, js::TempAllocPolicy> > /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/PodOperations.h:78:3
#2 0x7fbacd089451 in HashTable /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/HashTable.h:1514
#3 0x7fbacd089451 in HashSet /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/HashTable.h:464
#4 0x7fbacd089451 in GCHashSet /builds/worker/workspace/build/src/obj-firefox/dist/include/js/GCHashTable.h:256
#5 0x7fbacd089451 in DispatchWrapper<JS::GCHashSet<JS::PropertyKey, mozilla::DefaultHasher<jsid>, js::TempAllocPolicy> > /builds/worker/workspace/build/src/obj-firefox/dist/include/js/RootingAPI.h:840
#6 0x7fbacd089451 in Rooted<JSContext *, JS::GCHashSet<JS::PropertyKey, mozilla::DefaultHasher<jsid>, js::TempAllocPolicy> > /builds/worker/workspace/build/src/obj-firefox/dist/include/js/RootingAPI.h:1025
#7 0x7fbacd089451 in Snapshot(JSContext*, JS::Handle<JSObject*>, unsigned int, JS::MutableHandle<JS::StackGCVector<JS::PropertyKey, js::TempAllocPolicy> >) /builds/worker/workspace/build/src/js/src/vm/Iteration.cpp:444
#8 0x7fbacd83e0d7 in JS_Enumerate(JSContext*, JS::Handle<JSObject*>, JS::MutableHandle<JS::GCVector<JS::PropertyKey, 0ul, js::TempAllocPolicy> >) /builds/worker/workspace/build/src/js/src/jsapi.cpp:2530:8
#9 0x7fbac256099d in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:250:10
#10 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#11 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#12 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#13 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#14 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#15 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
#16 0x7fbac2560c69 in (anonymous namespace)::GetFunctionName(JSContext*, JS::Handle<JSObject*>) /builds/worker/workspace/build/src/js/xpconnect/src/XPCWrappedJSClass.cpp:271:12
and GetFunctionName
keeps calling itself again and again.
https://searchfox.org/mozilla-central/rev/6dab6dad9cc852011a14275a8b2c2c03ed7600a7/js/xpconnect/src/XPCWrappedJSClass.cpp#239,271
Updated•6 years ago
|
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Almost no crashes on 66, 67 -> wontfix
Updated•6 years ago
|
Reporter | ||
Comment 2•6 years ago
|
||
66 is unaffected, the code just throws an error instead of crashing, as expected.
Comment 3•6 years ago
|
||
I looked at this a little bit. This code takes an object, unwraps it, then looks for a single property that is an object. If it has one, it recursively calls GetFunctionName. In this case, the property is wrappedJSObject, which ends up being the exact same object, so we get the infinite loop. I don't know how Kris's changes caused this to break. A trivial fix would be to just detect the loop and give up in that case.
Comment 4•6 years ago
|
||
If you have an old build around where this works handy, what properties does the object returned by, say,
Cc["@mozilla.org/profile/migrator;1?app=browser&type=firefox"].createInstance(Ci.nsISupports)
have? Maybe the old way of creating things just happened to have another property on the object, so this code would ignore it.
Comment 5•6 years ago
|
||
I guess the difference is that the old code had a JS-implemented factory, so the QueryInterface went straight to the raw JS object rather than going through XPConnect, and that threw because the objects don't explicitly implement nsINamed. That's arguably a bug. If you'd done .getService().QueryInterface(Ci.nsINamed)
you'd get the same behavior you get with the pure gerService call now.
I suppose we should fix this, although since bug 1528383 the wrappedJSObject
properties can just be removed.
Updated•6 years ago
|
Reporter | ||
Comment 6•6 years ago
|
||
Tested in 66.0.2
Cc["@mozilla.org/profile/migrator;1?app=browser&type=firefox"].createInstance(Ci.nsISupports)
works and produces a XPCWrappedNative_NoHelper
object with a QueryInterface
method, like in Nightly.
Cc["@mozilla.org/profile/migrator;1?app=browser&type=firefox"].createInstance(Ci.nsINamed);
reports a NS_NOINTERFACE:
error coming from XPCOMUtils.jsm:433, and throws this exception:
Exception
columnNumber: 0
data: null
filename: "debugger eval code"
lineNumber: 1
location: XPCWrappedNative_NoHelper { QueryInterface: QueryInterface(), filename: Getter, name: Getter, … }
message: "ComponentManager::CreateInstance returned failure code:"
name: "NS_ERROR_XPC_CI_RETURNED_FAILURE"
result: 2153185301
stack: "@debugger eval code:1:7\ngetEvalResult@resource://devtools/server/actors/webconsole/eval-with-debugger.js:134:18\nexports.evalWithDebugger@resource://devtools/server/actors/webconsole/eval-with-debugger.js:105:18\nevaluateJS@resource://devtools/server/actors/webconsole.js:1005:22\nevaluateJSAsync@resource://devtools/server/actors/webconsole.js:910:22\nonPacket@resource://devtools/server/main.js:1286:15\nsend/<@resource://devtools/shared/transport/local-transport.js:64:11\nexports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14\nexports.makeInfallible/<@resource://devtools/shared/ThreadSafeDevToolsUtils.js:109:14\n"
<prototype>: ExceptionPrototype { toString: toString(), name: Getter, message: Getter, … }
Updated•6 years ago
|
Reporter | ||
Comment 7•6 years ago
|
||
(In reply to Kris Maglione [:kmag] from comment #5)
You are right, Cc["@mozilla.org/profile/migrator;1?app=browser&type=firefox"].getService().QueryInterface(Ci.nsINamed)
already crashed before bug 1524688. The regression window for that is https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=df9beb781895fcd0493c21e95ad313e0044515ec&tochange=564e82f0f289af976da01c2d50507017bbc152b5
Comment 8•6 years ago
|
||
That includes the patch that implemented this nsIName behavior, so that makes sense.
Comment 9•6 years ago
|
||
The priority flag is not set for this bug.
:neha, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•6 years ago
|
Comment 10•6 years ago
|
||
Bulk change of P3 carryover bugs to wontfix for 68.
Updated•3 years ago
|
Updated•2 years ago
|
Description
•