Closed
Bug 1480732
Opened 6 years ago
Closed 6 years ago
Use atomicops_internals_portable.h on non-x86 MSVC platform
Categories
(Core :: IPC, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: m_kato, Assigned: froydnj)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
jld
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•6 years ago
|
OS: Unspecified → Windows
Hardware: Unspecified → ARM64
Updated•6 years ago
|
Assignee | ||
Comment 1•6 years ago
|
||
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().
Assignee: nobody → nfroyd
Assignee | ||
Comment 2•6 years ago
|
||
Also, we're completely disabling the sandbox on AArch64 Windows for now.
Assignee | ||
Comment 3•6 years ago
|
||
I'm not entirely sure how this works on x86-64 Windows, which also uses
a #define for MemoryBarrier...I think something about intrinsics.
Attachment #9003902 -
Flags: review?(jld)
Comment 4•6 years ago
|
||
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.
Attachment #9003902 -
Flags: review?(jld) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa6c9b69514
make ipc/'s atomicops.h work on aarch64 windows; r=jld
Comment 6•6 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Comment 7•6 years ago
|
||
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.
Flags: needinfo?(nfroyd)
Assignee | ||
Comment 8•6 years ago
|
||
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?
Flags: needinfo?(nfroyd)
You need to log in
before you can comment on or make changes to this bug.
Description
•