1225033 - Export js::jit::AtomicOperations from the engine

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

[Lars T Hansen [:lth]]

These operations are (a) compatible with the JIT's use of mutual exclusion and can be used to implement mutual exclusion with jitted and interpreted code and (b) safe for data races when used from C++.  Client code will want to use these when accessing shared memory; for example, where WebGL opts in to shared memory it currently uses memcpy() to move bits to/from shared memory but this creates a danger of C++ Undefined Behavior.  It should instead use AtomicOperations::memcpySafeWhenRacy(), but that function is not currently available to it.

Comment 1

[Lars T Hansen [:lth]]

When bug 1176214 lands, it will land with comments in dom/bindings/TypedArray.h and in dom/canvas/WebGL2ContextBuffers.cpp that reference the present bug, saying that the API provided here is forthcoming.  When the present bug lands and provides that API those comments should be updated, and the code in WebGL2ContextBuffers.cpp should actually use this API.

Comment 2

[Lars T Hansen [:lth]]

There will be similar comments + issues in dom/canvas/WebGLContextBuffers.cpp and dom/canvas/WebGLElementArrayCache.cpp.  In all cases, search for "bug 1225033".

Comment 3

[Lars T Hansen [:lth]]

It would be nice to get this into FF53.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information

Comment 5

[Lars T Hansen [:lth]]

Safe-for-races functions are now available in the JIT (landed with bug 1394420). We now need to:

• export them from the engine
• use them in the DOM when appropriate

The "when appropriate" bit is important. The safe-for-races functions are generally slower than raw libc functions, this is especially true for memcpy / memmove. Performance-sensitive code that works on shared memory may wish to special-case the unshared path for this reason.

Comment 6

[Anne (:annevk)]

Are these functions also defined in the relevant specifications so DOM specifications can chose which to use?

Comment 7

[Lars T Hansen [:lth]]

What "relevant specifications"? (These are in any case strictly internal, they provide compatibility between C++ code and jitted code and they avoid undefined behavior in C++ operations on shared memory.)

Comment 8

[Anne (:annevk)]

Also, do I interpret your comments elsewhere correctly that this does not necessarily block bug 1477743, but would be good to fix quickly nonetheless? What kind of thing should we not do before this is fixed?

Comment 9

[Lars T Hansen [:lth]]

With PGO there's a minor risk that the C++ compiler will recognize that DOM code triggers undefined behavior if it operates on shared memory and then "optimizes" code accordingly. I don't think this is a big risk but it's hard to know for sure. There's a slightly larger risk that we'll run into TSAN error reports since TSAN, too, looks for UB resulting from races. If SAB is enabled by default then presumably any TSAN runs we do will be run with SAB enabled, and if DOM uses straight C++ to access memory racily then TSAN may complain.

I don't think this should block re-enabling SAB but it's something we need to address. The JS engine is, or will shortly be, clean wrt to this issue, so it's just DOM.

Comment 10

[Henri Sivonen (:hsivonen)]

Is there any chance of identifying C++11/C11 atomic operations that are equivalent to loadSafeWhenRacy() and storeSafeWhenRacy()? Since Rust atomics have C11 atomic semantics, documenting racy-but-safe SAB access in terms of C++11/C11 atomics would enable efficient access from C++ and Rust without having to go through function pointers on a per-byte basis.

Comment 11

[Lars T Hansen [:lth]]

(In reply to Henri Sivonen (:hsivonen) from comment #10)

Is there any chance of identifying C++11/C11 atomic operations that are equivalent to loadSafeWhenRacy() and storeSafeWhenRacy()?

No. We had this initially and this is how the "portable" fallback layer for tier-3 platforms works, but from all the reading I've done this steps into UB pretty quickly because the C++ atomics assume race-free programs. Also, since they assume race-free programs the C++ compiler optimizes them in various ways, omitting fences when DRF lets that be assumed etc, the gcc docs are explicit about that. This would violate the JS/wasm memory model; that memory model allows for optimization of atomics, but probably with different semantics than C++.

(The portable layer is in jit/shared/AtomicOperations-feeling-lucky-{gcc,msvc}.{h,cpp}.)

Since Rust atomics have C11 atomic semantics, documenting racy-but-safe SAB access in terms of C++11/C11 atomics would enable efficient access from C++ and Rust without having to go through function pointers on a per-byte basis.

Yes it would, but I don't think this works.