Bug 1869497 - Introduce MOZ_NAKED to centralize all attributes we expect from naked r=glandium,win-reviewers,mhowell

In the fx codebase, we tend to use __attribute__((naked)) for plain
assembly functions with a C interface. Instrumenting these functions can
mess up this assembly, so we conservatively also want to prevent
instrumentation.

Differential Revision: https://phabricator.services.mozilla.com/D196154

mfbt/Attributes.h

@@ -88,6 +88,29 @@
 #  endif
 #endif
 
+#if defined(__GNUC__) || \
+    (defined(__clang__) && __has_attribute(no_profile_instrument_function))
+#  define MOZ_NOPROFILE __attribute__((no_profile_instrument_function))
+#else
+#  define MOZ_NOPROFILE
+#endif
+
+#if defined(__GNUC__) || \
+    (defined(__clang__) && __has_attribute(no_instrument_function))
+#  define MOZ_NOINSTRUMENT __attribute__((no_instrument_function))
+#else
+#  define MOZ_NOINSTRUMENT
+#endif
+
+/*
+ * MOZ_NAKED tells the compiler that the function only contains assembly and
+ * that it should not try to inject code that may mess with the assembly in it.
+ *
+ * See https://github.com/llvm/llvm-project/issues/74573 for the interaction
+ * between naked and no_profile_instrument_function.
+ */
+#define MOZ_NAKED __attribute__((naked)) MOZ_NOPROFILE MOZ_NOINSTRUMENT
+
 /**
  * Per clang's documentation:
  *

mozglue/misc/NativeNt.h

@@ -1740,12 +1740,7 @@ class AutoMappedView final {
 // CheckStack ensures that stack memory pages are committed up to a given size
 // in bytes from the current stack pointer. It updates the thread stack limit,
 // which points to the lowest committed stack address.
-//
-// See https://github.com/llvm/llvm-project/issues/74573 for the interaction
-// between naked and no_profile_instrument_function.
-MOZ_NEVER_INLINE __attribute__((no_profile_instrument_function))
-__attribute__((naked)) inline void
-CheckStack(uint32_t size) {
+MOZ_NEVER_INLINE MOZ_NAKED inline void CheckStack(uint32_t size) {
   asm volatile(
       "mov %ecx, %eax;"
 #  if defined(__MINGW32__)

toolkit/xre/dllservices/tests/AssemblyPayloads.h

@@ -11,6 +11,8 @@
 #ifndef mozilla_AssemblyPayloads_h
 #define mozilla_AssemblyPayloads_h
 
+#include "mozilla/Attributes.h"
+
 #include <cstdint>
 
 #define PADDING_256_NOP                                              \
@@ -37,7 +39,7 @@ extern "C" {
 #  if defined(_M_X64)
 constexpr uintptr_t JumpDestination = 0x7fff00000000;
 
-__declspec(dllexport) __attribute__((naked)) void MovPushRet() {
+__declspec(dllexport) MOZ_NAKED void MovPushRet() {
   asm volatile(
       "mov %0, %%rax;"
       "push %%rax;"
@@ -46,7 +48,7 @@ __declspec(dllexport) __attribute__((naked)) void MovPushRet() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void MovRaxJump() {
+__declspec(dllexport) MOZ_NAKED void MovRaxJump() {
   asm volatile(
       "mov %0, %%rax;"
       "jmpq *%%rax;"
@@ -54,7 +56,7 @@ __declspec(dllexport) __attribute__((naked)) void MovRaxJump() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void DoubleJump() {
+__declspec(dllexport) MOZ_NAKED void DoubleJump() {
   asm volatile(
       "jmp label1;"
 
@@ -71,7 +73,7 @@ __declspec(dllexport) __attribute__((naked)) void DoubleJump() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void NearJump() {
+__declspec(dllexport) MOZ_NAKED void NearJump() {
   asm volatile(
       "jae label3;"
       "je  label3;"
@@ -90,7 +92,7 @@ __declspec(dllexport) __attribute__((naked)) void NearJump() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void OpcodeFF() {
+__declspec(dllexport) MOZ_NAKED void OpcodeFF() {
   // Skip PUSH (FF /6) because clang prefers Opcode 50+rd
   // to translate PUSH r64 rather than Opcode FF.
   asm volatile(
@@ -102,14 +104,14 @@ __declspec(dllexport) __attribute__((naked)) void OpcodeFF() {
       "int $3;int $3;int $3;int $3;");
 }
 
-__declspec(dllexport) __attribute__((naked)) void IndirectCall() {
+__declspec(dllexport) MOZ_NAKED void IndirectCall() {
   asm volatile(
       "call *(%rip);"  // Indirect call to 0x90909090`90909090
       "nop;nop;nop;nop;nop;nop;nop;nop;"
       "ret;");
 }
 
-__declspec(dllexport) __attribute__((naked)) void MovImm64() {
+__declspec(dllexport) MOZ_NAKED void MovImm64() {
   asm volatile(
       "mov $0x1234567812345678, %r10;"
       "nop;nop;nop");
@@ -118,7 +120,7 @@ __declspec(dllexport) __attribute__((naked)) void MovImm64() {
 #    if !defined(MOZ_CODE_COVERAGE)
 // This code reproduces bug 1798787: it uses the same prologue, the same unwind
 // info, and it has a call instruction that starts within the 13 first bytes.
-__attribute__((naked)) void DetouredCallCode(uintptr_t aCallee) {
+MOZ_NAKED void DetouredCallCode(uintptr_t aCallee) {
   asm volatile(
       "subq $0x28, %rsp;"
       "testq %rcx, %rcx;"
@@ -147,7 +149,7 @@ extern decltype(&DetouredCallCode) gDetouredCall;
 // -- it will not detour DetouredCallJumper. We need to do this to point our
 // hooking code to the dynamic code, because our hooking API works with an
 // exported function name.
-__attribute__((naked)) __declspec(dllexport noinline) void DetouredCallJumper(
+MOZ_NAKED __declspec(dllexport noinline) void DetouredCallJumper(
     uintptr_t aCallee) {
   // Ideally we would want this to be:
   //   jmp qword ptr [rip + offset gDetouredCall]
@@ -160,7 +162,7 @@ __attribute__((naked)) __declspec(dllexport noinline) void DetouredCallJumper(
 #  elif defined(_M_IX86)
 constexpr uintptr_t JumpDestination = 0x7fff0000;
 
-__declspec(dllexport) __attribute__((naked)) void PushRet() {
+__declspec(dllexport) MOZ_NAKED void PushRet() {
   asm volatile(
       "push %0;"
       "ret;"
@@ -168,7 +170,7 @@ __declspec(dllexport) __attribute__((naked)) void PushRet() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void MovEaxJump() {
+__declspec(dllexport) MOZ_NAKED void MovEaxJump() {
   asm volatile(
       "mov %0, %%eax;"
       "jmp *%%eax;"
@@ -176,20 +178,20 @@ __declspec(dllexport) __attribute__((naked)) void MovEaxJump() {
       : "i"(JumpDestination));
 }
 
-__declspec(dllexport) __attribute__((naked)) void Opcode83() {
+__declspec(dllexport) MOZ_NAKED void Opcode83() {
   asm volatile(
       "xor $0x42, %eax;"
       "cmpl $1, 0xc(%ebp);");
 }
 
-__declspec(dllexport) __attribute__((naked)) void LockPrefix() {
+__declspec(dllexport) MOZ_NAKED void LockPrefix() {
   // Test an instruction with a LOCK prefix (0xf0) at a non-zero offset
   asm volatile(
       "push $0x7c;"
       "lock push $0x7c;");
 }
 
-__declspec(dllexport) __attribute__((naked)) void LooksLikeLockPrefix() {
+__declspec(dllexport) MOZ_NAKED void LooksLikeLockPrefix() {
   // This is for a regression scenario of bug 1625452, where we double-counted
   // the offset in CountPrefixBytes.  When we count prefix bytes in front of
   // the 2nd PUSH located at offset 2, we mistakenly started counting from
@@ -204,7 +206,7 @@ __declspec(dllexport) __attribute__((naked)) void LooksLikeLockPrefix() {
       "push $0x0000f0cc;");
 }
 
-__declspec(dllexport) __attribute__((naked)) void DoubleJump() {
+__declspec(dllexport) MOZ_NAKED void DoubleJump() {
   asm volatile(
       "jmp label1;"
 
@@ -223,7 +225,7 @@ __declspec(dllexport) __attribute__((naked)) void DoubleJump() {
 #  endif
 
 #  if !defined(_M_ARM64)
-__declspec(dllexport) __attribute__((naked)) void UnsupportedOp() {
+__declspec(dllexport) MOZ_NAKED void UnsupportedOp() {
   asm volatile(
       "ud2;"
       "nop;nop;nop;nop;nop;nop;nop;nop;"
@@ -232,7 +234,7 @@ __declspec(dllexport) __attribute__((naked)) void UnsupportedOp() {
 
 // bug 1816936
 // Make sure no instruction ends at 5 (for x86) or 13 (for x64) bytes
-__declspec(dllexport) __attribute__((naked)) void SpareBytesAfterDetour() {
+__declspec(dllexport) MOZ_NAKED void SpareBytesAfterDetour() {
   asm volatile(
       "incl %eax;"                // 2 bytes on x64, 1 byte on x86
       "mov $0x01234567, %eax;"    // 5 bytes
@@ -246,8 +248,7 @@ __declspec(dllexport) __attribute__((naked)) void SpareBytesAfterDetour() {
 // This is slightly different than SpareBytesAfterDetour so the compiler doesn't
 // combine them, which would make the test that detours this one behave
 // unexpectedly since it is already detoured.
-__declspec(dllexport)
-    __attribute__((naked)) void SpareBytesAfterDetourFor10BytePatch() {
+__declspec(dllexport) MOZ_NAKED void SpareBytesAfterDetourFor10BytePatch() {
   asm volatile(
       "incl %eax;"                // 2 bytes
       "mov $0x01234567, %ecx;"    // 5 bytes

widget/windows/nsAppShell.cpp

@@ -487,7 +487,7 @@ MOZ_NEVER_INLINE static AtomTableInformation DiagnoseUserAtomTable() {
 }  // namespace
 
 #if defined(MOZ_DIAGNOSTIC_ASSERT_ENABLED) && defined(_M_X64)
-MOZ_NEVER_INLINE __attribute__((naked)) void EnableTrapFlag() {
+MOZ_NEVER_INLINE MOZ_NAKED void EnableTrapFlag() {
   asm volatile(
       "pushfq;"
       "orw $0x100,(%rsp);"
@@ -495,9 +495,7 @@ MOZ_NEVER_INLINE __attribute__((naked)) void EnableTrapFlag() {
       "retq;");
 }
 
-MOZ_NEVER_INLINE __attribute__((naked)) void DisableTrapFlag() {
-  asm volatile("retq;");
-}
+MOZ_NEVER_INLINE MOZ_NAKED void DisableTrapFlag() { asm volatile("retq;"); }
 
 #  define SSD_MAX_USER32_STEPS 0x1800
 #  define SSD_MAX_ERROR_STATES 0x200

xpcom/reflect/xptcall/md/win32/xptcstubs.cpp

@@ -82,12 +82,7 @@ PrepareAndDispatch(nsXPTCStubBase* self, uint32_t methodIndex,
 
 } // extern "C"
 
-static
-__attribute__((naked))
-// Compiler-inserted instrumentation is going to botch our assembly below,
-// so forbid the compiler from doing that.
-__attribute__((no_instrument_function))
-void SharedStub(void)
+static MOZ_NAKED void SharedStub(void)
 {
     __asm {
         push ebp            // set up simple stack frame
@@ -113,7 +108,7 @@ void SharedStub(void)
 
 // these macros get expanded (many times) in the file #included below
 #define STUB_ENTRY(n) \
-__attribute__((naked)) nsresult __stdcall nsXPTCStubBase::Stub##n() \
+MOZ_NAKED nsresult __stdcall nsXPTCStubBase::Stub##n() \
 { __asm mov ecx, n __asm jmp SharedStub }
 
 #define SENTINEL_ENTRY(n) \