1512162 - assert when typing in URL bar on ppc64le

Status: RESOLVED DUPLICATE of bug 1576303

(Core :: XPConnect, defect, P3)

Description

[Dan Horák]

I get an assert (and crash) on Fedora ppc64le when typing in the URL bar. I've bisected that to the enabling of -fstack-protector-strong via bug 1503589 (https://hg.mozilla.org/mozilla-central/rev/b8d320ccbfbc)

...
--DOMWINDOW == 26 (0x7fff81c23000) [pid = 61170] [serial = 8] [outer = (nil)] [url = about:sessionrestore]
--DOCSHELL 0x7fff82e9e000 == 7 [pid = 61170] [id = {b3a47210-5f92-4e0e-945e-2fc76bcdcacf}]
--DOMWINDOW == 7 (0x7fffa926c000) [pid = 61257] [serial = 8] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 6 (0x7fffa36b6400) [pid = 61257] [serial = 5] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 5 (0x7fffa6cb5000) [pid = 61257] [serial = 2] [outer = (nil)] [url = about:blank]
--DOMWINDOW == 4 (0x7fffa5341c00) [pid = 61257] [serial = 3] [outer = (nil)] [url = about:home]
Assertion failure: isDouble(), at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:445
#01: ???[/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/libxul.so +0x2a613dc]
...
#69: ???[/lib64/libc.so.6 +0x24878]
#70: __libc_start_main[/lib64/libc.so.6 +0x24a84]
#71: ??? (???:???)
Hit MOZ_CRASH(Aborting on channel error.) at /mnt/dan/firefox.git/ipc/glue/MessageChannel.cpp:2487
Hit MOZ_CRASH(Aborting on channel error.) at /mnt/dan/firefox.git/ipc/glue/MessageChannel.cpp:2487
Hit MOZ_CRASH(Aborting on channel error.) at /mnt/dan/firefox.git/ipc/glue/MessageChannel.cpp:2487
Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=21.2494) Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=19.7034) Crash Annotation GraphicsCriticalError: |[C0][GFX1-]: Receive IPC close with reason=AbnormalShutdown (t=10.9908) #01: ???[/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/libxul.so +0x208b030]
#01: ???[/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/libxul.so +0x208b030]
#01: ???[/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/libxul.so +0x208b030]
...



The backtrace from gdb looks as
#0  0x00007fffa2809d3c in __libc_signal_restore_set (set=0x7fffcf6edbc8) at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#1  0x00007fffa2809d3c in raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
#2  0x00007fff98cefa48 in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=<optimized out>, info=<optimized out>, context=0x7fffcf6edea0)
    at /mnt/dan/firefox.git/toolkit/profile/nsProfileLock.cpp:165
#3  0x00007fff99503b80 in js::UnixExceptionHandler(int, siginfo_t*, void*) (signum=<optimized out>, info=0x7fffcf6eec18, context=0x7fffcf6edea0)
    at /mnt/dan/firefox.git/js/src/ds/MemoryProtectionExceptionHandler.cpp:273
#4  0x00007fffa28704d8 in <signal handler called> () at arch/powerpc/kernel/vdso64/sigtramp.S
#5  0x00007fff933313f8 in JS::Value::setDouble(double) (d=<optimized out>, this=<optimized out>) at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:445
#6  0x00007fff933313f8 in JS::Value::setNumber(double) (this=<optimized out>, d=<optimized out>) at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:517
#7  0x00007fff93328ff0 in js::MutableWrappedPtrOperations<JS::Value, JS::MutableHandle<JS::Value> >::setNumber(double) (d=<optimized out>, this=0x7fffcf6eef68)
    at /mnt/dan/firefox.git/js/xpconnect/src/XPCConvert.cpp:134
#8  0x00007fff93328ff0 in XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, unsigned int, nsresult*) (d=..., s=0x7fffcf6ef1d8, type=..., iid=0x7fffcf6ef078, arrlen=<optimized out>, pErr=0x7fffcf6ef03c) at /mnt/dan/firefox.git/js/xpconnect/src/XPCConvert.cpp:134
#9  0x00007fff933875a0 in CallMethodHelper::GatherAndConvertResults() (this=0x7fffcf6ef148) at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNative.cpp:1344
#10 0x00007fff93372818 in CallMethodHelper::Call() (this=<optimized out>) at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNative.cpp:1218
#11 0x00007fff93372818 in XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) (ccx=..., mode=XPCWrappedNative::CALL_METHOD)
    at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNative.cpp:1173
#12 0x00007fff93380c60 in XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*) (cx=<optimized out>, argc=<optimized out>, vp=0x7fff7e0115a8)
    at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNativeJSOps.cpp:948
#13 0x00007fff98eb6b10 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7fff8ef61000, native=0x7fff93380630 <XPC_WN_CallMethod(JSContext*, unsigned int, JS::Value*)>, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:443
#14 0x00007fff98e9b53c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:535
#15 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#16 0x00007fff98e9bf38 in js::CallFromStack(JSContext*, JS::CallArgs const&) (cx=<optimized out>, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:594
#17 0x00007fff98e93784 in Interpret(JSContext*, js::RunState&) (cx=<optimized out>, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:3348
#18 0x00007fff98e9aae4 in js::RunScript(JSContext*, js::RunState&) (cx=0x7fff8ef61000, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:423
#19 0x00007fff98e9bc28 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:563
#20 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#21 0x00007fff98e9bfac in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, fval=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:606
#22 0x00007fff98e9c2e4 in js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, thisv=..., getter=..., rval=...)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:730
#23 0x00007fff99235a68 in CallGetter (vp=..., shape=..., receiver=..., obj=..., cx=<optimized out>) at /mnt/dan/firefox.git/js/src/vm/NativeObject.cpp:2246
#24 0x00007fff99235a68 in GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (cx=0x7fff8ef61000, receiver=..., obj=..., shape=..., vp=...)
    at /mnt/dan/firefox.git/js/src/vm/NativeObject.cpp:2298
#25 0x00007fff99238b7c in NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType) (cx=<optimized out>, obj=..., receiver=..., id=..., nameLookup=NotNameLookup, vp=...) at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Class.h:302
#26 0x00007fff99238d8c in js::NativeGetProperty(JSContext*, JS::Handle<js::NativeObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) (cx=<optimized out>, obj=..., receiver=..., id=..., vp=...) at /mnt/dan/firefox.git/js/src/vm/NativeObject.cpp:2584
#27 0x00007fff98eae6a8 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::PropertyKey>, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, obj=..., receiver=..., id=..., vp=...) at /mnt/dan/firefox.git/js/src/vm/ObjectOperations-inl.h:117
#28 0x00007fff98e818a0 in js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>) (vp=..., name=<optimized out>, receiver=..., obj=..., cx=<optimized out>) at /mnt/dan/firefox.git/js/src/vm/ObjectOperations-inl.h:124
#29 0x00007fff98e818a0 in js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>) (cx=<optimized out>, v=..., name=..., vp=...)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:4759
#30 0x00007fff98e91f40 in GetPropertyOperation (vp=..., lval=..., pc=0x7fff7f8a78a7 "5\002", script=..., fp=0x7fff7e011238, cx=<optimized out>)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:215
#31 0x00007fff98e91f40 in Interpret(JSContext*, js::RunState&) (cx=<optimized out>, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:3048
#32 0x00007fff98e9aae4 in js::RunScript(JSContext*, js::RunState&) (cx=0x7fff8ef61000, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:423
#33 0x00007fff98e9bc28 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
---Type <return> to continue, or q <return> to quit---
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:563
#34 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#35 0x00007fff98e9bfac in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=<optimized out>, fval=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:606
#36 0x00007fff992e3a80 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, name=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/SelfHosting.cpp:1840
#37 0x00007fff98fca7b8 in AsyncFunctionResume(JSContext*, JS::Handle<js::PromiseObject*>, JS::HandleValue, ResumeKind, JS::HandleValue) (cx=<optimized out>, resultPromise=..., generatorVal=..., kind=ResumeKind::Normal, valueOrReason=...) at /mnt/dan/firefox.git/js/src/vm/AsyncFunction.cpp:202
#38 0x00007fff98fca9f8 in AsyncFunctionStart(JSContext*, JS::Handle<js::PromiseObject*>, JS::HandleValue) (cx=<optimized out>, resultPromise=..., generatorVal=...)
    at /mnt/dan/firefox.git/js/src/vm/AsyncFunction.cpp:217
#39 0x00007fff98fdaa00 in WrappedAsyncFunction(JSContext*, unsigned int, JS::Value*) (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>)
    at /mnt/dan/firefox.git/js/src/vm/AsyncFunction.cpp:94
#40 0x00007fff98eb6b10 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7fff8ef61000, native=0x7fff98fda64c <WrappedAsyncFunction(JSContext*, unsigned int, JS::Value*)>, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:443
#41 0x00007fff98e9b53c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:535
#42 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#43 0x00007fff98e9bf38 in js::CallFromStack(JSContext*, JS::CallArgs const&) (cx=<optimized out>, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:594
#44 0x00007fff98e93784 in Interpret(JSContext*, js::RunState&) (cx=<optimized out>, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:3348
#45 0x00007fff98e9aae4 in js::RunScript(JSContext*, js::RunState&) (cx=0x7fff8ef61000, state=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:423
#46 0x00007fff98e9bc28 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:563
#47 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#48 0x00007fff98e9bfac in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=<optimized out>, fval=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:606
#49 0x00007fff992e3a80 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, name=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/SelfHosting.cpp:1840
#50 0x00007fff98fca7b8 in AsyncFunctionResume(JSContext*, JS::Handle<js::PromiseObject*>, JS::HandleValue, ResumeKind, JS::HandleValue) (cx=<optimized out>, resultPromise=..., generatorVal=..., kind=ResumeKind::Normal, valueOrReason=...) at /mnt/dan/firefox.git/js/src/vm/AsyncFunction.cpp:202
#51 0x00007fff98fcaa40 in js::AsyncFunctionAwaitedFulfilled(JSContext*, JS::Handle<js::PromiseObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>) (cx=<optimized out>, resultPromise=..., generatorVal=..., value=...) at /mnt/dan/firefox.git/js/src/vm/AsyncFunction.cpp:231
#52 0x00007fff98f48668 in AsyncFunctionPromiseReactionJob(JSContext*, JS::Handle<PromiseReactionRecord*>, JS::MutableHandleValue) (cx=<optimized out>, reaction=..., rval=...)
    at /mnt/dan/firefox.git/js/src/builtin/Promise.cpp:1471
#53 0x00007fff98f7cafc in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>)
    at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/CallArgs.h:100
#54 0x00007fff98eb6b10 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (cx=0x7fff8ef61000, native=0x7fff98f7c778 <PromiseReactionJob(JSContext*, unsigned int, JS::Value*)>, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:443
#55 0x00007fff98e9b53c in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=<optimized out>, args=..., construct=js::NO_CONSTRUCT)
    at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:535
#56 0x00007fff98e9bd70 in InternalCall(JSContext*, js::AnyInvokeArgs const&) (cx=0x7fff8ef61000, args=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:590
#57 0x00007fff98e9bfac in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) (cx=<optimized out>, fval=..., thisv=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/vm/Interpreter.cpp:606
#58 0x00007fff9956beb4 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (cx=0x7fff8ef61000, thisv=..., fval=..., args=..., rval=...) at /mnt/dan/firefox.git/js/src/jsapi.cpp:2651
#59 0x00007fff94755260 in mozilla::dom::PromiseJobCallback::Call(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&) (this=<optimized out>, cx=<optimized out>, aThisVal=..., aRv=...)
    at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/jsapi.h:153
#60 0x00007fff91d818e0 in mozilla::dom::PromiseJobCallback::Call(mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) (this=
    0x7fff81c33100, aRv=..., aExecutionReason=<optimized out>, aExceptionHandling=<optimized out>, aRealm=<optimized out>)
    at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/mozilla/dom/CallbackObject.h:312
#61 0x00007fff91d87b34 in mozilla::dom::PromiseJobCallback::Call(char const*) (aExecutionReason=0x7fff9ba2ccd8 "promise callback", this=0x7fff81c33100)
    at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/mozilla/ErrorResult.h:753
#62 0x00007fff91d87b34 in mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&) (this=0x7fff7d88bd40, aAso=...) at /mnt/dan/firefox.git/xpcom/base/CycleCollectedJSContext.cpp:236
#63 0x00007fff91d7dc6c in mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool) (this=0x7fff9ef30000, aForce=false)
    at /mnt/dan/firefox.git/xpcom/base/CycleCollectedJSContext.cpp:550
---Type <return> to continue, or q <return> to quit---
#64 0x00007fff91d7e170 in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) (this=0x7fff9ef30000, aRecursionDepth=<optimized out>)
    at /mnt/dan/firefox.git/xpcom/base/CycleCollectedJSContext.cpp:392
#65 0x00007fff9332cd08 in XPCJSContext::AfterProcessTask(unsigned int) (this=0x7fff9ef30000, aNewRecursionDepth=<optimized out>)
    at /mnt/dan/firefox.git/js/xpconnect/src/XPCJSContext.cpp:1252
#66 0x00007fff91ed82fc in nsThread::ProcessNextEvent(bool, bool*) (this=0x7fffa1ef0b80, aMayWait=<optimized out>, aResult=0x7fffcf6f3ca7)
    at /mnt/dan/firefox.git/xpcom/threads/nsThread.cpp:1215
#67 0x00007fff91eca24c in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7fffa1ef0b80, aMayWait=<optimized out>) at /mnt/dan/firefox.git/xpcom/threads/nsThreadUtils.cpp:468
#68 0x00007fff9295dcdc in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7fffa1e45b40, aDelegate=0x7fffa1e90240) at /mnt/dan/firefox.git/ipc/glue/MessagePump.cpp:88
#69 0x00007fff9289af64 in MessageLoop::RunInternal() (this=0x7fffa1e90240) at /mnt/dan/firefox.git/ipc/chromium/src/base/message_loop.cc:314
#70 0x00007fff9289b00c in MessageLoop::RunHandler() (this=0x7fffa1e90240) at /mnt/dan/firefox.git/ipc/chromium/src/base/message_loop.cc:307
#71 0x00007fff9289b2e8 in MessageLoop::Run() (this=0x7fffa1e90240) at /mnt/dan/firefox.git/ipc/chromium/src/base/message_loop.cc:289
#72 0x00007fff96c2f7c0 in nsBaseAppShell::Run() (this=0x7fffa1e37400) at /mnt/dan/firefox.git/widget/nsBaseAppShell.cpp:137
#73 0x00007fff98b7a580 in nsAppStartup::Run() (this=0x7fff901e4700) at /mnt/dan/firefox.git/toolkit/components/startup/nsAppStartup.cpp:271
#74 0x00007fff98d0df60 in XREMain::XRE_mainRun() (this=0x7fffcf6f4110) at /mnt/dan/firefox.git/toolkit/xre/nsAppRunner.cpp:4622
#75 0x00007fff98d0f63c in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=0x7fffcf6f4110, argc=4, argv=0x7fffcf6f5818, aConfig=...)
    at /mnt/dan/firefox.git/toolkit/xre/nsAppRunner.cpp:4760
#76 0x00007fff98d0ffc0 in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=<optimized out>, argv=0x7fffcf6f5818, aConfig=...)
    at /mnt/dan/firefox.git/toolkit/xre/nsAppRunner.cpp:4845
#77 0x00007fff98d28014 in mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=<optimized out>, argc=<optimized out>, argv=<optimized out>, aConfig=...)
    at /mnt/dan/firefox.git/toolkit/xre/Bootstrap.cpp:39
#78 0x0000000119817098 in do_main(int, char**, char**) (argc=4, argv=0x7fffcf6f5818, envp=<optimized out>) at /mnt/dan/firefox.git/browser/app/nsBrowserApp.cpp:214
#79 0x00000001198171c4 in main(int, char**, char**) (argc=<optimized out>, argv=0x7fffcf6f5818, envp=0x7fffcf6f5840) at /mnt/dan/firefox.git/browser/app/nsBrowserApp.cpp:293

Comment 1

[Dan Horák]

For the record - trunk builds and run with the mentioned change reverted

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Is it a genuine bug that's being surfaced now or is this toolchain's implementation of stack canaries broken?

Comment 3

[Dan Horák]

Fedora builds whole distro with -fstack-protector-strong for all arches, so the toolchain should be working fine. So likely this is a bug on FF side.

Comment 4

[A. Wilcox [:awilfox]]

I've just built m-u at:

changeset:   501723:f3c4afc3c780
bookmark:    inbound
user:        Nika Layzell <[e-mail]>
date:        Tue Dec 04 17:18:31 2018 -0500
summary:     Bug 1512085 - Don't overlap IDs between content and middleman process, r=bhackett

Full clobber, ensured --enable-hardening was passed, -fstack-protector-strong is noted in all .mk files that deal with C/C++.  Browser is working fine and allows me to type out whole URLs into the URL bar on Adélie Linux with musl libc 1.1.20: powerpc64-foxkit-linux-musl (64-bit PowerPC, big endian).  May be a glibc issue?

Comment 5

[Cameron Kaiser [:spectre]]

:awilfox, dumb questions just for completeness: debug or opt build? gcc or clang?

Looking at the stack trace it seems like JS::Value::setDouble(double) (frame 5) hit the signal handler, so I'm not sure the libc is the explanation. That function has this interesting little comment:

  void setDouble(double d) {
    // Don't assign to asDouble_ to fix a miscompilation with GCC 5.2.1 and
    // 5.3.1. See bug 1312488.
    *this = Value(d);
    MOZ_ASSERT(isDouble());
  }

However, the isDouble() assert also triggers, though that could be because the signal got hit and everything is now fubar. Dan, just shooting in the dark here, but if you change that to

  void setDouble(double d) {
    asDouble_ = d;
    MOZ_ASSERT(isDouble());
  }

does that fix the problem? If so, we may need a JavaScript peer to look at the patch.

Comment 6

[Cameron Kaiser [:spectre]]

(If you're not able to test, I'm pulling down a tree and can try to build remotely, though I won't be on my T2 until I'm back from my business trip.)

Comment 7

[Cameron Kaiser [:spectre]]

By the way, that file is js/public/Value.h, sorry.

Comment 8

[Dan Horák]

Unfortunately the proposed change doesn't eliminate the assertion.

this is my .mozconfig

export CC=/usr/bin/gcc
export CXX=/usr/bin/g++
export PATH=$HOME/.cargo/bin:$PATH

mk_add_options MOZ_MAKE_FLAGS="-j16"
ac_add_options --enable-application=browser
ac_add_options --enable-optimize="-Og"
ac_add_options --enable-debug
ac_add_options --disable-release
ac_add_options --disable-tests
ac_add_options --enable-linker=bfd

Comment 9

[Dan Horák]

With --enable-optimize="-O0" the problem goes away, but with targeted "non-optimization" in Value.h

diff --git a/js/public/Value.h b/js/public/Value.h
index ae8a7227fd..53d81746e8 100644
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -438,10 +438,8 @@ union alignas(8) Value {
     asBits_ = bitsFromTagAndPayload(JSVAL_TAG_INT32, uint32_t(i));
   }
 
-  void setDouble(double d) {
-    // Don't assign to asDouble_ to fix a miscompilation with GCC 5.2.1 and
-    // 5.3.1. See bug 1312488.
-    *this = Value(d);
+  __attribute__((optimize(("O0")))) void setDouble(double d) {
+    asDouble_ = d;
     MOZ_ASSERT(isDouble());
   }
 
@@ -507,7 +505,7 @@ union alignas(8) Value {
     }
   }
 
-  bool setNumber(double d) {
+  __attribute__((optimize(("O0")))) bool setNumber(double d) {
     int32_t i;
     if (mozilla::NumberIsInt32(d, &i)) {
       setInt32(i);
@@ -588,7 +586,7 @@ union alignas(8) Value {
     return asBits_ == bitsFromTagAndPayload(JSVAL_TAG_INT32, uint32_t(i32));
   }
 
-  bool isDouble() const {
+  __attribute__((optimize(("O0")))) bool isDouble() const {
 #if defined(JS_NUNBOX32)
     return uint32_t(toTag()) <= uint32_t(JSVAL_TAG_CLEAR);
 #elif defined(JS_PUNBOX64)

I get following

#0  0x00007fff8b309d5c in __libc_signal_restore_set (set=0x7ffffa382028) at ../sysdeps/unix/sysv/linux/nptl-signals.h:80
#1  0x00007fff8b309d5c in raise (sig=<optimized out>) at ../sysdeps/unix/sysv/linux/raise.c:48
#2  0x00007fff80b78c48 in nsProfileLock::FatalSignalHandler(int, siginfo_t*, void*) (signo=<optimized out>, info=<optimized out>, context=0x7ffffa382300)
    at /mnt/dan/firefox.git/toolkit/profile/nsProfileLock.cpp:165
#3  0x00007fff8138421c in js::UnixExceptionHandler(int, siginfo_t*, void*) (signum=<optimized out>, info=0x7ffffa383078, context=0x7ffffa382300)
    at /mnt/dan/firefox.git/js/src/ds/MemoryProtectionExceptionHandler.cpp:273
#4  0x00007fff8b3704d8 in <signal handler called> () at arch/powerpc/kernel/vdso64/sigtramp.S
#5  0x00007fff79d7866c in JS::Value::setDouble(double) (this=0x7ffffa383518, d=-nan(0x9800000000000)) at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:443
#6  0x00007fff7b1a8bb8 in JS::Value::setNumber(double) (this=0x7ffffa383518, d=-nan(0x9800000000000)) at /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:515
#7  0x00007fff7b17dec8 in js::MutableWrappedPtrOperations<JS::Value, JS::MutableHandle<JS::Value> >::setNumber(double) (d=<optimized out>, this=0x7ffffa383428)
    at /mnt/dan/firefox.git/js/xpconnect/src/XPCConvert.cpp:134
#8  0x00007fff7b17dec8 in XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, unsigned int, nsresult*) (d=..., s=0x7ffffa383698, type=..., iid=0x7ffffa383538, arrlen=<optimized out>, pErr=0x7ffffa3834fc) at /mnt/dan/firefox.git/js/xpconnect/src/XPCConvert.cpp:134
#9  0x00007fff7b1ffca0 in CallMethodHelper::GatherAndConvertResults() (this=0x7ffffa383608) at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNative.cpp:1344
#10 0x00007fff7b1deb68 in CallMethodHelper::Call() (this=<optimized out>) at /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedNative.cpp:1218
...

Comment 10

[Dan Horák]

Seems the d=-nan(0x9800000000000) value isn't changing between runs. IMHO the failure comes from the "suggestions" function that offers search items or sites from the browsing history.

Comment 11

[Gian-Carlo Pascutto [:gcp]]

>Fedora builds whole distro with -fstack-protector-strong for all arches, so the toolchain should be working fine.

Firefox is unfortunately quite capable of hitting those kind of bugs that other apps don't, so this won't necessarily follow. (I'm not saying it's not a Firefox bug)

Comment 12

[Dan Horák]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #11)
> >Fedora builds whole distro with -fstack-protector-strong for all arches, so the toolchain should be working fine.
> 
> Firefox is unfortunately quite capable of hitting those kind of bugs that
> other apps don't, so this won't necessarily follow. (I'm not saying it's not
> a Firefox bug)

I don't disagree :-)

Comment 13

[Dan Horák]

If I see right, then the official Fedora Firefox builds use the hardening compiler options including -fstack-protector-strong for all arches. And such FF works fine as you can see from this comment :-)

Comment 14

[Cameron Kaiser [:spectre]]

My next suspect is XPConnect, especially since we've had recent work on that. It could still be a gcc bug. :awilfox, your working build, was it gcc or clang? Debug, opt, ?

Comment 15

[A. Wilcox [:awilfox]]

(In reply to Cameron Kaiser [:spectre] from comment #5)
> :awilfox, dumb questions just for completeness: debug or opt build? gcc or
> clang?

GCC 6.4.0:

awilcox on gwyn [pts/3 Wed 5 21:10] ~: gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/powerpc64-foxkit-linux-musl/6.4.0/lto-wrapper
Target: powerpc64-foxkit-linux-musl
Configured with: /usr/src/packages/system/gcc/src/gcc-6.4.0/configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --build=powerpc64-foxkit-linux-musl --host=powerpc64-foxkit-linux-musl --target=powerpc64-foxkit-linux-musl --with-pkgversion='Adelie 6.4.0' --with-bugurl=http://bts.adelielinux.org/ --enable-checking=release --disable-fixed-point --disable-libstdcxx-pch --disable-multilib --disable-werror --disable-symvers --enable-__cxa_atexit --enable-default-pie --enable-cloog-backend --enable-languages=c,c++,objc,java,go,fortran --with-abi=elfv2 --enable-secureplt --enable-decimal-float=no --disable-libquadmath --disable-libmpx --disable-libmudflap --disable-libsanitizer --enable-shared --enable-threads --enable-tls --with-system-zlib --with-linker-hash-style=gnu
Thread model: posix
gcc version 6.4.0 (Adelie 6.4.0) 


Debug build:

ac_add_options --prefix=/usr
ac_add_options --libdir=/usr/lib
ac_add_options --disable-crashreporter
ac_add_options --disable-install-strip
ac_add_options --disable-jemalloc
ac_add_options --disable-profiling
ac_add_options --disable-strip
ac_add_options --enable-tests
ac_add_options --enable-application=browser
ac_add_options --enable-alsa
ac_add_options --enable-dbus
ac_add_options --enable-debug
ac_add_options --enable-default-toolkit=cairo-gtk3
ac_add_options --enable-official-branding
ac_add_options --enable-pulseaudio
ac_add_options --enable-startup-notification
ac_add_options --enable-system-ffi
ac_add_options --enable-updater
ac_add_options --with-system-bz2
ac_add_options --with-system-icu
ac_add_options --with-system-jpeg
ac_add_options --with-system-libevent
ac_add_options --with-system-nspr
ac_add_options --with-system-zlib
ac_add_options --host=powerpc64-foxkit-linux-musl
ac_add_options --target=powerpc64-foxkit-linux-musl
mk_add_options MOZ_MAKE_FLAGS=-j32

Comment 16

[A. Wilcox [:awilfox]]

Dan, can you try and run:

./mach test js/xpconnect/tests/unit/test_bug809674.js

This test passes here.  I know that when I was debugging float representations in XPConnect, this test was vital in poking it in weird ways.

Comment 17

[A. Wilcox [:awilfox]]

Example passing output:

 0:02.63 WARNING MOZ_NODE_PATH environment variable not set. Tests requiring http/2 will fail.
 0:02.64 INFO Using at most 96 threads.
 0:02.64 SUITE_START: xpcshell - running 1 tests
 0:02.64 TEST_START: js/xpconnect/tests/unit/test_bug809674.js
 0:02.64 INFO js/xpconnect/tests/unit/test_bug809674.js | full command: ['/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin/xpcshell', '-g', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin', '-a', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin', '-r', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin/components/httpd.manifest', '-m', '-s', '-e', 'const _HEAD_JS_PATH = "/var/clean/mozppc/mozilla-unified/testing/xpcshell/head.js";', '-e', 'const _MOZINFO_JS_PATH = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-profile-H9Ika0/mozinfo.json";', '-e', 'const _PREFS_FILE = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/user.js";', '-e', 'const _TESTING_MODULES_DIR = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/modules/";', '-f', '/var/clean/mozppc/mozilla-unified/testing/xpcshell/head.js', '-p', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-plugins-5J9JAv', '-e', 'const _HEAD_FILES = [];', '-e', 'const _JSDEBUGGER_PORT = 0;', '-e', u'const _TEST_FILE = ["/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js"];', '-e', u'const _TEST_NAME = "js/xpconnect/tests/unit/test_bug809674.js";', '-e', '_execute_test(); quit(0);']
 0:02.64 INFO js/xpconnect/tests/unit/test_bug809674.js | current directory: u'/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/xpcshell/js/xpconnect/tests/unit'
 0:02.64 INFO js/xpconnect/tests/unit/test_bug809674.js | environment: ['XPCOM_DEBUG_BREAK=stack-and-abort', 'MOZ_CRASHREPORTER=1', 'LD_LIBRARY_PATH=/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin:obj-powerpc64-foxkit-linux-musl/dist/bin', 'XPCSHELL_TEST_TEMP_DIR=/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-other-_00SCQ', 'MOZ_DISABLE_CONTENT_SANDBOX=1', 'MOZ_DISABLE_NONLOCAL_CONNECTIONS=1', 'MOZ_DEVELOPER_REPO_DIR=/var/clean/mozppc/mozilla-unified', 'XPCSHELL_TEST_PROFILE_DIR=/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-profile-H9Ika0', 'MOZ_CRASHREPORTER_NO_REPORT=1']
 0:03.00 pid:50152 Full command: ['/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin/xpcshell', '-g', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin', '-a', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin', '-r', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/dist/bin/components/httpd.manifest', '-m', '-s', '-e', 'const _HEAD_JS_PATH = "/var/clean/mozppc/mozilla-unified/testing/xpcshell/head.js";', '-e', 'const _MOZINFO_JS_PATH = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-profile-H9Ika0/mozinfo.json";', '-e', 'const _PREFS_FILE = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/user.js";', '-e', 'const _TESTING_MODULES_DIR = "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/modules/";', '-f', '/var/clean/mozppc/mozilla-unified/testing/xpcshell/head.js', '-p', '/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/temp/xpc-plugins-5J9JAv', '-e', 'const _HEAD_FILES = [];', '-e', 'const _JSDEBUGGER_PORT = 0;', '-e', u'const _TEST_FILE = ["/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js"];', '-e', u'const _TEST_NAME = "js/xpconnect/tests/unit/test_bug809674.js";', '-e', '_execute_test(); quit(0);']
pid:50152 Couldn't convert chrome URL: chrome://branding/locale/brand.properties
 0:03.01 INFO (xpcshell/head.js) | test MAIN run_test pending (1)
 0:03.02 PASS run_test - [run_test : 15] 46 == 46
 0:03.02 PASS run_test - [run_test : 18] 16 == 16
 0:03.02 PASS run_test - [run_test : 19] 2 == 2
 0:03.03 PASS run_test - [run_test : 20] 63 == 63
 0:03.03 PASS run_test - [run_test : 22] "foox" == "foox"
 0:03.03 PASS run_test - [run_test : 23] "foo1.2" == "foo1.2"
 0:03.04 PASS run_test - [run_test : 24] "1234foo" == "1234foo"
 0:03.04 PASS run_test - [run_test : 26] 255 == 255
 0:03.04 PASS run_test - [run_test : 28] 7 == 7
 0:03.04 PASS run_test - [run_test : 29] "undefined" == "undefined"
 0:03.04 PASS run_test - [run_test : 33] 42 == 42
 0:03.05 PASS run_test - [run_test : 35] [object XPCWrappedNative_NoHelper] == [object XPCWrappedNative_NoHelper]
 0:03.05 PASS run_test - [run_test : 37] 123 == 123
 0:03.05 PASS run_test - [run_test : 39] 124 == 124
 0:03.06 pid:50152 [50152, Main Thread] WARNING: IDL methods marked with [optional_argc] may not be implemented in JS: file /var/clean/mozppc/mozilla-unified/js/xpconnect/src/XPCWrappedJSClass.cpp, line 970
 0:03.06 pid:50152 JavaScript error: /var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js, line 43: Error: IDL methods marked with [optional_argc] may not be implemented in JS
 0:03.06 PASS run_test - [run_test : 46] true == true
 0:03.06 PASS run_test - [run_test : 47] true == true
 0:03.06 INFO (xpcshell/head.js) | test MAIN run_test finished (1)
 0:03.06 INFO exiting test
 0:03.07 pid:50152 [50152, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /var/clean/mozppc/mozilla-unified/dom/media/CubebUtils.cpp, line 358
 0:03.07 INFO "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
 0:03.07 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "Error: IDL methods marked with [optional_argc] may not be implemented in JS" {file: "/var/clean/mozppc/mozilla-unified/obj-powerpc64-foxkit-linux-musl/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js" line: 43}]"
 0:03.28 pid:50152 nsStringStats
 0:03.28 pid:50152  => mAllocCount:           6215
 0:03.28 pid:50152  => mReallocCount:            0
 0:03.29 pid:50152  => mFreeCount:            6215
 0:03.29 pid:50152  => mShareCount:           9768
 0:03.29 pid:50152  => mAdoptCount:            125
 0:03.29 pid:50152  => mAdoptFreeCount:        125
 0:03.29 pid:50152  => Process ID: 50152, Thread ID: 1100546691104
 0:03.30 TEST_END: Test PASS. Subtests passed 16/16. Unexpected 0
 0:03.31 INFO INFO | Result summary:
 0:03.31 INFO INFO | Passed: 1
 0:03.31 INFO INFO | Failed: 0
 0:03.31 INFO INFO | Todo: 0
 0:03.31 INFO INFO | Retried: 0
 0:03.31 SUITE_END
 0:03.31 
Overall Summary
===============

xpcshell
~~~~~~~~
Ran 17 checks (1 tests, 16 subtests)
Expected results: 17
OK

Comment 18

[Dan Horák]

Seems it's passing

 0:01.76 WARNING MOZ_NODE_PATH environment variable not set. Tests requiring http/2 will fail.
 0:01.76 INFO Using at most 96 threads.
 0:01.77 SUITE_START: xpcshell - running 1 tests
 0:01.79 TEST_START: js/xpconnect/tests/unit/test_bug809674.js
 0:01.79 INFO js/xpconnect/tests/unit/test_bug809674.js | full command: ['/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/xpcshell', '-g', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin', '-a', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin', '-r', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/components/httpd.manifest', '-m', '-s', '-e', 'const _HEAD_JS_PATH = "/mnt/dan/firefox.git/testing/xpcshell/head.js";', '-e', 'const _MOZINFO_JS_PATH = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-profile-hXOXOn/mozinfo.json";', '-e', 'const _PREFS_FILE = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/user.js";', '-e', 'const _TESTING_MODULES_DIR = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/modules/";', '-f', '/mnt/dan/firefox.git/testing/xpcshell/head.js', '-p', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-plugins-jvSyfy', '-e', 'const _HEAD_FILES = [];', '-e', 'const _JSDEBUGGER_PORT = 0;', '-e', u'const _TEST_FILE = ["/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js"];', '-e', u'const _TEST_NAME = "js/xpconnect/tests/unit/test_bug809674.js";', '-e', '_execute_test(); quit(0);']
 0:01.79 INFO js/xpconnect/tests/unit/test_bug809674.js | current directory: u'/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/xpcshell/js/xpconnect/tests/unit'
 0:01.79 INFO js/xpconnect/tests/unit/test_bug809674.js | environment: ['XPCOM_DEBUG_BREAK=stack-and-abort', 'LD_LIBRARY_PATH=/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin', 'MOZ_CRASHREPORTER=1', 'XPCSHELL_TEST_TEMP_DIR=/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-other-ymDoGE', 'XPCSHELL_TEST_PROFILE_DIR=/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-profile-hXOXOn', 'MOZ_DISABLE_CONTENT_SANDBOX=1', 'MOZ_DEVELOPER_REPO_DIR=/mnt/dan/firefox.git', 'MOZ_DISABLE_NONLOCAL_CONNECTIONS=1', 'MOZ_CRASHREPORTER_NO_REPORT=1']
 0:02.09 pid:8170 Full command: ['/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/xpcshell', '-g', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin', '-a', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin', '-r', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/dist/bin/components/httpd.manifest', '-m', '-s', '-e', 'const _HEAD_JS_PATH = "/mnt/dan/firefox.git/testing/xpcshell/head.js";', '-e', 'const _MOZINFO_JS_PATH = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-profile-hXOXOn/mozinfo.json";', '-e', 'const _PREFS_FILE = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/user.js";', '-e', 'const _TESTING_MODULES_DIR = "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/modules/";', '-f', '/mnt/dan/firefox.git/testing/xpcshell/head.js', '-p', '/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/temp/xpc-plugins-jvSyfy', '-e', 'const _HEAD_FILES = [];', '-e', 'const _JSDEBUGGER_PORT = 0;', '-e', u'const _TEST_FILE = ["/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js"];', '-e', u'const _TEST_NAME = "js/xpconnect/tests/unit/test_bug809674.js";', '-e', '_execute_test(); quit(0);']
pid:8170 Couldn't convert chrome URL: chrome://branding/locale/brand.properties
 0:02.12 INFO (xpcshell/head.js) | test MAIN run_test pending (1)
 0:02.12 PASS run_test - [run_test : 15] 46 == 46
 0:02.13 PASS run_test - [run_test : 18] 16 == 16
 0:02.13 PASS run_test - [run_test : 19] 2 == 2
 0:02.13 PASS run_test - [run_test : 20] 63 == 63
 0:02.13 PASS run_test - [run_test : 22] "foox" == "foox"
 0:02.13 PASS run_test - [run_test : 23] "foo1.2" == "foo1.2"
 0:02.14 PASS run_test - [run_test : 24] "1234foo" == "1234foo"
 0:02.14 PASS run_test - [run_test : 26] 255 == 255
 0:02.14 PASS run_test - [run_test : 28] 7 == 7
 0:02.14 PASS run_test - [run_test : 29] "undefined" == "undefined"
 0:02.14 PASS run_test - [run_test : 33] 42 == 42
 0:02.15 PASS run_test - [run_test : 35] [object XPCWrappedNative_NoHelper] == [object XPCWrappedNative_NoHelper]
 0:02.15 PASS run_test - [run_test : 37] 123 == 123
 0:02.15 PASS run_test - [run_test : 39] 124 == 124
 0:02.15 pid:8170 [8170, Main Thread] WARNING: IDL methods marked with [optional_argc] may not be implemented in JS: file /mnt/dan/firefox.git/js/xpconnect/src/XPCWrappedJSClass.cpp, line 970
 0:02.15 pid:8170 JavaScript error: /mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js, line 43: Error: IDL methods marked with [optional_argc] may not be implemented in JS
 0:02.15 PASS run_test - [run_test : 46] true == true
 0:02.15 PASS run_test - [run_test : 47] true == true
 0:02.16 INFO (xpcshell/head.js) | test MAIN run_test finished (1)
 0:02.16 INFO exiting test
 0:02.16 pid:8170 [8170, Main Thread] WARNING: Could not get the program name for a cubeb stream.: 'NS_SUCCEEDED(rv)', file /mnt/dan/firefox.git/dom/media/CubebUtils.cpp, line 358
 0:02.16 INFO "CONSOLE_MESSAGE: (info) No chrome package registered for chrome://branding/locale/brand.properties"
 0:02.16 INFO "CONSOLE_MESSAGE: (error) [JavaScript Error: "Error: IDL methods marked with [optional_argc] may not be implemented in JS" {file: "/mnt/dan/firefox.git/obj-powerpc64le-unknown-linux-gnu/_tests/xpcshell/js/xpconnect/tests/unit/test_bug809674.js" line: 43}]"
 0:02.33 pid:8170 nsStringStats
 0:02.33 pid:8170  => mAllocCount:           6166
 0:02.33 pid:8170  => mReallocCount:            0
 0:02.33 pid:8170  => mFreeCount:            6166
 0:02.33 pid:8170  => mShareCount:           9747
 0:02.33 pid:8170  => mAdoptCount:            125
 0:02.33 pid:8170  => mAdoptFreeCount:        125
 0:02.33 pid:8170  => Process ID: 8170, Thread ID: 140735374378592
 0:02.33 TEST_END: Test PASS. Subtests passed 16/16. Unexpected 0
 0:02.34 INFO INFO | Result summary:
 0:02.35 INFO INFO | Passed: 1
 0:02.35 INFO INFO | Failed: 0
 0:02.35 INFO INFO | Todo: 0
 0:02.35 INFO INFO | Retried: 0
 0:02.35 SUITE_END
 0:02.35 
Overall Summary
===============

xpcshell
~~~~~~~~
Ran 17 checks (1 tests, 16 subtests)
Expected results: 17
OK

Comment 19

[Dan Horák]

it's passing even with a specific float test

diff --git a/js/xpconnect/tests/unit/test_bug809674.js b/js/xpconnect/tests/unit/test_bug809674.js
index d6c386fdf2..f101f84e6b 100644
--- a/js/xpconnect/tests/unit/test_bug809674.js
+++ b/js/xpconnect/tests/unit/test_bug809674.js
@@ -22,6 +22,7 @@ function run_test() {
   Assert.equal(o.addVals("foo", "x"), "foox");
   Assert.equal(o.addVals("foo", 1.2), "foo1.2");
   Assert.equal(o.addVals(1234, "foo"), "1234foo");
+  Assert.equal(o.addVals(1.2, 3.4), 4.6);
 
   Assert.equal(o.addMany(1, 2, 4, 8, 16, 32, 64, 128), 255);
 
and with "./mach test js/xpconnect/tests/unit" all unit tests pass except test_file2.js

Comment 20

[Dan Horák]

The raw value, that invokes the assert, is 0xfff9800000000000 and it gives following in gdb

$12 = {asBits_ = 18444914486360932352, asDouble_ = -nan(0x9800000000000), debugView_ = {payload47_ = 0, tag_ = JSVAL_TAG_UNDEFINED}, s_ = {payload_ = {i32_ = 0, u32_ = 0, 
      why_ = JS_ELEMENTS_HOLE}}}

Looks to me as an "empty" value is being passed thru the call stack, but interpreted as double at the top.

Comment 21

[Cameron Kaiser [:spectre]]

Okay, I have a reproducing build up on my Talos. My initial theory in comment 5 is incorrect because the signal handler being hit is the SIGSEGV from the assert, not from the stack protection. It's asserting because of what Dan points out in comment 20, that it's a bogus double. I'm looking at XPConnect first.

Comment 22

[Cameron Kaiser [:spectre]]

It's not barfing on the JSVAL_TAG_UNDEFINED. It's barfing on a different value:

#0  0x00007fffe75875e4 in JS::Value::setDouble(double)
    (d=<optimized out>, this=<optimized out>)
    at /home/spectre/src/mozilla-central/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:445
#1  0x00007fffe75875e4 in JS::Value::setNumber(double)
    (this=<optimized out>, d=<optimized out>)
    at /home/spectre/src/mozilla-central/obj-powerpc64le-unknown-linux-gnu/dist/include/js/Value.h:517
#2  0x00007fffe757f168 in js::MutableWrappedPtrOperations<JS::Value, JS::MutableHandle<JS::Value> >::setNumber(double) (d=<optimized out>, this=0x7fffffff7d68)
    at /home/spectre/src/mozilla-central/obj-powerpc64le-unknown-linux-gnu/dist/include/js/RootingAPI.h:632
(More stack frames follow...)
(gdb) up 3
#3  XPCConvert::NativeData2JS (d=..., s=0x7fffffff7fd8, type=..., 
    iid=0x7fffffff7e78, arrlen=<optimized out>, pErr=0x7fffffff7e3c)
    at /home/spectre/src/mozilla-central/js/xpconnect/src/XPCConvert.cpp:142
142	      d.setNumber(*static_cast<const float*>(s));
(gdb) x/16x (char *)s
0x7fffffff7fd8:	0x00	0x00	0xcc	0xff	0x00	0x00	0x00	0x00
0x7fffffff7fe0:	0x00	0x00	0x00	0x00	0x00	0x00	0x00	0x00
(gdb) p type
$1 = (const nsXPTType &) @0x7ffff0dc4d82: {static T_I8 = TD_INT8, 
  static T_I16 = TD_INT16, static T_I32 = TD_INT32, static T_I64 = TD_INT64, 
  static T_U8 = TD_UINT8, static T_U16 = TD_UINT16, static T_U32 = TD_UINT32, 
  static T_U64 = TD_UINT64, static T_FLOAT = TD_FLOAT, 
  static T_DOUBLE = TD_DOUBLE, static T_BOOL = TD_BOOL, 
  static T_CHAR = TD_CHAR, static T_WCHAR = TD_WCHAR, static T_VOID = TD_VOID, 
  static T_IID = TD_PNSIID, static T_CHAR_STR = TD_PSTRING, 
  static T_WCHAR_STR = TD_PWSTRING, static T_INTERFACE = TD_INTERFACE_TYPE, 
  static T_INTERFACE_IS = TD_INTERFACE_IS_TYPE, 
  static T_LEGACY_ARRAY = TD_LEGACY_ARRAY, 
  static T_PSTRING_SIZE_IS = TD_PSTRING_SIZE_IS, 
  static T_PWSTRING_SIZE_IS = TD_PWSTRING_SIZE_IS, 
  static T_UTF8STRING = TD_UTF8STRING, static T_CSTRING = TD_CSTRING, 
  static T_ASTRING = TD_ASTRING, static T_JSVAL = TD_JSVAL, 
  static T_DOMOBJECT = TD_DOMOBJECT, static T_PROMISE = TD_PROMISE, 
  static T_ARRAY = TD_ARRAY, mTag = 8 '\b', mInParam = 0 '\000', 
  mOutParam = 1 '\001', mOptionalParam = 0 '\000', mData1 = 0 '\000', 
  mData2 = 0 '\000'}
(gdb) 

So the value, which should be a jsval 0xffcc00000000 (IIRC JSVAL_TAG_OBJET), is getting truncated and ends up a float (tag of 8).

Comment 23

[Cameron Kaiser [:spectre]]

This smells like an endian issue which got unmasked, by the way. Both Dan and my system are running Fedora in LE. :awilfox, Adelie is BE, correct?

Comment 24

[Cameron Kaiser [:spectre]]

This wallpapers the issue, for the record. It is not the correct fix.

diff -r e766954700c7 js/xpconnect/src/XPCConvert.cpp
--- a/js/xpconnect/src/XPCConvert.cpp   Fri Dec 07 11:29:48 2018 +0200
+++ b/js/xpconnect/src/XPCConvert.cpp   Sat Dec 08 16:36:47 2018 -0800
@@ -100,16 +100,24 @@
                                uint32_t arrlen, nsresult* pErr) {
   MOZ_ASSERT(s, "bad param");
 
   AutoJSContext cx;
   if (pErr) {
     *pErr = NS_ERROR_XPC_BAD_CONVERT_NATIVE;
   }
 
+if (*((uint32_t *)s) == 0xffcc0000) {
+  if (type.Tag() == nsXPTType::T_FLOAT) {
+      *(uint64_t *)s = 0xffcc000000000000;
+      d.set(*static_cast<const Value*>(s));
+      return JS_WrapValue(cx, d);
+  }
+}
+
   switch (type.Tag()) {
     case nsXPTType::T_I8:
       d.setInt32(*static_cast<const int8_t*>(s));
       return true;
     case nsXPTType::T_I16:
       d.setInt32(*static_cast<const int16_t*>(s));
       return true;
     case nsXPTType::T_I32:

Comment 25

[A. Wilcox [:awilfox]]

(In reply to Cameron Kaiser [:spectre] from comment #23)
> This smells like an endian issue which got unmasked, by the way. Both Dan
> and my system are running Fedora in LE. :awilfox, Adelie is BE, correct?

Adélie is big-endian only, that is correct.  It's weird to see an endianness bug in the other direction!  Not sure it matters any, but the last endian bug I saw in js/ was bug 1499277.  Maybe feel around there and see if there's anything lingering I missed.

Comment 26

[Cameron Kaiser [:spectre]]

:bholley, if you're not a good choice to ask please advise, but it looks like the defective value should be a 64-bit int from the perspective of XPConnect. Where's somewhere to start debugging why it seems like we get different types for that value? How can I verify what the "correct" XPT type is?

Comment 27

[Bobby Holley (:bholley)]

(In reply to Cameron Kaiser [:spectre] from comment #26)
> :bholley, if you're not a good choice to ask please advise, but it looks
> like the defective value should be a 64-bit int from the perspective of
> XPConnect.

Totally willing to believe we have endian-ness issues, since I don't think we have any tier-1 big-endian platforms right now. Could also be something melse.

> Where's somewhere to start debugging why it seems like we get
> different types for that value?

In general, I would suggest tracing the value back to its source and finding the point where it becomes what you don't expect. rr makes this extra easy, though not sure if rr works on ppc. Adding a bunch of logging is a poor-man's approach. 

> How can I verify what the "correct" XPT type
> is?

I'd suggest walking up a stack frame or two to find the nsXPTMethodInfo, figuring out what method/interface this corresponds to, and checking the corresponding IDL.

Good luck!

Comment 28

[A. Wilcox [:awilfox]]

(In reply to Bobby Holley (:bholley) from comment #27)
> Totally willing to believe we have endian-ness issues, since I don't think
> we have any tier-1 big-endian platforms right now. Could also be something
> melse.

We don't, BE is all Tier 3.  But this is an issue on ppc64el, which is the wrong-way-round PowerPC (i.e. little endian).  I'm not seeing this bug on ppc64 (big endian).

> rr makes this extra easy,
> though not sure if rr works on ppc. Adding a bunch of logging is a
> poor-man's approach. 

Unfortunately, rr requires x86 internals.  Even an ARM port is blue-skyed at this point, per GitHub issues.

Comment 29

[Cameron Kaiser [:spectre]]

Attachment: Work around miscompilation

Okay, after much gritting of teeth, I think we are indeed looking at a miscompilation. It looks like the switch statement gets compiled wrong on ppc64le with this combination of options.

For some reason hoisting the I64 path out of the switch gets around the problem, at least on my system. Dan, does this work for you? If it does, it's NPOTB, just runs what the code should have run and shouldn't regress big-endian, so I think it would be safe to commit.

Comment 30

[Dan Horák]

I don't know what is different in my builds, but they are still crashing with the workaround applied.

Comment 31

[Dan Horák]

And regarding big-endian - there is a big difference in the compiler setup between Fedora, which uses -mcpu=power8, and Adelie, which goes with the default (970/power4 AFAIK).

Comment 32

[A. Wilcox [:awilfox]]

From our ppc64 builder:

   # -O2           -> Perform second-level optimisations.
   #                  Not -Os because PPC64 machines aren't starved for space.
   # -ggdb         -> Generate GDB debugging information.
   #                  This is used with splitdebug to make -dbg split packages.
   # -mcpu=970     -> Require a Power4+ (equivalent) "970" or higher CPU.
   # -mtune=power9 -> Tune for Power9 machines.
   # -maltivec     -> Always enable AltiVec.
   #                  AltiVec extensions are available on every CPU we target.
   # -mlong-dou... -> Ensure musl ABI is followed.
   # -fno-inlin... -> Required to work around bug in GCC 6.3 that causes doubles
   #                  to break in libffi.
   export CFLAGS="-O2 -ggdb -mcpu=970 -mtune=power9 -maltivec -mlong-double-64 -fno-inline-small-functions"

This is of course used for CXXFLAGS too.  Note we're also using GCC 6.4, not 7/8.  :spectre can you show gcc -v and what CFLAGS/CXXFLAGS you are using (if any custom, i.e. --enable-optimize="")?  Dan, same?

Comment 33

[Cameron Kaiser [:spectre]]

I screwed up. This was actually my error, I ran it against the wrong test build. For the record, I'm building with the Fedora default "gcc version 8.2.1 20181105 (Red Hat 8.2.1-5) (GCC)," and this is the .mozconfig:

export CC=/usr/bin/gcc
export CXX=/usr/bin/g++

mk_add_options MOZ_MAKE_FLAGS="-j24"
ac_add_options --enable-application=browser
ac_add_options --enable-optimize="-Og -mcpu=power9"
ac_add_options --enable-debug
ac_add_options --disable-release
ac_add_options --enable-linker=bfd

export GN=/home/spectre/bin/gn
export RUSTC_OPT_LEVEL=0

Comment 34

[Cameron Kaiser [:spectre]]

I'm completely wrong on all my theories. I took :bholley's suggestion and dumped out the nsXPTMethodInfo. It turns out it's looking for a float pref, so the type is correct. The float pref is returned as 0x00000000, but somewhere after the invocation it gets stomped into 0xffcc0000, which is not a float.

I still think it's incorrect code generation. :dan, does *this* fix it? Don't ask how I found this.

diff -r e766954700c7 js/xpconnect/src/XPCWrappedNative.cpp
--- a/js/xpconnect/src/XPCWrappedNative.cpp     Fri Dec 07 11:29:48 2018 +0200
+++ b/js/xpconnect/src/XPCWrappedNative.cpp     Tue Dec 11 22:24:20 2018 -0800
@@ -1199,16 +1199,17 @@
   if (!ConvertIndependentParams(&foundDependentParam)) {
     return false;
   }
 
   if (foundDependentParam && !ConvertDependentParams()) {
     return false;
   }
 
+fprintf(stderr, "foo\n");
   mInvokeResult = Invoke();
 
   if (JS_IsExceptionPending(mCallContext)) {
     return false;
   }
 
   if (NS_FAILED(mInvokeResult)) {
     ThrowBadResult(mInvokeResult, mCallContext);

It seems to on mine (yes, it's spammy). I need to see what exactly the fprintf() is fixing as a side effect, but I think the generated code is munging the nsXPTCVariant on the stack because when I examine that memory without this extra fprintf(), it's already corrupt. I wonder if padding the nsXPTCVariant would help.

Comment 35

[Dan Horák]

So the fprintf() does "fix" the problem, as does the following (-O1 still crashes), which is a sign (not a proof) of a miscompilation

diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp
index 4e117fe27e..4191785bf6 100644
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1173,6 +1173,7 @@ bool XPCWrappedNative::CallMethod(XPCCallContext& ccx,
   return helper.get().Call();
 }
 
+__attribute__((optimize(("O0"))))
 bool CallMethodHelper::Call() {
   mCallContext.SetRetVal(JS::UndefinedValue());

Comment 36

[Dan Horák]

BTW is it possible to disable the "jumbo-build" feature when multiple sources under js/ are merged together (as the "Unified_*.cpp" files) before sending to the compiler?

Comment 37

[Cameron Kaiser [:spectre]]

This is just nuts. It actually got corrupted by the operating system!

Thread 1 "firefox" hit Breakpoint 1, 0x00007fffe618b4c4 in nsPrefBranch::GetFloatPref (this=0x7fffc111cf00, 
    aPrefName=0x7fffc6ad1fe0 "autoFill.stddevMultiplier", 
    aRetVal=aRetVal@entry=0x7fffffff7fd8)
    at /home/spectre/src/mozilla-central/modules/libpref/Preferences.cpp:2139
[...]
(gdb) watch *0x7fffffff7fd8
Watchpoint 2: *0x7fffffff7fd8
(gdb) cont
Continuing.
[Switching to Thread 0x7fffd455f150 (LWP 23172)]

Thread 15 "Timer" hit Watchpoint 2: *0x7fffffff7fd8

Old value = 0
New value = -3407872
__do_get_tspec () at arch/powerpc/kernel/vdso64/gettimeofday.S:267
267	arch/powerpc/kernel/vdso64/gettimeofday.S: No such file or directory.
(gdb) x/16x 0x7fffffff7fd8
0x7fffffff7fd8:	0x00	0x00	0xcc	0xff	0x00	0x00	0x00	0x00
0x7fffffff7fe0:	0x00	0x00	0x00	0x00	0x00	0x00	0x00	0x00

I think a better solution is just to disable stack protection for that portion of XPConnect only. Patch coming up (I'm testing a couple things first).

Comment 38

[A. Wilcox [:awilfox]]

That explains a lot about why we didn't see it in Adélie: musl doesn't use vDSO on PowerPC (32 nor 64).

Comment 39

[Cameron Kaiser [:spectre]]

I'm not convinced that's the whole story. For example, I just tested my proposed fix in an opt build, but it didn't occur there even without the patch. It could be that vDSO interacts badly with stack protection but you'd expect to see that occur other places. Anyway, the simplest solution is indeed to turn off stack protection for the iffy part of XPConnect. Trying to decorate specific functions didn't work because there's a lot of weird interplay so I disabled it for the XPCWrappedNative module and that fixes the issue on debug without regressing opt. Here it comes.

Comment 40

[Cameron Kaiser [:spectre]]

Attachment: Disable stack protection for XPCWrappedNative on ppc64le

As per the previous comment this disables stack protection for the affected portion. This patch is very very very very very very very targetted to gcc on linux on ppc64le, so it's NPOTB for everything else including tier-1.

:bholley, if you're the wrong person to review this, please advise. Thank you!

Comment 41

[Cameron Kaiser [:spectre]]

Thank you kindly!

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c892a6147ae
Disable stack protection for a portion of XPConnect on ppc64le due to a compiler bug. r=bholley

Comment 43

[Dan Horák]

Attachment: 0001-Bug-1512162-Disable-stack-protection-for-a-portion-o.patch

Good job, Cameron.

I would add the push_options/pop_options pragmas, because the XPCWrappedNative.cpp file is compiled as a part of a bigger "unified" source file and thus the stack protection would be disabled for the next files too (https://hg.mozilla.org/integration/mozilla-inbound/file/5c892a6147ae/js/xpconnect/src/moz.build#l39)

Comment 44

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/5c892a6147ae

Comment 45

[Luke-Jr]

Where are the upstream GCC (or Linux?) bug(s) for this issue?

Comment 46

[Dan Horák]

There isn't one yet, it can still be a real bug somewhere in FF.

Comment 47

[Cameron Kaiser [:spectre]]

(In reply to Dan Horák from comment #43)
> I would add the push_options/pop_options pragmas, because the
> XPCWrappedNative.cpp file is compiled as a part of a bigger "unified" source
> file and thus the stack protection would be disabled for the next files too
> (https://hg.mozilla.org/integration/mozilla-inbound/file/5c892a6147ae/js/
> xpconnect/src/moz.build#l39)

That's a good idea. Followup patch coming up.

Comment 48

[Cameron Kaiser [:spectre]]

Attachment: Narrow the non-stack-protected window on ppc64le

Per Dan's suggestion, here is a followup that narrows the non-stack-protected window in XPConnect for ppc64le. To avoid the same preprocessor logic in two places, this just sets a flag so the two halves don't get out of sync. Again, narrowly focused to only the affected platform, so NPOTB for everything else including tier-1.

Comment 49

[Neha Kochar [:neha]]

Priority down to P3 as it affects only tier-3 platforms - thanks :bholley

Comment 50

[Cameron Kaiser [:spectre]]

Thanks again for the review!

Comment 51

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1512162: update incognito schema and support system addons

aswan for startupData changes.  rpl for everything.

Comment 52

[Shane Caraveo (:mixedpuppy)]

I've no idea how I got a bug number so wrong...trying to undo this.

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5ff59b1aaeb
Followup: narrow non-stack-protected window for XPConnect on ppc64le. r=bhollley

Comment 54

[Cosmin Sabou [:CosminS]]

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

Comment 55

[Cameron Kaiser [:spectre]]

Comment on attachment 9030976 [details] [diff] [review]
Disable stack protection for XPCWrappedNative on ppc64le

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1503589

User impact if declined: ppc64le systems may crash depending on compiler optimization flags as soon as any characters are entered into the URL bar.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): NPOTB for tier-1 builds.

String changes made/needed: none

Comment 56

[Cameron Kaiser [:spectre]]

Comment on attachment 9031206 [details] [diff] [review]
Narrow the non-stack-protected window on ppc64le

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1503589

User impact if declined: Same as above.

Is this code covered by automated tests?: Yes

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): NPOTB for tier-1 (same as above).

String changes made/needed: none

Comment 57

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9030976 [details] [diff] [review]
Disable stack protection for XPCWrappedNative on ppc64le

[Triage Comment]
NPOTB for Tier-1 platforms, approved for 65.0b5.

Comment 58

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e1ec36f44f73
https://hg.mozilla.org/releases/mozilla-beta/rev/bc01e3051105

Comment 59

[Cameron Kaiser [:spectre]]

Looks like this has surfaced again on trunk (confirmed by both Dan and I).

Comment 60

[Cameron Kaiser [:spectre]]

Okay, I think I have a better understanding of what's going on. The stack protector business was just a wallpaper, and the vDSO was actually an artifact of gdb. When I did some manual code stepping, this appeared:

Thread 1 "firefox" hit Breakpoint 1, 0x00007fffe5d50dec in nsPrefBranch::GetFloatPref (this=0x7fffcbfbb760, 
    aPrefName=0x7fffd286a740 "autoFill.stddevMultiplier", 
    aRetVal=aRetVal@entry=0x7fffffff8090)
    at /home/spectre/src/mozilla-central/modules/libpref/Preferences.cpp:2139
2139	nsPrefBranch::GetFloatPref(const char* aPrefName, float* aRetVal) {
(gdb) print $f31
$1 = -nan(0x9800000000000)
(gdb) b *0x7fffe5d50f90
Breakpoint 2 at 0x7fffe5d50f90: file /home/spectre/src/mozilla-central/obj-powerpc64le-unknown-linux-gnu/dist/include/nsError.h, line 30.
(gdb) cont
Continuing.

Thread 1 "firefox" hit Breakpoint 2, 0x00007fffe5d50f90 in nsPrefBranch::GetFloatPrefWithDefault (this=<optimized out>, aPrefName=<optimized out>, 
    aDefaultValue=nan(0x1), aArgc=<optimized out>, aRetVal=0x7fffffff8090)
    at /home/spectre/src/mozilla-central/obj-powerpc64le-unknown-linux-gnu/dist/include/nsError.h:30
30	  return static_cast<uint32_t>(aErr) & 0x80000000;
(gdb) i reg f31
f31            -nan(0x9800000000000) (raw 0xfff9800000000000)
(gdb) si
2131	    *aRetVal = aDefaultValue;
(gdb) i reg f31
f31            -nan(0x9800000000000) (raw 0xfff9800000000000)
(gdb) x/i $pc
=> 0x7fffe5d50f94 <nsPrefBranch::GetFloatPrefWithDefault(char const*, float, unsigned char, float*)+108>:	stfs    f31,0(r30)
(gdb) i reg r30
r30            0x7fffffff8090      140737488322704
(gdb) x/16b 0x7fffffff8090
0x7fffffff8090:	0x00	0x00	0x00	0x00	0x00	0x00	0x00	0x00
0x7fffffff8098:	0x00	0x00	0x00	0x00	0x00	0x00	0x00	0x00
(gdb) i reg f31
f31            -nan(0x9800000000000) (raw 0xfff9800000000000)
(gdb) si
2132	    return NS_OK;
(gdb) x/16b 0x7fffffff8090
0x7fffffff8090:	0x00	0x00	0xcc	0xff	0x00	0x00	0x00	0x00
0x7fffffff8098:	0x00	0x00	0x00	0x00	0x00	0x00	0x00	0x00
(gdb) i reg f31
f31            -nan(0x9800000000000) (raw 0xfff9800000000000)
(gdb) i reg r30
r30            0x7fffffff8090      140737488322704

It's not the vDSO stomping on the retval, it's actually the preferences module writing a bogus 32-bit float. The bogus float that ends up in f31 is aDefaultValue, which has a starting value that is NaN. However, in browser/components/urlbar/UrlbarPrefs.jsm, the default value should be 0.0, based on this line (["autoFill.stddevMultiplier", [0.0, "getFloatPref"]],). So this is indeed XPConnect and it's something wrong with our invocation routine.

Comment 61

[Cameron Kaiser [:spectre]]

If I read this right, the value appears to already be abnormal coming from JS to native. Debugging further is slow going since POWER9 hardware watchpoints are currently disabled due to a kernel problem. :bholley, do you think this is still XPConnect, or might this be a problem with how jsvals are dealt with? The value should be 0.0.

Thread 1 "firefox" hit Breakpoint 1, XPCConvert::JSData2Native (
    cx=<optimized out>, d=0x7fffffff92e8, s=..., type=..., iid=0x7fffffff91e8, 
    arrlen=<optimized out>, pErr=<optimized out>)
    at /home/spectre/src/mozilla-central/js/xpconnect/src/XPCConvert.cpp:496
496	      return ConvertToPrimitive(cx, s, static_cast<float*>(d));
(gdb) p type
$7 = (const nsXPTType &) @0x7ffff0d291a4: {static T_I8 = TD_INT8, 
  static T_I16 = TD_INT16, static T_I32 = TD_INT32, static T_I64 = TD_INT64, 
  static T_U8 = TD_UINT8, static T_U16 = TD_UINT16, static T_U32 = TD_UINT32, 
  static T_U64 = TD_UINT64, static T_FLOAT = TD_FLOAT, 
  static T_DOUBLE = TD_DOUBLE, static T_BOOL = TD_BOOL, 
  static T_CHAR = TD_CHAR, static T_WCHAR = TD_WCHAR, static T_VOID = TD_VOID, 
  static T_NSIDPTR = TD_NSIDPTR, static T_CHAR_STR = TD_PSTRING, 
  static T_WCHAR_STR = TD_PWSTRING, static T_INTERFACE = TD_INTERFACE_TYPE, 
  static T_INTERFACE_IS = TD_INTERFACE_IS_TYPE, 
  static T_LEGACY_ARRAY = TD_LEGACY_ARRAY, 
  static T_PSTRING_SIZE_IS = TD_PSTRING_SIZE_IS, 
  static T_PWSTRING_SIZE_IS = TD_PWSTRING_SIZE_IS, 
  static T_UTF8STRING = TD_UTF8STRING, static T_CSTRING = TD_CSTRING, 
  static T_ASTRING = TD_ASTRING, static T_NSID = TD_NSID, 
  static T_JSVAL = TD_JSVAL, static T_DOMOBJECT = TD_DOMOBJECT, 
  static T_PROMISE = TD_PROMISE, static T_ARRAY = TD_ARRAY, mTag = 8 '\b', 
  mInParam = 1 '\001', mOutParam = 0 '\000', mOptionalParam = 0 '\000', 
  mData1 = 0 '\000', mData2 = 0 '\000'}
(gdb) p s
$8 = {<js::HandleBase<JS::Value, JS::Handle<JS::Value> >> = {<js::WrappedPtrOperations<JS::Value, JS::Handle<JS::Value> >> = {<No data fields>}, <No data fields>}, ptr = 0x7fffffff91e0}
(gdb) x/f s.ptr
0x7fffffff91e0:	-nan(0x8800000000001)
(gdb) x/gx s.ptr
0x7fffffff91e0:	0xfff8800000000001
(gdb) x/8bx s.ptr
0x7fffffff91e0:	0x01	0x00	0x00	0x00	0x00	0x80	0xf8	0xff

Comment 62

[Timothy Pearson]

I wanted to reach out in response to https://www.talospace.com/2019/02/raptor-help-us-out-here-on-software-side.html

As far as I know the hardware watchpoints can be reenabled by reversing this patch [1] in the kernel. For a development machine this is reasonable, but if you're using the machine for other purposes as well you may or may not want to do that.

We could set you up with a large POWER8 VM to get you unblocked while the POWER9 fixes are being worked on as well.

We do want to support Firefox on POWER; I had no idea the JIT effort was stalled on this. As a bit of background, at least from our perspective, way back at the beginning there were some issues getting underlying pieces to work (IIRC Rust related problems) and there was some initial pushback against adding support for the ppc64 systems at that point. While that has since been cleared, I remain somewhat concerned that as a Tier 3 platform with (at least by my current understanding) no room for promotion a single x86-centric Rust or related low level change could disable the entire port pretty much overnight. If there is some safeguard against this (or even a possible path to promotion above Tier 3 over time), it will be easier for me to free up developer resources to assist.

If you ever need to reach out to me directly, you can contact me via the Email address on this Bugzilla account. Again we very much want to see Firefox on POWER with full JIT; it's quite possible we misread the actual upstream community support for the port.

[1] https://www.spinics.net/lists/kvm-ppc/msg13543.html

Comment 63

[Bobby Holley (:bholley)]

It seems plausible that xpconnect/xptcall is getting the conversion wrong somehow, but I can't say much offhand.

Assuming this reproduces reliably, I'd recommend generating a minimal testcase and logging the value as it travels across the marshalling routines. Ideally you'd do this with a single method invocation in an xpcshell testcase, but if that doesn't work you could make your logging happen only for the particular method call you care about.

Comment 64

[A. Wilcox [:awilfox]]

This discussion should likely be taking place somewhere else, but I wanted to clarify as someone who has been working on those underlying Rust related pieces:

The Rust community is quite amenable to POWER, and PPC64 both BE and LE are Tier 2 per [https://forge.rust-lang.org/platform-support.html](the platform support page). This means any change must build on PPC64 or it will not be accepted. The main difference between tier 1 and tier 2 is that tier 2 does not necessarily pass all of its test suite. And I can confirm that locally, even powerpc64-linux-musl is passing its test suite fully (100%, 0 failures).

Firefox's platform support is much different. Tier 3 means that the community is the maintainer. As long as patches do not break tier1 (which is currently x86 and ARM), you can push through whatever changes necessary to keep the browser functioning. There is a MIPS JIT in central right now, and I can assure you MIPS has absolutely no tier 1 support nor do I envision it gaining such any time soon.

I'm still hoping to land a PPC64 BE JIT at some point, cribbing what I can off TenFourFox, but I need gfx working properly first. And that's why I've been trying to convince the gfx team to help me on weekends / when they're free to get it working. I can't work on a JIT while I am fighting the browser to just stay alive because of idiotic Skia assertions.

Comment 65

[Timothy Pearson]

(In reply to A. Wilcox [:awilfox] from comment #64)

This discussion should likely be taking place somewhere else

<snip>

Agreed, I just figured that it would be good to at least address the immediate concerns blocking this bug report. I'll reach out via Email to both of you; if anyone else wants to be added to the off-tracker discussion just let me know via Email.

Comment 66

[Cameron Kaiser [:spectre]]

Getting back on topic, the possibility of a compiler bug is cropping up again. It's not the stack protector; building the entire browser without the stack protector didn't fix it. However, while stumbling around with printfs, this did:

diff -r 7ab4a0c9980f js/xpconnect/src/XPCWrappedNative.cpp
--- a/js/xpconnect/src/XPCWrappedNative.cpp     Sat Feb 16 11:36:46 2019 +0200
+++ b/js/xpconnect/src/XPCWrappedNative.cpp     Tue Feb 26 21:30:14 2019 -0800
@@ -1630,16 +1630,20 @@
 
   return true;
 }
 
 nsresult CallMethodHelper::Invoke() {
   uint32_t argc = mDispatchParams.Length();
   nsXPTCVariant* argv = mDispatchParams.Elements();
 
+if (mVTableIndex == 8 && argv[0].type == 15 && argv[1].type == 8) {
+  fprintf(stderr, "HIT!!!\n");
+}
+
   return NS_InvokeByIndex(mCallee, mVTableIndex, argc, argv);
 }
 
 static void TraceParam(JSTracer* aTrc, void* aVal, const nsXPTType& aType,
                        uint32_t aArrayLen = 0) {
   if (aType.Tag() == nsXPTType::T_JSVAL) {
     JS::UnsafeTraceRoot(aTrc, (JS::Value*)aVal,
                         "XPCWrappedNative::CallMethod param");

Dan, confirm, but your deoptimization from comment 35 also seems to fix the glitch, so we're back to suspecting the compiler.

I'm trying to see if a more targetted deoptimization in the call stack will still yield good performance. Either way, I'm just going to yank my stack protector patch at the same time since all it did was probably tickle the compiler somewhere else.

Comment 67

[Cameron Kaiser [:spectre]]

Actually, it looks like just backing out the earlier patch is sufficient to fix the problem. I still don't know what we're wallpapering here, but since it's clearly not the right fix anyway, I guess that's the simplest approach for now. I'm going to try a couple different kinds of build configurations and if it sticks, I'll submit a backout.

Comment 68

[Cameron Kaiser [:spectre]]

Backing it out seems to fix things on current trunk in debug and opt. I think that if this pops up again, we should look at the intersection between -O0 and -Og for either CallMethodHelper::Call() or CallMethodHelper::Invoke() (because deoptimizing that also fixed the problem, at least in this manifestation). I'm not sure what we're tickling in the compiler yet, but this is clearly not the right fix and it was only a temporary measure anyway, so let's take it out.

Comment 69

[Cameron Kaiser [:spectre]]

Attachment: Backout stack protection disable for ppc64le in XPConnect

It's outlived its usefulness.

Comment 70

[Cameron Kaiser [:spectre]]

Thanks!

Comment 71

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e097bf9457c3
Reenable stack protection for ppc64le in XPConnect. r=bholley

Comment 72

[Raul Gurzau (:RaulG)]

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

Comment 73

[Cameron Kaiser [:spectre]]

Damn it, this bug is back again in my weekly smoketest build.

Comment 74

[Cameron Kaiser [:spectre]]

The difference between -Og and -O0 is __attribute__((optimize(("auto-inc-dec combine-stack-adjustments compare-elim cprop-registers dce defer-pop dse forward-propagate guess-branch-probability ipa-profile ipa-pure-const ipa-reference ipa-reference-addressable merge-constants omit-frame-pointer reorder-blocks shrink-wrap shrink-wrap-separate split-wide-types ssa-backprop tree-ccp tree-ch tree-coalesce-vars tree-copy-prop tree-dce tree-dominator-opts tree-dse tree-forwprop tree-fre tree-phiprop tree-scev-cprop tree-sink tree-slsr tree-ter unit-at-a-time")))). We'll just go one at a time through them until it works.

Comment 75

[Cameron Kaiser [:spectre]]

It must be a non-toggleable option. Flipping all those off or in various combinations doesn't fix the problem; only Dan's deoptimization from comment 35 seems to (still) fix it. I think that's what we'll have to do, specifically for ppc64le gcc. I'll check a couple other things first and then get a patch ready.

Comment 76

[Cameron Kaiser [:spectre]]

I found something better. I'm not sure if this is just spreading the badness around rather than fixing it, but removing two particular optimizations seems to undo the crash. Dan, since you're already needinfoed, instead of __attribute__((optimize(("O0")))) as you have in comment 35, does decorating CallMethodHelper::Call() with __attribute__((optimize(("no-graphite-identity,no-guess-branch-probability")))) fix it for you as well?

I'm going to disassemble the function both ways and see if I can narrow it down further, but this could be a stopgap solution.

Comment 77

[Cameron Kaiser [:spectre]]

The main difference between -Og and -Og -fno-graphite-identity -fno-guess-branch-probability is that the function is not only not inlined, but function calls within it are also not inlined. __attribute__((noinline)) thus didn't work, but __attribute__((noinline,optimize(("no-guess-branch-probability")))) does. Dan, do you confirm?

I'm thinking this would be a better choice, since I'd like as little messing around with the optimizer as possible.

Comment 78

[Dan Horák]

yup, no crash after applying

diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp
index d6f672a8df..367d554792 100644
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1157,6 +1157,7 @@ bool XPCWrappedNative::CallMethod(XPCCallContext& ccx,
   return helper.get().Call();
 }
 
+__attribute__((noinline,optimize(("no-guess-branch-probability"))))
 bool CallMethodHelper::Call() {
   mCallContext.SetRetVal(JS::UndefinedValue());
 

Comment 79

[Dan Horák]

Seems most of the CallMethodHelper's methods are declared with MOZ_ALWAYS_INLINE, so a buggy inlining might be the issue. Because the following helps too

diff --git a/js/xpconnect/src/XPCWrappedNative.cpp b/js/xpconnect/src/XPCWrappedNative.cpp
index d6f672a8df..1ce28899d1 100644
--- a/js/xpconnect/src/XPCWrappedNative.cpp
+++ b/js/xpconnect/src/XPCWrappedNative.cpp
@@ -1157,6 +1157,8 @@ bool XPCWrappedNative::CallMethod(XPCCallContext& ccx,
   return helper.get().Call();
 }
 
+__attribute__((noinline,noclone))
 bool CallMethodHelper::Call() {
   mCallContext.SetRetVal(JS::UndefinedValue());
 
@@ -1315,6 +1317,7 @@ bool CallMethodHelper::GetOutParamSource(uint8_t paramIndex,
   return true;
 }
 
+__attribute__((noinline,noclone))
 bool CallMethodHelper::GatherAndConvertResults() {
   // now we iterate through the native params to gather and convert results
   uint8_t paramCount = mMethodInfo->GetParamCount();

The "noinline,noclone" is a recommendation I got from the gcc people some time ago when looking at possible miscompilations.

Comment 80

[Cameron Kaiser [:spectre]]

I think I like noinline,noclone better than dorking around with optimization options. I'll prep that as a patch.

Comment 81

[Cameron Kaiser [:spectre]]

Attachment: Use targeted attributes instead to deal with incorrect inlining

Tested working at multiple optimization levels. This is a better approach than to disable the stack protector and is probably actually dealing with the real problem, which appears to be an edge case with inlining in the compiler. Uses same conditions as before to be NPOTB.

Comment 82

[Cameron Kaiser [:spectre]]

Attachment: Bug 1512162 - A better fix for incorrect code generation on ppc64le. r?bholley

Comment 83

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 84

[Cameron Kaiser [:spectre]]

Thanks!

I don't know why Lando won't let me create a sign on (I can't seem to link it to my bugzilla account, everything else I try requires 2FA and it won't tell me how to do that), so I'll just set checkin-needed.

Comment 85

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a2ce7a5ce7d
A better fix for incorrect code generation on ppc64le. r=bholley

Comment 86

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/1a2ce7a5ce7d

Comment 87

[BugBot [:suhaib / :marco/ :calixte]]

Change the status for beta to have the same as nightly and release.
For more information, please visit auto_nag documentation.

Comment 88

[Cameron Kaiser [:spectre]]

Comment on attachment 9075022 [details]
Bug 1512162 - A better fix for incorrect code generation on ppc64le. r?bholley

Beta/Release Uplift Approval Request

• User impact if declined: Crashes depending on the optimization level when any URL is typed on ppc64le.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Patch is NPOTB (conditions are the same as before).
• String changes made/needed:

Comment 89

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9075021 [details] [diff] [review]
Use targeted attributes instead to deal with incorrect inlining

68 is in RC at this point, so moving the requests accordingly. I'm guessing this might be more relevant for ESR68 than release 68 too?

Comment 90

[Cameron Kaiser [:spectre]]

Yes, please (both release and ESR). That would be very helpful. Since it's NPOTB, it would not require a respin.

Comment 91

[Julien Cristau [:jcristau]]

I don't think this warrants uplift to release.

Comment 92

[Cameron Kaiser [:spectre]]

Comment on attachment 9075022 [details]
Bug 1512162 - A better fix for incorrect code generation on ppc64le. r?bholley

Beta/Release Uplift Approval Request

• User impact if declined: (same as above) Crashes depending on the optimization level when any URL is typed on ppc64le.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): NPOTB.
• String changes made/needed: None

Comment 93

[Cameron Kaiser [:spectre]]

Okay, asking for beta then (and ESR).

Comment 94

[Julien Cristau [:jcristau]]

Comment on attachment 9075022 [details]
Bug 1512162 - A better fix for incorrect code generation on ppc64le. r?bholley

beta already has that change

Comment 95

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9075021 [details] [diff] [review]
Use targeted attributes instead to deal with incorrect inlining

Fixes a PowerPC issue. NPOTB for supported platforms. Approved for 68.1esr.

Comment 96

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/582e8db79ecc

Comment 97

[Dan Horák]

There might be an issue when the current workaround is compiled with gcc 8.3 (for example in F-29), see the error bellow I got from our CI

In file included from /home/jenkins/workspace/Firefox-default/label/ppc64le/firefox/obj-powerpc64le-unknown-linux-gnu/js/xpconnect/src/Unified_cpp_js_xpconnect_src1.cpp:65:
/home/jenkins/workspace/Firefox-default/label/ppc64le/firefox/js/xpconnect/src/XPCWrappedNative.cpp: In member function ‘bool CallMethodHelper::Call()’:
/home/jenkins/workspace/Firefox-default/label/ppc64le/firefox/js/xpconnect/src/XPCWrappedNative.cpp:1326:6: error: inlining failed in call to always_inline ‘bool CallMethodHelper::GatherAndConvertResults()’: function not inlinable
 bool CallMethodHelper::GatherAndConvertResults() {
      ^~~~~~~~~~~~~~~~
/home/jenkins/workspace/Firefox-default/label/ppc64le/firefox/js/xpconnect/src/XPCWrappedNative.cpp:1206:33: note: called from here
   return GatherAndConvertResults();
          ~~~~~~~~~~~~~~~~~~~~~~~^~
gmake[4]: *** [/home/jenkins/workspace/Firefox-default/label/ppc64le/firefox/config/rules.mk:803: Unified_cpp_js_xpconnect_src1.o] Error 1

Comment 98

[Cameron Kaiser [:spectre]]

That doesn't make any sense. Wasn't the point of the patch not to inline that function? Or are you saying it's clashing with the always_inline? If so, maybe we should add an analogous change to the header file.

Unfortunately I'm on F30, not F29 anymore, so I can't test.

Comment 99

[Dan Horák]

(In reply to Cameron Kaiser [:spectre] from comment #98)

That doesn't make any sense. Wasn't the point of the patch not to inline that function? Or are you saying it's clashing with the always_inline? If so, maybe we should add an analogous change to the header file.

Unfortunately I'm on F30, not F29 anymore, so I can't test.

From what I know now (still need to look at the details) it's the clash between "always_inline" and "noinline" with gcc 8, where gcc 9 is able to cope with it (but still prints some warnings IIRC).

Comment 100

[Dan Horák]

diff -up ./js/xpconnect/src/XPCWrappedNative.cpp.orig ./js/xpconnect/src/XPCWrappedNative.cpp
--- ./js/xpconnect/src/XPCWrappedNative.cpp.orig	2019-07-16 11:16:23.355401295 +0200
+++ ./js/xpconnect/src/XPCWrappedNative.cpp	2019-07-16 11:13:29.581694872 +0200
@@ -1092,7 +1092,7 @@ class MOZ_STACK_CLASS CallMethodHelper f
   MOZ_ALWAYS_INLINE bool GetOutParamSource(uint8_t paramIndex,
                                            MutableHandleValue srcp) const;
 
-  MOZ_ALWAYS_INLINE bool GatherAndConvertResults();
+  /*MOZ_ALWAYS_INLINE*/ bool GatherAndConvertResults();
 
   MOZ_ALWAYS_INLINE bool QueryInterfaceFastPath();
 
@@ -1139,7 +1139,7 @@ class MOZ_STACK_CLASS CallMethodHelper f
 
   ~CallMethodHelper();
 
-  MOZ_ALWAYS_INLINE bool Call();
+  /*MOZ_ALWAYS_INLINE*/ bool Call();
 
   // Trace implementation so we can put our CallMethodHelper in a Rooted<T>.
   void trace(JSTracer* aTrc);

is required for build with gcc version 8.3.1 20190223 (Red Hat 8.3.1-2) (GCC) in Fedora 29

Comment 101

[Cameron Kaiser [:spectre]]

oic. Well, I guess we could wrap that in the same #ifdef conditions since it won't hurt gcc 9.

Comment 102

[Jonathan Brielmaier]

Attachment: backtrace with applied patch

I applied the patch on openSUSE Tumbleweed on firefox 68.0.1 (non ESR). I double checked that the patch is successfully applied.

But nonetheless I still get the same error: immediately crash while typing in the URL bar.

Comment 103

[Jonathan Brielmaier]

(In reply to jonathan.brielmaier from comment #102)

I applied the patch on openSUSE Tumbleweed on firefox 68.0.1 (non ESR). I
double checked that the patch is successfully applied.

But nonetheless I still get the same error: immediately crash while typing
in the URL bar.

You can find the package at home:jbrielmaier:ppc64le/MozillaFirefox.

Do others see the same error? If not my assumption could be the compiler flags on Tumbleweed, because they tend to be quit "agressive".

Can I download somewhere a firefox for pcc64le including the patch? Like the normal firefox releases as tarball including the libraries? So not using the system libraries.

Comment 104

[Cameron Kaiser [:spectre]]

This seems different than this particular bug -- I don't see it asserting here, it actually looks like a real honest to goodness crash. Is there an assertion before it goes down? What are your compiler flags? Is this a debug build (and if it's not, does a debug build perform differently?)?

Comment 105

[Jonathan Brielmaier]

(In reply to Cameron Kaiser [:spectre] from comment #104)

This seems different than this particular bug -- I don't see it asserting here, it actually looks like a real honest to goodness crash. Is there an assertion before it goes down? What are your compiler flags? Is this a debug build (and if it's not, does a debug build perform differently?)?

I'll look closer to see what's going on before.

Compiler flags are:

-fmessage-length=0 -grecord-gcc-switches -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=8 -g

from the system and additionally -mminimal-toc from the package build on ppc64le.

Comment 106

[Cameron Kaiser [:spectre]]

Those options seem a bit unusual to me. I am immediately skeptical of the -flto -- I have had problems with building Fx on ppc64le with LTO enabled before. Are you using bfd or gold to link? The .mozconfigs I'm using to build my internal test builds (both debug and opt) are here: https://www.talospace.com/2019/05/firefox-67-on-power.html

Comment 107

[Jonathan Brielmaier]

(In reply to Cameron Kaiser [:spectre] from comment #106)

Those options seem a bit unusual to me. I am immediately skeptical of the -flto -- I have had problems with building Fx on ppc64le with LTO enabled before. Are you using bfd or gold to link? The .mozconfigs I'm using to build my internal test builds (both debug and opt) are here: https://www.talospace.com/2019/05/firefox-67-on-power.html

I tried to build firefox-esr68 locally with mach. I used basically your .mozconfig:

export CC=/usr/bin/gcc
export CXX=/usr/bin/g++

mk_add_options MOZ_MAKE_FLAGS="-j24"
ac_add_options --enable-application=browser
ac_add_options --enable-optimize="-O1 -mcpu=power9"
ac_add_options --enable-debug
ac_add_options --enable-linker=bfd

export GN=/usr/bin/gn # if you have it
export RUSTC_OPT_LEVEL=2

So this is without the compiler flags mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1512162#c105. This particular config built successfully and the URL bar error was gone :)

I now try to build Firefox in Tumbleweed with this configuration, -flto disabled and the other compiler flags of Tumbleweed, but I get a compiler error:

[ 6256s] 101:10.08 In file included from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsAutoPtr.h:10,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsHashKeys.h:12,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsCSSPropertyID.h:14,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/mozilla/ServoBindingTypes.h:106,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/mozilla/ServoStyleConstsForwards.h:27,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/mozilla/ServoStyleConsts.h:28,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsStyleConsts.h:17,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/gfxTypes.h:11,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/gfxASurface.h:14,
[ 6256s] 101:10.09                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/gfxXlibSurface.h:9,
[ 6256s] 101:10.10                  from /home/abuild/rpmbuild/BUILD/firefox-68.0.1/widget/gtk/WindowSurfaceX11Image.h:14,
[ 6256s] 101:10.10                  from /home/abuild/rpmbuild/BUILD/firefox-68.0.1/widget/gtk/WindowSurfaceX11Image.cpp:7,
[ 6256s] 101:10.10                  from /home/abuild/rpmbuild/BUILD/obj/widget/gtk/Unified_cpp_widget_gtk1.cpp:2:
[ 6256s] 101:10.11 /home/abuild/rpmbuild/BUILD/obj/dist/include/nsCOMPtr.h: In instantiation of 'void nsCOMPtr<T>::assign_from_qi(nsQueryInterface<U>, const nsIID&) [with U = nsIFile; T = nsIFile; nsIID = nsID]':
[ 6256s] 101:10.11 /home/abuild/rpmbuild/BUILD/obj/dist/include/nsCOMPtr.h:570:5:   required from 'nsCOMPtr<T>::nsCOMPtr(nsQueryInterface<U>) [with U = nsIFile; T = nsIFile]'
[ 6256s] 101:10.11 /home/abuild/rpmbuild/BUILD/firefox-68.0.1/widget/gtk/nsFilePicker.cpp:774:54:   required from here
[ 6256s] 101:10.11 /home/abuild/rpmbuild/BUILD/obj/dist/include/nsCOMPtr.h:1159:7: error: static assertion failed: don't use do_QueryInterface for compile-time-determinable casts
[ 6256s] 101:10.11  1159 |       !(mozilla::IsSame<T, U>::value || mozilla::IsBaseOf<T, U>::value),
[ 6256s] 101:10.11       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 6258s] 101:11.27 In file included from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsISupportsUtils.h:14,
[ 6258s] 101:11.27                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/nsCOMPtr.h:30,
[ 6258s] 101:11.28                  from /home/abuild/rpmbuild/BUILD/obj/dist/include/mozilla/ComposerCommandsUpdater.h:10,
[ 6258s] 101:11.28                  from /home/abuild/rpmbuild/BUILD/firefox-68.0.1/editor/composer/ComposerCommandsUpdater.cpp:7,
[ 6258s] 101:11.28                  from /home/abuild/rpmbuild/BUILD/obj/editor/composer/Unified_cpp_editor_composer0.cpp:2:
[ 6258s] 101:11.28 /home/abuild/rpmbuild/BUILD/firefox-68.0.1/editor/composer/ComposerCommandsUpdater.cpp: In member function 'virtual nsresult mozilla::ComposerCommandsUpdater::QueryInterface(const nsIID&, void**)':

Comment 108

[Cameron Kaiser [:spectre]]

I'm not sure what to make of that, but what's going on here is definitely not the root cause of this bug. Can you open a new one?

Comment 109

[Jonathan Brielmaier]

After some testing I found a proper workaround on Tumbleweed.

The package gets now compiled with -O1 instead of -O2. I don't apply the patch, because it doesn't change anything for good or bad...

Comment 110

[Cameron Kaiser [:spectre]]

Jonathan, I am now able to reproduce your bug with the same stack trace, and have filed it (and cc'ed you) as bug 1576303.

Comment 111

[msirringhaus]

(In reply to Cameron Kaiser [:spectre] from comment #101)

oic. Well, I guess we could wrap that in the same #ifdef conditions since it won't hurt gcc 9.

Is there a #ifdef-patch now for this problem with gcc < 9 instead of commenting out /*MOZ_ALWAYS_INLINE*/?
I'm hitting this as well.

Comment 112

[Dan Horák]

Probably not (yet).

Comment 113

[Cameron Kaiser [:spectre]]

We're no longer certain this is the actual problem. See bug 1576303 for more.

Comment 114

[msirringhaus]

(In reply to Cameron Kaiser [:spectre] from comment #113)

We're no longer certain this is the actual problem. See bug 1576303 for more.

My current problem is a broken build (as mentioned in comment #100), so I haven't even gotten so far as to test the actual functionality.
The patch mentioned there works, but I was hoping for a "cleaner" one.

Comment 115

[Cameron Kaiser [:spectre]]

I'd have to defer that to someone with a gcc-8 install, since I don't have any systems running it.

Comment 116

[msirringhaus]

Its not that big of a deal. For now, I simply put an ifdef around the patch in comment #100, which works.
Just wanted to check in, if a cleaner fix exists already.

Comment 117

[Ted Campbell [:tcampbell]]

https://bugzilla.mozilla.org/show_bug.cgi?id=1576303#c53

This is actually an old XPCOM bug and not a compiler issue. The ABI code handles floating pointer arguments incorrectly.

Comment 118

[Cameron Kaiser [:spectre]]

Duping into bug 1576303, since that is the ultimate solution to this issue.