Steve Singer (:stevensn) Assignee
	Description • 9 years ago

Firefox on my ppc64 (linux) machine is crashing (Segmentation fault) on startup in 
 js::TypedArrayObject::getElement(unsigned int)


This appears to be introduced between
10e00e356576 (works)
239195d791ce (fails)


The stack trace is

#0  0x00003fffb2faecd4 in js::TypedArrayObject::getElement(unsigned int) (this=0x3fff969077a0, index=0)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/TypedArrayObject.cpp:1650
#1  0x00003fffb2f069a4 in js::NativeObject::getDenseOrTypedArrayElement(unsigned int) (this=<optimized out>, idx=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/NativeObject-inl.h:240
#2  0x00003fffb2ee6f40 in js::NativeGetPropertyNoGC(JSContext*, js::NativeObject*, JS::Value const&, jsid, JS::Value*) (vp=..., nameLookup=NotNameLookup, id=..., receiver=..., obj=<optimized out>, cx=0x3fff97fafc00) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/NativeObject.cpp:1927
#3  0x00003fffb2ee6f40 in js::NativeGetPropertyNoGC(JSContext*, js::NativeObject*, JS::Value const&, jsid, JS::Value*) (cx=0x3fff97fafc00, obj=<optimized out>, receiver=..., id=..., vp=0x3fff96792468) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/NativeObject.cpp:1971
#4  0x00003fffb2be9828 in js::GetElementNoGC(JSContext*, JSObject*, JS::Value const&, unsigned int, JS::Value*) (vp=<optimized out>, id=..., receiver=..., obj=<optimized out>, cx=<optimized out>) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/NativeObject.h:1476
#5  0x00003fffb2be9828 in js::GetElementNoGC(JSContext*, JSObject*, JS::Value const&, unsigned int, JS::Value*) (cx=<optimized out>, obj=<optimized out>, receiver=..., index=<optimized out>, vp=<optimized out>) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsobjinlines.h:210
#6  0x00003fffb2ef98ec in Interpret(JSContext*, js::RunState&) (vp=<optimized out>, index=<optimized out>, receiver=<optimized out>, obj=<optimized out>, cx=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsobjinlines.h:216
#7  0x00003fffb2ef98ec in Interpret(JSContext*, js::RunState&) (res=..., key=..., receiver=..., obj=..., op=<optimized out>, cx=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter-inl.h:424
#8  0x00003fffb2ef98ec in Interpret(JSContext*, js::RunState&) (res=..., rref=..., lref=..., op=<optimized out>, cx=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter-inl.h:556
#9  0x00003fffb2ef98ec in Interpret(JSContext*, js::RunState&) (cx=0x3fff97fafc00, state=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:2611
#10 0x00003fffb2efdc3c in js::RunScript(JSContext*, js::RunState&) (cx=cx@entry=0x3fff97fafc00, state=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:391
#11 0x00003fffb2efe070 in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=cx@entry=0x3fff97fafc00, args=..., construct=construct@entry=js::NO_CONSTRUCT)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:462
#12 0x00003fffb2f04054 in js::SpreadCallOperation(JSContext*, JS::Handle<JSScript*>, unsigned char*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (cx=0x3fff97fafc00, script=..., pc=0x3fff96c4480e ")\005\231\rΐ\210\004\326\210\025\317\070\"\357\210\t", thisv=..., callee=..., arr=..., newTarget=..., res=...) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:4539
#13 0x00003fffb2ef247c in Interpret(JSContext*, js::RunState&) (cx=0x3fff97fafc00, state=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:2723
#14 0x00003fffb2efdc3c in js::RunScript(JSContext*, js::RunState&) (cx=cx@entry=0x3fff97fafc00, state=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:391
#15 0x00003fffb2efe070 in js::Invoke(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (cx=cx@entry=0x3fff97fafc00, args=..., construct=construct@entry=js::NO_CONSTRUCT)
 at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:462
#16 0x00003fffb2efeea8 in js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>) (cx=0x3fff97fafc00, thisv=..., fval=..., argc=<optimized out>, argv=<optimized out>, rval=...) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/Interpreter.cpp:496
#17 0x00003fffb2d5fcf0 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) (cx=0x3fff97fafc00, thisv=..., fval=..., args=..., rval=...) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jsapi.cpp:2837
#18 0x00003fffb17c20c8 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) (this=<optimized out>, cx=0x3fff97fafc00, aThisVal=..., event=..., aRv=...) at /home/ssinger/build-out.ppc64/dom/bindings/EventListenerBinding.cpp:47
#19 0x00003fffb19a9a00 in mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) (this=this@entry=
    0x3fff96d2a700, thisVal=@0x3fff96bfd318: 0x3fff97eb1600, event=..., aRv=..., aExecutionReason=<optimized out>, 
    aExecutionReason@entry=0x0, aExceptionHandling=aExceptionHandling@entry=mozilla::dom::CallbackObject::eReportExceptions, aCompartment=aCompartment@entry=0x0)
    at ../../dist/include/mozilla/dom/EventListenerBinding.h:54
#20 0x00003fffb19a11d8 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) (this=this@entry=0x3fff96d43a00, aListener=<optimized out>, aListener@entry=0x3fff96d43a30, aDOMEvent=0x3fff9464af60, aCurrentTarget=aCurrentTarget@entry=0x3fff97eb1600)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventListenerManager.cpp:1025
#21 0x00003fffb19a1634 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) (this=0x3fff96d43a00, aPresContext=0x0, aEvent=0x3fff9498ccd0, aDOMEvent=0x3fff96bfd6f8, aCurrentTarget=0x3fff97eb1600, aEventStatus=0x3fff96bfd700)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventListenerManager.cpp:1156
#22 0x00003fffb19915d4 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) (aEventStatus=0x3fff96bfd700, aCurrentTarget=0x3fff97eb1600, aDOMEvent=0x3fff96bfd6f8, aEvent=0x3fff9498ccd0, aPresContext=0x0, this=<optimized out>) at ../../dist/include/mozilla/EventListenerManager.h:350
#23 0x00003fffb19915d4 in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) (this=0x3fff952d07c8, aVisitor=..., aCd=...) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventDispatcher.cpp:225
#24 0x00003fffb1982d28 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) (aChain=..., aVisitor=..., aCallback=aCallback@entry=0x0, aCd=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventDispatcher.cpp:315
#25 0x00003fffb1988014 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) (aTarget=<optimized out>, aPresContext=0x0, aEvent=0x3fff9498ccd0, aDOMEvent=0x3fff9464af60, aEventStatus=0x3fff96bfd8f0, aCallback=0x0, aTargets=0x0) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventDispatcher.cpp:654
#26 0x00003fffb19883f4 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) (aTarget=<optimized out>, aEvent=<optimized out>, aDOMEvent=0x3fff9464af60, aPresContext=0x0, aEventStatus=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/events/EventDispatcher.cpp:723
#27 0x00003fffb1df3c1c in (anonymous namespace)::MessageEventRunnable::DispatchDOMEvent(JSContext*, mozilla::DOMEventTargetHelper*, bool, mozilla::dom::workers::WorkerPrivate
#28 0x00003fffb1df3d5c in (anonymous namespace)::MessageEventRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (this=<optimized out>, aCx=<optimized out>, aWorkerPrivate=<optimized out>) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/workers/WorkerPrivate.cpp:746
#29 0x00003fffb1ddda2c in mozilla::dom::workers::WorkerRunnable::Run() (this=0x3fff93010400)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/workers/WorkerRunnable.cpp:360
#30 0x00003fffb0671920 in nsThread::ProcessNextEvent(bool, bool*) (this=0x3fff97f23400, aMayWait=<optimized out>, aResult=0x3fff96bfde80)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/threads/nsThread.cpp:964
#31 0x00003fffb069c0cc in NS_ProcessNextEvent(nsIThread*, bool) (aThread=<optimized out>, aMayWait=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/glue/nsThreadUtils.cpp:297
#32 0x00003fffb1decb74 in mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) (this=0x3fff97e98000, aCx=0x3fff97fafc00)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/workers/WorkerPrivate.cpp:4511
#33 0x00003fffb1dba980 in (anonymous namespace)::WorkerThreadPrimaryRunnable::Run() (this=0x3fff96dcff40)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/dom/workers/RuntimeService.cpp:2722
#34 0x00003fffb0671920 in nsThread::ProcessNextEvent(bool, bool*) (this=0x3fff97f23400, aMayWait=<optimized out>, aResult=0x3fff96bfe420)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/threads/nsThread.cpp:964
#35 0x00003fffb069c0cc in NS_ProcessNextEvent(nsIThread*, bool) (aThread=<optimized out>, aMayWait=<optimized out>)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/glue/nsThreadUtils.cpp:297
#36 0x00003fffb095c258 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (this=0x3fff96d21200, aDelegate=0x3fffb4bc7e00)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/glue/MessagePump.cpp:326
#37 0x00003fffb090e1e4 in MessageLoop::RunInternal() (this=<optimized out>, this@entry=0x3fffb4bc7e00)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/chromium/src/base/message_loop.cc:234
#38 0x00003fffb090e454 in MessageLoop::Run() (this=0x3fffb4bc7e00) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/chromium/src/base/message_loop.cc:227
#39 0x00003fffb090e454 in MessageLoop::Run() (this=0x3fffb4bc7e00) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/chromium/src/base/message_loop.cc:201
#40 0x00003fffb0674ff4 in nsThread::ThreadFunc(void*) (aArg=0x3fff97f23400) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/xpcom/threads/nsThread.cpp:376
#41 0x00003fffb76cfc14 in _pt_root (arg=0x3fffb4b1e620) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/nsprpub/pr/src/pthreads/ptthread.c:212
#42 0x00003fffb7f1bc44 in .start_thread () at /lib64/libpthread.so.0
#43 0x00003fffb7a942a8 in .__clone () at /lib64/libc.so.6

	Lars T Hansen [:lth]
	Comment 1 • 9 years ago

Since you appear to have source code, can you map TypedArrayObject.cpp:1650 back to the source and tell me what line that is (ie include the text of the line in a followup here)?  I'm guessing it's the MOZ_CRASH in getElement() but I'd like to know for sure.

	Lars T Hansen [:lth]
	Comment 2 • 9 years ago

Also relevant may be bug 1055472, haven't looked in detail at the regression window yet.

	Steve Singer (:stevensn) Assignee
	Comment 3 • 9 years ago

 js::TypedArrayObject::getElement (this=0x3fff969077a0, index=0) at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/TypedArrayObject.cpp:1650
1650            return Uint8ClampedArray::getIndexValue(this, index);

To confirm this I tired replacing the the MOZ_CRASH in getElement with return Value() and it still crashes on 1650.

The regression range is part 1 through 15 of bug 1176214 (which landed together)
I could attempt narrow it further if required

	Lars T Hansen [:lth]
	Comment 4 • 9 years ago

Most of the patches on that bug need to land together, and likely this will have been the third patch ("core changes").

Fascinating call stack, this is in a worker while processing worker events.  Any information about the event and the code that is being run to handle it would be helpful.

I know this is on startup and this does not look like jitted code, but do you know if there's a JIT on this configuration?

	Steve Singer (:stevensn) Assignee
	Comment 5 • 9 years ago

How can I provide the information about the events/code your asking about?

There is no JIT (--disable-ion build) since no jit is supported on ppc64

	Lars T Hansen [:lth]
	Comment 6 • 9 years ago

(In reply to Steve Singer (:stevensn) from comment #5)
> How can I provide the information about the events/code your asking about?

You'd need to use a debugger and try to get information out of memory at suitable points.  I'm probably not able to help with that remotely, so if you don't know how to do it we're at an impasse.

I'll also try to reproduce the bug locally on x64 with all the jits disabled, maybe I'll be lucky.

	Lars T Hansen [:lth]
	Comment 7 • 9 years ago

I'm not able to reproduce the crash on an x64 debug build with the JITs disabled in the config.  And sadly I don't have any PPC64 hardware locally, only PPC32.

Do you know if there are tabs open that will be opened on startup, or is this just with the basic UI and no tabs?  And do you know if multiprocess (e10s) is enabled or not for this build?  What about extensions?

	Steve Singer (:stevensn) Assignee
	Comment 8 • 9 years ago

I compiled that file with optimizations disabled and got a slightly different stack trace.
Which makes more sense.

#0  0x00003fffb2fcbcf0 in js::jit::AtomicOperations::loadSafeWhenRacy<unsigned char>(unsigned char*) (addr=0x3fff954077e0 "mozLz40")
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jit/none/AtomicOperations-none.h:97
#1  0x00003fffb2fc2ca8 in js::jit::AtomicOperations::loadSafeWhenRacy<unsigned char>(SharedMem<unsigned char*>) (addr=...)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/jit/AtomicOperations.h:219
#2  0x00003fffb2f61dcc in (anonymous namespace)::TypedArrayObjectTemplate<unsigned char>::getIndex(JSObject*, uint32_t) (obj=0x3fff954077a0, index=0)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/TypedArrayObject.cpp:641
#3  0x00003fffb2f5ab74 in (anonymous namespace)::TypedArrayObjectTemplate<unsigned char>::getIndexValue(JSObject*, uint32_t) (tarray=0x3fff954077a0, index=0)
    at /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/js/src/vm/TypedArrayObject.cpp:902
#4  0x00003fffb2f45f14 in js::TypedArrayObject::getElement(unsigned int) (this=0x3fff954077a0, index=0)


Should the calls to jit::AtomicOperations all include a non jit version in an ifdef? Is there a cleaner way of doing this?
I also don't understand why your --disable-ion build worked

	Lars T Hansen [:lth]
	Comment 9 • 9 years ago

Ah, now we're really getting somewhere.  Thanks for the detective work.

The test build I created was not configured at compile time with no JITs.  Instead it was a standard build with both JITs disabled at runtime in about:config.  Hence I would get the platform layer for x86_64, not the platform layer for none.

I was told by Somebody In Authority (bug 1084248 comment 17) that "none"-configured builds would never be run, ever, hence there was no need to implement any of these operations - they can always just crash.  That was probably a miscommunication; cc'ing said Authority.  If "none"-configured builds can be run then the AtomicOperations must be fleshed out, since they provide JIT-compatible functionality to the C++ runtime even when there's no JIT.  Personally I would favor a PPC64-specific solution where jit/AtomicOperations.h adds a new case for PPC64 and includes a PPC64-specific implementation file.

(There's also the issue that, in the deepest sense, the safe-when-racy AtomicOperations simply cannot be implemented 100% correctly in a platform-independent way.  We can probably wink our way past that for now, as we do on the other platforms, where we still use unsafe operations such as memcpy because it's not been a priority to write that code yet.  But that's another reason to have a PPC64-specific solution, and to not try to make the "none" solution work.)

	Steve Singer (:stevensn) Assignee
	Comment 10 • 9 years ago

If adding ppc32/ppc64 atomic operations can be done without porting ION to ppc then I can take a stab at that over the holidays (unless Cameron already has a AtomicOperations-ppc file he wants to share)

I think we would still need a fallback for platform that we don't have a specific implementation for (sparc?)

	Lars T Hansen [:lth]
	Comment 11 • 9 years ago

No Ion porting is necessary for these functions, they reside in jit/ because they need to be compatible with whatever the jit does, and so the jit "owns" them.

If you look in the corresponding files for x86 and x64 you will find C++ implementations that are suitable for Clang and GCC, these are *technically* not correct because they create undefined behavior in C++ in the presence of data races but they will almost certainly work properly for you - you should start out by copying those.  (Ditto SPARC, if SPARC no longer has an Ion back-end.)  If you have some other compiler you may have to do a little work, but it is probably not a huge amount of work.

	Cameron Kaiser [:spectre]
	Comment 12 • 9 years ago

I haven't gotten to trying to port this yet to IonPower. I'll probably start with Fx40 sometime after the new year. When did this go into the JIT, which bug?

	Steve Singer (:stevensn) Assignee
	Comment 13 • 9 years ago

bug 1176214 introduced the call to the platform specific code from a non-ion code path but the functions themselves with the unimplemented -none versions seem to have been added with bug 1084248

	Lars T Hansen [:lth]
	Comment 14 • 9 years ago

Cameron, please cc me on that work, just because I like to know how it proceeds.  Presumably for IonPower you'll also have to do JIT implementations of all the atomic operations, but ARM and MIPS32 have walked that road already and should provide a lot of working structure.

	Cameron Kaiser [:spectre]
	Comment 15 • 9 years ago

(In reply to Lars T Hansen [:lth] from comment #14)
> Cameron, please cc me on that work, just because I like to know how it
> proceeds.  Presumably for IonPower you'll also have to do JIT
> implementations of all the atomic operations, but ARM and MIPS32 have walked
> that road already and should provide a lot of working structure.

It's going to depend greatly on whether it's ABI or OS dependent. If it ends up being specific to OS X/ppc, there might not be much utility to Linux/ppc. I haven't really looked at this yet since I've just been snowed in at work. We have some functioning atomics in other code, though.

	Lars T Hansen [:lth]
	Comment 16 • 9 years ago

(In reply to Cameron Kaiser [:spectre] from comment #15)
> (In reply to Lars T Hansen [:lth] from comment #14)
> > Cameron, please cc me on that work, just because I like to know how it
> > proceeds.  Presumably for IonPower you'll also have to do JIT
> > implementations of all the atomic operations, but ARM and MIPS32 have walked
> > that road already and should provide a lot of working structure.
> 
> It's going to depend greatly on whether it's ABI or OS dependent. If it ends
> up being specific to OS X/ppc, there might not be much utility to Linux/ppc.
> I haven't really looked at this yet since I've just been snowed in at work.
> We have some functioning atomics in other code, though.

So far, the atomics that you need to implement (exchange, compareExchange, add, sub, and, or, and xor, all for 8, 16, and 32 bit integers, plus a general ordering barrier) have all been ABI-independent on all platforms, but somewhat CPU-dependent, eg some older ARM only have 32-bit variants and 8 and 16 bit ops must be synthesized when that's the case.  Based on earlier investigation (see the "POWER and PowerPC" section of https://docs.google.com/document/d/1NDGA_gZJ7M7w1Bh8S0AoDyEqwDdRh4uSoTPSNn77PFk/edit#heading=h.a6o4dubw5qla), PPC should be OK here.  But naturally I'm curious to know if that analysis is right :)

	Cameron Kaiser [:spectre]
	Comment 17 • 9 years ago

The TenFourFox work will be tracked in https://github.com/classilla/tenfourfox/issues/312

	Steve Singer (:stevensn) Assignee
	Comment 19 • 9 years ago

Attachment 8703287 [details] [diff] implements draft atomics based on the other implementations for ppc and ppc64. This seems to 'work' on ppc64 but I am not sure what a good test of the actual concurrency behavior would be.

I haven't yet tried a ppc32 build.

I am also not sure if we want to leave the MOZ_ASSERT version in for other plaforms or replace it with a compile time error

	Lars T Hansen [:lth]
	Comment 20 • 9 years ago

The patch is presumably incorrect for ppc32 since ppc32 should have a full Ion implementation and would want to include this file from the bottom of jit/AtomicOperations.h.  Frankly that would be cleaner for ppc64 too.  But that's just a nit.

We have some test cases for atomic behavior, assuming you have a multicore machine to test on.

A number of the TypedArray and asm.js shell tests in js/src/tests and js/src/jit-test will exercise these functions, for example.  For the JS shell, you can also run js/src/tests/shell/futex.js.

If you build a full browser you can also try to run some of my multiprocessing benchmarks, which will generally fail spectacularly if the mutual exclusion does not work properly:  clone https://github.com/lars-t-hansen/parlib-simple and then run demo/mandelbrot-animation2, for example.

	Lars T Hansen [:lth]
	Comment 22 • 9 years ago

Comment on attachment 8704233 [details] [diff] [review]
Add atomics for ppc/ppc64

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

That should do just fine for the time being.

If you know you're going to be using a specific compiler family (eg gcc) or your compiler will always support the newer gcc atomic primitives (so you won't have to use __sync_whatever; the arm64 code does this) or even then latest ISO C atomic primitives, then it's no shame to clean up that file further, but there's no rush.

	Steve Singer (:stevensn) Assignee
	Comment 23 • 9 years ago

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a381227493e1

	Pulsebot
	Comment 24 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/6b77efce4af9

	Ryan VanderMeulen [:RyanVM]
	Comment 25 • 9 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/6b77efce4af9

	Steve Singer (:stevensn) Assignee
	Comment 26 • 9 years ago

Comment on attachment 8704233 [details] [diff] [review]
Add atomics for ppc/ppc64

Approval Request Comment
[Feature/regressing bug #]:1176214
[User impact if declined]: ppc and ppc64 platforms will crash on startup
[Describe test coverage new/current, TreeHerder]: Tested in m-c
[Risks and why]: Low - not part of the build for tier1 platforms
[String/UUID change made/needed]:none

	Sylvestre Ledru [:Sylvestre]
	Comment 27 • 9 years ago

Comment on attachment 8704233 [details] [diff] [review]
Add atomics for ppc/ppc64

Fix a crash on ppc, taking it, especially as 45 is an ESR.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 28 • 9 years ago
bugherder uplift

https://hg.mozilla.org/releases/mozilla-aurora/rev/d8843dfeb706

	Steve Singer (:stevensn) Assignee
	Comment 29 • 9 years ago

Landry, I'm not sure if there are any other tier3 platforms that the *BSD's are supporting (sparc?) but they would have the same issue as ppc

	Landry Breuil (:gaston)
	Comment 30 • 9 years ago

sparc32 is dead, ppc doesnt build/link since a while, and i dont have a sparc64 builder anymore - ccing martin@netbsd who still bravely cares about firefox on NetBSD/sparc64 :)

	Martin Husemann
	Comment 31 • 9 years ago

Landry is right: sparc32 is dead, and sparc64 does not (currently) have JIT at all.
I have mid-term plans to fix that, but untill I get around to that, I guess this is a don't care - unless I misread the patch.

If it comes up in actual builds, I'll be happy to provide a patch for sparc64.

	Steve Singer (:stevensn) Assignee
	Comment 32 • 9 years ago

Martin, the code is used in non-jit builds, so right now I would expect sparc64 to crash on startup like ppc/ppc64 did.  You should test a sparc64 build (45+) to see if it starts.

	Martin Husemann
	Comment 33 • 9 years ago

Indeed, as expected it asserts now.

The implementation for sparc* is nearly identical to the ppc one, only a few preprocessor changes.

Would it be preferable to create a none/AtomicOperations-generic.h instead and do the differences as #ifdefs in there?

	Thomas Klausner
	Comment 34 • 8 years ago

Should this bug report be re-opened, or a new one filed?
It still contains uncommitted changes by Martin for better sparc64 support.

	Martin Husemann
	Comment 35 • 8 years ago

Thanks for the reminder - I only came around actually testing this patches in the last few days.
They fix the build for me and seem to work.

Do you prefer a separate ticket for this?

	Martin Husemann
	Comment 36 • 8 years ago

Attachment #8710903 [details] [diff] still needs to be applied

	Pulsebot
	Comment 37 • 8 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f937e9c8f4d

	Carsten Book [:Tomcat]
	Comment 38 • 8 years ago
bugherder

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

	Ian Stakenvicius
	Comment 39 • 8 years ago

This is a little late to the game, given the ESR45 series is almost finished, but hopefully it will still apply to 52.x and beyond as well.  The atomics for ia64 were submitted to gentoo linux by Émeric Maschino and address the same sort of crashes that were present on ppc/ppc64 and sparc prior to their patches.

	Lars T Hansen [:lth]
	Comment 40 • 8 years ago

Comment on attachment 8841824 [details] [diff] [review]
Add atomics for ia64

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

Is there any reason you can't just use none/AtomicOperations-ppc.h, like Alpha, HPPA, and SuperH do?  Once I reorg AtomicOperations-ia64.h to have the same definition order as the former file, the differences are completely superficial and should have no impact on functionality or compilability.

(I really should rename AtomicOperations-ppc.h to AtomicOperations-feeling-lucky.h)

	Lars T Hansen [:lth]
	Comment 41 • 7 years ago

Ian, curious if you have any remarks to comment 40.

	Ian Stakenvicius
	Comment 42 • 7 years ago

Hi Lars -- Sorry for the delay.  I don't know.  I'm not familiar enough with the details if the ia64 platform, but I do know that Émeric spent a fair bit of time with diagnosing support on ia64, and to get firefox to build and run he went this route with his patch.  If they are functionally the same then I expect using just -ppc.h would be fine, so I'll see if he can test and provide feedback for 52.

(Added Émeric to the CC)

	Lars T Hansen [:lth]
	Comment 43 • 7 years ago

Thanks, Ian.  Even small changes to the -ppc.h file would be fine with me, to accomodate other platforms.

	Lars T Hansen [:lth]
	Comment 44 • 7 years ago

Comment on attachment 8841824 [details] [diff] [review]
Add atomics for ia64

(Cancelling review in anticipation of more information.)

	Lars T Hansen [:lth]
	Comment 45 • 7 years ago

Bug 1347128 attempts to address the general problem, that tier-3 platforms are happy to run unsafe code (for the atomic operations) and just need a convenient way to opt into that.