base: refactor ImmediateCrashTest.ExpectedOpcodeSequence

This change refactors this test by:
1) Splitting out the library & bytecode-snarfing logic to a helper
   function
2) Deduplicating some code that existed on each platform, via use of
   a couple of typedefs
3) Has the test not hang onto the shared library if one of the gtest
   asserts fail - I don't think this is a problem in practice, but.

Bug: None
Change-Id: Iea33973b141de7cb5fe1aaeae5eb1d6773d31c06
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414684Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810407}

base/BUILD.gn

@@ -3219,6 +3219,12 @@ test("base_unittests") {
       "strings/sys_string_conversions_mac_unittest.mm",
     ]
 
+    sources -= [
+      # The immediate crash tests require shared libraries, which we don't
+      # support loading on iOS.
+      "immediate_crash_unittest.cc",
+    ]
+
     if (use_allocator_shim) {
       sources += [
         "allocator/allocator_interception_mac_unittest.mm",

base/immediate_crash_unittest.cc

@@ -6,35 +6,94 @@
 
 #include <stdint.h>
 
-#include <algorithm>
-
 #include "base/base_paths.h"
+#include "base/compiler_specific.h"
 #include "base/containers/span.h"
 #include "base/files/file_path.h"
-#include "base/native_library.h"
 #include "base/optional.h"
 #include "base/path_service.h"
+#include "base/scoped_native_library.h"
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace base {
 
+namespace {
+
 // Compile test.
-int TestImmediateCrashTreatedAsNoReturn() {
+int ALLOW_UNUSED_TYPE TestImmediateCrashTreatedAsNoReturn() {
   IMMEDIATE_CRASH();
 }
 
-// iOS is excluded, since it doesn't support loading shared libraries.
-#if defined(OS_WIN) || defined(OS_MAC) || defined(OS_LINUX) || \
-    defined(OS_ANDROID) || defined(OS_CHROMEOS) || defined(OS_FUCHSIA)
+#if defined(ARCH_CPU_X86_FAMILY)
+// This is tricksy and false, since x86 instructions are not all one byte long,
+// but there is no better alternative short of implementing an x86 instruction
+// decoder.
+using Instruction = uint8_t;
+
+// https://software.intel.com/en-us/download/intel-64-and-ia-32-architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4
+// Look for RET opcode (0xc3). Note that 0xC3 is a substring of several
+// other opcodes (VMRESUME, MOVNTI), and can also be encoded as part of an
+// argument to another opcode. None of these other cases are expected to be
+// present, so a simple byte scan should be Good Enough™.
+constexpr Instruction kRet = 0xc3;
+// INT3 ; UD2
+constexpr Instruction kRequiredBody[] = {0xcc, 0x0f, 0x0b};
+constexpr Instruction kOptionalFooter[] = {};
 
-// Checks that the IMMEDIATE_CRASH() macro produces specific instructions; see
-// comments in immediate_crash.h for the requirements.
-TEST(ImmediateCrashTest, ExpectedOpcodeSequence) {
-  // TestFunction1() and TestFunction2() are defined in a shared library in an
-  // attempt to guarantee that they are located next to each other.
-  NativeLibraryLoadError load_error;
+#elif defined(ARCH_CPU_ARMEL)
+using Instruction = uint16_t;
+
+// T32 opcode reference: https://developer.arm.com/docs/ddi0487/latest
+// Actually BX LR, canonical encoding:
+constexpr Instruction kRet = 0x4770;
+// BKPT #0; UDF #0
+constexpr Instruction kRequiredBody[] = {0xbe00, 0xde00};
+constexpr Instruction kOptionalFooter[] = {};
+
+#elif defined(ARCH_CPU_ARM64)
+using Instruction = uint32_t;
+
+// A64 opcode reference: https://developer.arm.com/docs/ddi0487/latest
+// Use an enum here rather than separate constexpr vars because otherwise some
+// of the vars will end up unused on each platform, upsetting
+// -Wunused-const-variable.
+enum {
+  // There are multiple valid encodings of return (which is really a special
+  // form of branch). This is the one clang seems to use:
+  kRet = 0xd65f03c0,
+  kBrk0 = 0xd4200000,
+  kBrk1 = 0xd4200020,
+  kBrkF000 = 0xd43e0000,
+  kHlt0 = 0xd4400000,
+};
+
+#if defined(OS_WIN)
+
+constexpr Instruction kRequiredBody[] = {kBrkF000, kBrk1};
+constexpr Instruction kOptionalFooter[] = {};
+
+#elif defined(OS_MAC)
+
+constexpr Instruction kRequiredBody[] = {kBrk0, kHlt0};
+// Some clangs emit a BRK #1 for __builtin_unreachable(), but some do not, so
+// it is allowed but not required to occur.
+constexpr Instruction kOptionalFooter[] = {kBrk1};
+
+#else
+
+constexpr Instruction kRequiredBody[] = {kBrk0, kHlt0};
+constexpr Instruction kOptionalFooter[] = {};
+
+#endif
+
+#endif
+
+// This function loads a shared library that defines two functions,
+// TestFunction1 and TestFunction2. It then returns the bytes of the body of
+// whichever of those functions happens to come first in the library.
+void GetTestFunctionInstructions(std::vector<Instruction>* body) {
   FilePath helper_library_path;
 #if !defined(OS_ANDROID) && !defined(OS_FUCHSIA)
   // On Android M, DIR_EXE == /system/bin when running base_unittests.
@@ -47,53 +106,17 @@ TEST(ImmediateCrashTest, ExpectedOpcodeSequence) {
 #if defined(OS_ANDROID) && defined(COMPONENT_BUILD)
   helper_library_path = helper_library_path.ReplaceExtension(".cr.so");
 #endif
-  // TODO(dcheng): Shouldn't GetNativeLibraryName just return a FilePath?
-  NativeLibrary helper_library =
-      LoadNativeLibrary(helper_library_path, &load_error);
-  ASSERT_TRUE(helper_library)
-      << "shared library load failed: " << load_error.ToString();
-
-  // TestFunction1() and TestFunction2() each contain two IMMEDIATE_CRASH()
-  // invocations. IMMEDIATE_CRASH() should be treated as a noreturn sequence and
-  // optimized into the function epilogue. The general strategy is to find the
-  // return opcode, then scan the following bytes for the opcodes for two
-  // consecutive IMMEDIATE_CRASH() sequences.
-  void* a =
-      GetFunctionPointerFromNativeLibrary(helper_library, "TestFunction1");
+  ScopedNativeLibrary helper_library(helper_library_path);
+  ASSERT_TRUE(helper_library.is_valid())
+      << "shared library load failed: "
+      << helper_library.GetError()->ToString();
+
+  void* a = helper_library.GetFunctionPointer("TestFunction1");
   ASSERT_TRUE(a);
-  void* b =
-      GetFunctionPointerFromNativeLibrary(helper_library, "TestFunction2");
+  void* b = helper_library.GetFunctionPointer("TestFunction2");
   ASSERT_TRUE(b);
 
-#if defined(ARCH_CPU_X86_FAMILY)
-
-  // X86 opcode reference:
-  // https://software.intel.com/en-us/download/intel-64-and-ia-32-architectures-sdm-combined-volumes-1-2a-2b-2c-2d-3a-3b-3c-3d-and-4
-  span<const uint8_t> function_body =
-      a < b ? make_span(static_cast<const uint8_t*>(a),
-                        static_cast<const uint8_t*>(b))
-            : make_span(static_cast<const uint8_t*>(b),
-                        static_cast<const uint8_t*>(a));
-  SCOPED_TRACE(HexEncode(function_body.data(), function_body.size_bytes()));
-
-  // Look for RETN opcode (0xC3). Note that 0xC3 is a substring of several
-  // other opcodes (VMRESUME, MOVNTI), and can also be encoded as part of an
-  // argument to another opcode. None of these other cases are expected to be
-  // present, so a simple byte scan should be Good Enough™.
-  auto it = std::find(function_body.begin(), function_body.end(), 0xC3);
-  ASSERT_NE(function_body.end(), it) << "Failed to find return! ";
-
-  // Look for two IMMEDIATE_CRASH() opcode sequences.
-  for (int i = 0; i < 2; ++i) {
-    // INT 3
-    EXPECT_EQ(0xCC, *++it);
-    // UD2
-    EXPECT_EQ(0x0F, *++it);
-    EXPECT_EQ(0x0B, *++it);
-  }
-
-#elif defined(ARCH_CPU_ARMEL)
-
+#if defined(ARCH_CPU_ARMEL)
   // Routines loaded from a shared library will have the LSB in the pointer set
   // if encoded as T32 instructions. The rest of this test assumes T32.
   ASSERT_TRUE(reinterpret_cast<uintptr_t>(a) & 0x1)
@@ -104,79 +127,60 @@ TEST(ImmediateCrashTest, ExpectedOpcodeSequence) {
   // Mask off the lowest bit.
   a = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(a) & ~uintptr_t{0x1});
   b = reinterpret_cast<void*>(reinterpret_cast<uintptr_t>(b) & ~uintptr_t{0x1});
+#endif
 
-  // T32 opcode reference: https://developer.arm.com/docs/ddi0487/latest
-  span<const uint16_t> function_body =
-      a < b ? make_span(static_cast<const uint16_t*>(a),
-                        static_cast<const uint16_t*>(b))
-            : make_span(static_cast<const uint16_t*>(b),
-                        static_cast<const uint16_t*>(a));
-  SCOPED_TRACE(HexEncode(function_body.data(), function_body.size_bytes()));
-
-  // Look for the standard return opcode sequence (BX LR).
-  auto it = std::find(function_body.begin(), function_body.end(), 0x4770);
-  ASSERT_NE(function_body.end(), it) << "Failed to find return! ";
-
-  // Look for two IMMEDIATE_CRASH() opcode sequences.
-  for (int i = 0; i < 2; ++i) {
-    // BKPT #0
-    EXPECT_EQ(0xBE00, *++it);
-    // UDF #0
-    EXPECT_EQ(0xDE00, *++it);
-  }
-
-#elif defined(ARCH_CPU_ARM64)
-
-  // A64 opcode reference: https://developer.arm.com/docs/ddi0487/latest
-  span<const uint32_t> function_body =
-      a < b ? make_span(static_cast<const uint32_t*>(a),
-                        static_cast<const uint32_t*>(b))
-            : make_span(static_cast<const uint32_t*>(b),
-                        static_cast<const uint32_t*>(a));
-  SCOPED_TRACE(HexEncode(function_body.data(), function_body.size_bytes()));
-
-  // Look for RET. There appears to be multiple valid encodings, so this is
-  // hardcoded to whatever clang currently emits...
-  auto it = std::find(function_body.begin(), function_body.end(), 0XD65F03C0);
-  ASSERT_NE(function_body.end(), it) << "Failed to find return! ";
-
-  // Look for two IMMEDIATE_CRASH() opcode sequences.
-  for (int i = 0; i < 2; ++i) {
-
-#if defined(OS_WIN)
-
-    // BRK #F000
-    EXPECT_EQ(0XD43E0000, *++it);
-    // BRK #1
-    EXPECT_EQ(0XD4200020, *++it);
-
-#elif defined(OS_MAC)
-
-    // BRK #0
-    EXPECT_EQ(0XD4200000, *++it);
-    // HLT #0
-    EXPECT_EQ(0XD4400000, *++it);
-
-    // Allow, but do not require, a BRK #1 after the HLT; some clangs emit this
-    // for __builtin_unreachable() but some do not.
-    if (*++it != 0XD4200020)
-      --it;
+  // There are two identical test functions starting at a and b, which may
+  // occur in the library in either order. Grab whichever one comes first,
+  // and use the address of the other to figure out where it ends.
+  const Instruction* const start = static_cast<Instruction*>(std::min(a, b));
+  const Instruction* const end = static_cast<Instruction*>(std::max(a, b));
 
-#else
+  for (const Instruction& instruction : make_span(start, end))
+    body->push_back(instruction);
+}
 
-    // BRK #0
-    EXPECT_EQ(0XD4200000, *++it);
-    // HLT #0
-    EXPECT_EQ(0xD4400000, *++it);
+base::Optional<std::vector<Instruction>> ExpectImmediateCrashInvocation(
+    std::vector<Instruction> instructions) {
+  auto iter = instructions.begin();
+  for (const auto inst : kRequiredBody) {
+    if (iter == instructions.end())
+      return base::nullopt;
+    EXPECT_EQ(inst, *iter);
+    iter++;
+  }
+  return base::make_optional(
+      std::vector<Instruction>(iter, instructions.end()));
+}
 
-#endif  //  defined(OS_WIN)
+std::vector<Instruction> MaybeSkipOptionalFooter(
+    std::vector<Instruction> instructions) {
+  auto iter = instructions.begin();
+  for (const auto inst : kOptionalFooter) {
+    if (iter == instructions.end() || *iter != inst)
+      break;
+    iter++;
   }
+  return std::vector<Instruction>(iter, instructions.end());
+}
 
-#endif  // defined(ARCH_CPU_X86_FAMILY)
+}  // namespace
 
-  UnloadNativeLibrary(helper_library);
+// Checks that the IMMEDIATE_CRASH() macro produces specific instructions; see
+// comments in immediate_crash.h for the requirements.
+TEST(ImmediateCrashTest, ExpectedOpcodeSequence) {
+  std::vector<Instruction> body;
+  ASSERT_NO_FATAL_FAILURE(GetTestFunctionInstructions(&body));
+  SCOPED_TRACE(HexEncode(body.data(), body.size() * sizeof(Instruction)));
+
+  auto it = std::find(body.begin(), body.end(), kRet);
+  ASSERT_NE(body.end(), it) << "Failed to find return opcode";
+  it++;
+
+  body = std::vector<Instruction>(it, body.end());
+  auto result = ExpectImmediateCrashInvocation(body);
+  result = MaybeSkipOptionalFooter(result.value());
+  result = ExpectImmediateCrashInvocation(result.value());
+  ASSERT_TRUE(result);
 }
 
-#endif
-
 }  // namespace base