fix: get CFI tests passing for Windows on Arm

Corrects a problem with the inline assembly formatting on Windows
(Clang's assembler behaves differently with line-breaks vs semi-colons
on Windows than it does on other systems - was incorrectly squashing
nops). It also ensures that the right number of instructions are
skipped to avoid an instruction alignment crash when the linker's
configured incorrectly.

Bug: 1108769
Change-Id: I1f1f1ac7866c72d62068ad1db87f12c08204303c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2320570Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Richard Townsend <richard.townsend@arm.com>
Cr-Commit-Position: refs/heads/master@{#792378}

base/debug/invalid_access_win.cc

@@ -10,6 +10,7 @@
 
 #include "base/check.h"
 #include "base/win/windows_version.h"
+#include "build/build_config.h"
 
 namespace base {
 namespace debug {
@@ -17,9 +18,21 @@ namespace win {
 
 namespace {
 
+#if defined(ARCH_CPU_X86_FAMILY)
+// On x86/x64 systems, nop instructions are generally 1 byte.
+static constexpr int kNopInstructionSize = 1;
+#elif defined(ARCH_CPU_ARM64)
+// On Arm systems, all instructions are 4 bytes, fixed size.
+static constexpr int kNopInstructionSize = 4;
+#else
+#error "Unsupported architecture"
+#endif
+
 // Function that can be jumped midway into safely.
 __attribute__((naked)) int nop_sled() {
-  asm("nop; nop; ret");
+  asm("nop\n"
+      "nop\n"
+      "ret\n");
 }
 
 using FuncType = decltype(&nop_sled);
@@ -66,8 +79,8 @@ void TerminateWithHeapCorruption() {
 
 void TerminateWithControlFlowViolation() {
   // Call into the middle of the NOP sled.
-  FuncType func =
-      reinterpret_cast<FuncType>((reinterpret_cast<uintptr_t>(nop_sled)) + 0x1);
+  FuncType func = reinterpret_cast<FuncType>(
+      (reinterpret_cast<uintptr_t>(nop_sled)) + kNopInstructionSize);
   __try {
     // Generates a STATUS_STACK_BUFFER_OVERRUN exception if CFG triggers.
     IndirectCall(&func);

sandbox/win/tests/integration_tests/cfi_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/path_service.h"
 #include "base/win/windows_version.h"
+#include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace sandbox {
@@ -21,6 +22,16 @@ namespace sandbox {
 
 namespace {
 
+#if defined(ARCH_CPU_X86_FAMILY)
+// On x86/x64 systems, nop instructions are generally 1 byte.
+static constexpr int kNopInstructionSize = 1;
+#elif defined(ARCH_CPU_ARM64)
+// On Arm systems, all instructions are 4 bytes, fixed size.
+static constexpr int kNopInstructionSize = 4;
+#else
+#error "Unsupported architecture"
+#endif
+
 DWORD CALLBACK CopyProgressRoutine(LARGE_INTEGER total_file_size,
                                    LARGE_INTEGER total_bytes_transferred,
                                    LARGE_INTEGER stream_size,
@@ -30,11 +41,9 @@ DWORD CALLBACK CopyProgressRoutine(LARGE_INTEGER total_file_size,
                                    HANDLE source_file,
                                    HANDLE destination_file,
                                    LPVOID context) {
-  __asm {
-     nop
-     nop
-     ret
-  }
+  asm("nop\n"
+      "nop\n"
+      "ret\n");
   return PROGRESS_CONTINUE;
 }
 
@@ -57,7 +66,7 @@ TEST(CFGSupportTests, MsIndirectFailure) {
   // Create a bad callback pointer to midway into the callback function. This
   // should cause a CFG violation in MS code.
   auto bad_callback_func = reinterpret_cast<ProcessCallbackRoutineType>(
-      (reinterpret_cast<uintptr_t>(CopyProgressRoutine)) + 0x1);
+      (reinterpret_cast<uintptr_t>(CopyProgressRoutine)) + kNopInstructionSize);
 
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());