1480732 - Use atomicops_internals_portable.h on non-x86 MSVC platform

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Makoto Kato [:m_kato]]

Actually, we have a lot of atomic headers per each platform.h 

https://searchfox.org/mozilla-central/source/ipc/chromium/src/base/atomicops.h#137

// Include our platform specific implementation.
#if defined(OS_WIN) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_msvc.h"
#elif defined(OS_MACOSX) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_macosx.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARMEL)
#include "base/atomicops_internals_arm_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARM64)
#include "base/atomicops_internals_arm64_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_MIPS)
#include "base/atomicops_internals_mips_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_PPC_FAMILY)
#include "base/atomicops_internals_ppc_gcc.h"
#else
#include "base/atomicops_internals_mutex.h"
#endif

But the latest Chromium and our sandbox code uses atomicops_internals_portable.h on non-x86 msvc platform.

https://searchfox.org/mozilla-central/source/security/sandbox/chromium/base/atomicops.h#147

#if defined(OS_WIN)
// TODO(jfb): The MSVC header includes windows.h, which other files end up
//            relying on. Fix this as part of crbug.com/559247.
#  include "base/atomicops_internals_x86_msvc.h"
#else
#  include "base/atomicops_internals_portable.h"
#endif


So IPC should use atomicops_internals_portable.h on non-x86-msvc platform like security/sandbox.

Comment 1

[Nathan Froyd [:froydnj]]

There is not atomicops_internals_portable.h in our copy of IPC.  We can get away with using the x86 implementation, with a small trick for MemoryBarrier().

Comment 2

[Nathan Froyd [:froydnj]]

Also, we're completely disabling the sandbox on AArch64 Windows for now.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: make ipc/'s atomicops.h work on aarch64 windows

I'm not entirely sure how this works on x86-64 Windows, which also uses
a #define for MemoryBarrier...I think something about intrinsics.

Comment 4

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 9003902 [details] [diff] [review]
make ipc/'s atomicops.h work on aarch64 windows

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

Looks reasonable.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa6c9b69514
make ipc/'s atomicops.h work on aarch64 windows; r=jld

Comment 6

[Andrei Ciure[:aciure]]

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

Comment 7

[Masatoshi Kimura [:emk]]

Is this a temporary fix just to compile the tree on aarch64? I think the x86 header is insufficient because x86 requires less  memory barrier calls than other platforms.

Comment 8

[Nathan Froyd [:froydnj]]

That is a good point.  Yes, this is mostly to make the tree compile; I suppose for IPC code, we should probably be using the mutex implementation instead?