Revert "ART: Rewrite compiled code check in FaultHandler."

This reverts commit f65a0d10771ddd70cc2421c33f2071510cb5a4f2.

Reason for revert: Broke semi-space GC which holds the
mutator lock exclusively during `CleanupClassLoaders()`.

Bug: 38383823
Change-Id: I70afe1fd30eae29f3f28851b55c00b4eae588dad

9 files changed, 446 insertions, 517 deletions

diff --git a/runtime/arch/arm/fault_handler_arm.cc b/runtime/arch/arm/fault_handler_arm.cc
index 974e056f3a..7bd402f330 100644
--- a/runtime/arch/arm/fault_handler_arm.cc
+++ b/runtime/arch/arm/fault_handler_arm.cc
@@ -45,59 +45,76 @@ static uint32_t GetInstructionSize(uint8_t* pc) {
   return instr_size;
 }
 
-uintptr_t FaultManager::GetFaultPc(siginfo_t* siginfo ATTRIBUTE_UNUSED, void* context) {
+void FaultManager::GetMethodAndReturnPcAndSp(siginfo_t* siginfo ATTRIBUTE_UNUSED,
+                                             void* context,
+                                             ArtMethod** out_method,
+                                             uintptr_t* out_return_pc,
+                                             uintptr_t* out_sp,
+                                             bool* out_is_stack_overflow) {
   struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  if (sc->arm_sp == 0) {
-    VLOG(signals) << "Missing SP";
-    return 0u;
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
+  *out_sp = static_cast<uintptr_t>(sc->arm_sp);
+  VLOG(signals) << "sp: " << std::hex << *out_sp;
+  if (*out_sp == 0) {
+    return;
   }
-  return sc->arm_pc;
-}
-
-uintptr_t FaultManager::GetFaultSp(void* context) {
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  return sc->arm_sp;
-}
 
-bool NullPointerHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info, void* context) {
-  uintptr_t fault_address = reinterpret_cast<uintptr_t>(info->si_addr);
-  if (!IsValidFaultAddress(fault_address)) {
-    return false;
+  // In the case of a stack overflow, the stack is not valid and we can't
+  // get the method from the top of the stack.  However it's in r0.
+  uintptr_t* fault_addr = reinterpret_cast<uintptr_t*>(sc->fault_address);
+  uintptr_t* overflow_addr = reinterpret_cast<uintptr_t*>(
+      reinterpret_cast<uint8_t*>(*out_sp) - GetStackOverflowReservedBytes(InstructionSet::kArm));
+  if (overflow_addr == fault_addr) {
+    *out_method = reinterpret_cast<ArtMethod*>(sc->arm_r0);
+    *out_is_stack_overflow = true;
+  } else {
+    // The method is at the top of the stack.
+    *out_method = reinterpret_cast<ArtMethod*>(reinterpret_cast<uintptr_t*>(*out_sp)[0]);
+    *out_is_stack_overflow = false;
   }
 
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  ArtMethod** sp = reinterpret_cast<ArtMethod**>(sc->arm_sp);
-  if (!IsValidMethod(*sp)) {
-    return false;
+  // Work out the return PC.  This will be the address of the instruction
+  // following the faulting ldr/str instruction.  This is in thumb mode so
+  // the instruction might be a 16 or 32 bit one.  Also, the GC map always
+  // has the bottom bit of the PC set so we also need to set that.
+
+  // Need to work out the size of the instruction that caused the exception.
+  uint8_t* ptr = reinterpret_cast<uint8_t*>(sc->arm_pc);
+  VLOG(signals) << "pc: " << std::hex << static_cast<void*>(ptr);
+
+  if (ptr == nullptr) {
+    // Somebody jumped to 0x0. Definitely not ours, and will definitely segfault below.
+    *out_method = nullptr;
+    return;
   }
 
-  // For null checks in compiled code we insert a stack map that is immediately
-  // after the load/store instruction that might cause the fault and we need to
-  // pass the return PC to the handler. For null checks in Nterp, we similarly
-  // need the return PC to recognize that this was a null check in Nterp, so
-  // that the handler can get the needed data from the Nterp frame.
+  uint32_t instr_size = GetInstructionSize(ptr);
+
+  *out_return_pc = (sc->arm_pc + instr_size) | 1;
+}
 
-  // Note: Currently, Nterp is compiled to the A32 instruction set and managed
-  // code is compiled to the T32 instruction set.
-  // To find the stack map for compiled code, we need to set the bottom bit in
-  // the return PC indicating T32 just like we would if we were going to return
-  // to that PC (though we're going to jump to the exception handler instead).
+bool NullPointerHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info, void* context) {
+  if (!IsValidImplicitCheck(info)) {
+    return false;
+  }
+  // The code that looks for the catch location needs to know the value of the
+  // ARM PC at the point of call.  For Null checks we insert a GC map that is immediately after
+  // the load/store instruction that might cause the fault.  However the mapping table has
+  // the low bits set for thumb mode so we need to set the bottom bit for the LR
+  // register in order to find the mapping.
 
   // Need to work out the size of the instruction that caused the exception.
+  struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
   uint8_t* ptr = reinterpret_cast<uint8_t*>(sc->arm_pc);
   bool in_thumb_mode = sc->arm_cpsr & (1 << 5);
   uint32_t instr_size = in_thumb_mode ? GetInstructionSize(ptr) : 4;
-  uintptr_t return_pc = (sc->arm_pc + instr_size) | (in_thumb_mode ? 1 : 0);
+  uintptr_t gc_map_location = (sc->arm_pc + instr_size) | (in_thumb_mode ? 1 : 0);
 
-  // Push the return PC to the stack and pass the fault address in LR.
+  // Push the gc map location to the stack and pass the fault address in LR.
   sc->arm_sp -= sizeof(uintptr_t);
-  *reinterpret_cast<uintptr_t*>(sc->arm_sp) = return_pc;
-  sc->arm_lr = fault_address;
-
-  // Arrange for the signal handler to return to the NPE entrypoint.
+  *reinterpret_cast<uintptr_t*>(sc->arm_sp) = gc_map_location;
+  sc->arm_lr = reinterpret_cast<uintptr_t>(info->si_addr);
   sc->arm_pc = reinterpret_cast<uintptr_t>(art_quick_throw_null_pointer_exception_from_signal);
   // Make sure the thumb bit is set as the handler is in thumb mode.
   sc->arm_cpsr = sc->arm_cpsr | (1 << 5);
diff --git a/runtime/arch/arm64/fault_handler_arm64.cc b/runtime/arch/arm64/fault_handler_arm64.cc
index 963449268e..a5becf6b8e 100644
--- a/runtime/arch/arm64/fault_handler_arm64.cc
+++ b/runtime/arch/arm64/fault_handler_arm64.cc
@@ -38,56 +38,66 @@ extern "C" void art_quick_implicit_suspend();
 
 namespace art {
 
-uintptr_t FaultManager::GetFaultPc(siginfo_t* siginfo, void* context) {
+void FaultManager::GetMethodAndReturnPcAndSp(siginfo_t* siginfo,
+                                             void* context,
+                                             ArtMethod** out_method,
+                                             uintptr_t* out_return_pc,
+                                             uintptr_t* out_sp,
+                                             bool* out_is_stack_overflow) {
+  struct ucontext *uc = reinterpret_cast<struct ucontext *>(context);
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
+
   // SEGV_MTEAERR (Async MTE fault) is delivered at an arbitrary point after the actual fault.
   // Register contents, including PC and SP, are unrelated to the fault and can only confuse ART
   // signal handlers.
   if (siginfo->si_signo == SIGSEGV && siginfo->si_code == SEGV_MTEAERR) {
-    VLOG(signals) << "Async MTE fault";
-    return 0u;
+    return;
   }
 
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  if (sc->sp == 0) {
-    VLOG(signals) << "Missing SP";
-    return 0u;
+  *out_sp = static_cast<uintptr_t>(sc->sp);
+  VLOG(signals) << "sp: " << *out_sp;
+  if (*out_sp == 0) {
+    return;
+  }
+
+  // In the case of a stack overflow, the stack is not valid and we can't
+  // get the method from the top of the stack.  However it's in x0.
+  uintptr_t* fault_addr = reinterpret_cast<uintptr_t*>(sc->fault_address);
+  uintptr_t* overflow_addr = reinterpret_cast<uintptr_t*>(
+      reinterpret_cast<uint8_t*>(*out_sp) - GetStackOverflowReservedBytes(InstructionSet::kArm64));
+  if (overflow_addr == fault_addr) {
+    *out_method = reinterpret_cast<ArtMethod*>(sc->regs[0]);
+    *out_is_stack_overflow = true;
+  } else {
+    // The method is at the top of the stack.
+    *out_method = *reinterpret_cast<ArtMethod**>(*out_sp);
+    *out_is_stack_overflow = false;
   }
-  return sc->pc;
-}
 
-uintptr_t FaultManager::GetFaultSp(void* context) {
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  return sc->sp;
+  // Work out the return PC.  This will be the address of the instruction
+  // following the faulting ldr/str instruction.
+  VLOG(signals) << "pc: " << std::hex
+      << static_cast<void*>(reinterpret_cast<uint8_t*>(sc->pc));
+
+  *out_return_pc = sc->pc + 4;
 }
 
 bool NullPointerHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info, void* context) {
-  uintptr_t fault_address = reinterpret_cast<uintptr_t>(info->si_addr);
-  if (!IsValidFaultAddress(fault_address)) {
+  if (!IsValidImplicitCheck(info)) {
     return false;
   }
+  // The code that looks for the catch location needs to know the value of the
+  // PC at the point of call.  For Null checks we insert a GC map that is immediately after
+  // the load/store instruction that might cause the fault.
 
-  // For null checks in compiled code we insert a stack map that is immediately
-  // after the load/store instruction that might cause the fault and we need to
-  // pass the return PC to the handler. For null checks in Nterp, we similarly
-  // need the return PC to recognize that this was a null check in Nterp, so
-  // that the handler can get the needed data from the Nterp frame.
-
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
-  ArtMethod** sp = reinterpret_cast<ArtMethod**>(sc->sp);
-  uintptr_t return_pc = sc->pc + 4u;
-  if (!IsValidMethod(*sp) || !IsValidReturnPc(sp, return_pc)) {
-    return false;
-  }
+  struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
 
-  // Push the return PC to the stack and pass the fault address in LR.
+  // Push the gc map location to the stack and pass the fault address in LR.
   sc->sp -= sizeof(uintptr_t);
-  *reinterpret_cast<uintptr_t*>(sc->sp) = return_pc;
-  sc->regs[30] = fault_address;
+  *reinterpret_cast<uintptr_t*>(sc->sp) = sc->pc + 4;
+  sc->regs[30] = reinterpret_cast<uintptr_t>(info->si_addr);
 
-  // Arrange for the signal handler to return to the NPE entrypoint.
   sc->pc = reinterpret_cast<uintptr_t>(art_quick_throw_null_pointer_exception_from_signal);
   VLOG(signals) << "Generating null pointer exception";
   return true;
@@ -102,11 +112,12 @@ bool SuspensionHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info ATTRIBU
   constexpr uint32_t checkinst =
       0xf9400000 | (kSuspendCheckRegister << 5) | (kSuspendCheckRegister << 0);
 
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
+  struct ucontext *uc = reinterpret_cast<struct ucontext *>(context);
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
+  VLOG(signals) << "checking suspend";
 
   uint32_t inst = *reinterpret_cast<uint32_t*>(sc->pc);
-  VLOG(signals) << "checking suspend; inst: " << std::hex << inst << " checkinst: " << checkinst;
+  VLOG(signals) << "inst: " << std::hex << inst << " checkinst: " << checkinst;
   if (inst != checkinst) {
     // The instruction is not good, not ours.
     return false;
@@ -130,8 +141,8 @@ bool SuspensionHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info ATTRIBU
 
 bool StackOverflowHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* info ATTRIBUTE_UNUSED,
                                   void* context) {
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  struct sigcontext* sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
+  struct ucontext *uc = reinterpret_cast<struct ucontext *>(context);
+  struct sigcontext *sc = reinterpret_cast<struct sigcontext*>(&uc->uc_mcontext);
   VLOG(signals) << "stack overflow handler with sp at " << std::hex << &uc;
   VLOG(signals) << "sigcontext: " << std::hex << sc;
 
diff --git a/runtime/arch/x86/fault_handler_x86.cc b/runtime/arch/x86/fault_handler_x86.cc
index c485f0d707..3a08ec5cd1 100644
--- a/runtime/arch/x86/fault_handler_x86.cc
+++ b/runtime/arch/x86/fault_handler_x86.cc
@@ -25,7 +25,6 @@
 #include "base/logging.h"  // For VLOG.
 #include "base/macros.h"
 #include "base/safe_copy.h"
-#include "oat_quick_method_header.h"
 #include "runtime_globals.h"
 #include "thread-current-inl.h"
 
@@ -78,18 +77,30 @@ extern "C" void art_quick_test_suspend();
 
 // Get the size of an instruction in bytes.
 // Return 0 if the instruction is not handled.
-static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
-#define FETCH_OR_SKIP_BYTE(assignment)  \
-  do {                                  \
-    if (bytes == 0u) {                  \
-      return 0u;                        \
-    }                                   \
-    (assignment);                       \
-    ++pc;                               \
-    --bytes;                            \
+static uint32_t GetInstructionSize(const uint8_t* pc) {
+  // Don't segfault if pc points to garbage.
+  char buf[15];  // x86/x86-64 have a maximum instruction length of 15 bytes.
+  ssize_t bytes = SafeCopy(buf, pc, sizeof(buf));
+
+  if (bytes == 0) {
+    // Nothing was readable.
+    return 0;
+  }
+
+  if (bytes == -1) {
+    // SafeCopy not supported, assume that the entire range is readable.
+    bytes = 16;
+  } else {
+    pc = reinterpret_cast<uint8_t*>(buf);
+  }
+
+#define INCREMENT_PC()          \
+  do {                          \
+    pc++;                       \
+    if (pc - startpc > bytes) { \
+      return 0;                 \
+    }                           \
   } while (0)
-#define FETCH_BYTE(var) FETCH_OR_SKIP_BYTE((var) = *pc)
-#define SKIP_BYTE() FETCH_OR_SKIP_BYTE((void)0)
 
 #if defined(__x86_64)
   const bool x86_64 = true;
@@ -99,8 +110,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
 
   const uint8_t* startpc = pc;
 
-  uint8_t opcode;
-  FETCH_BYTE(opcode);
+  uint8_t opcode = *pc;
+  INCREMENT_PC();
   uint8_t modrm;
   bool has_modrm = false;
   bool two_byte = false;
@@ -132,7 +143,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
 
       // Group 4
       case 0x67:
-        FETCH_BYTE(opcode);
+        opcode = *pc;
+        INCREMENT_PC();
         prefix_present = true;
         break;
     }
@@ -142,13 +154,15 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
   }
 
   if (x86_64 && opcode >= 0x40 && opcode <= 0x4f) {
-    FETCH_BYTE(opcode);
+    opcode = *pc;
+    INCREMENT_PC();
   }
 
   if (opcode == 0x0f) {
     // Two byte opcode
     two_byte = true;
-    FETCH_BYTE(opcode);
+    opcode = *pc;
+    INCREMENT_PC();
   }
 
   bool unhandled_instruction = false;
@@ -161,7 +175,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
       case 0xb7:
       case 0xbe:        // movsx
       case 0xbf:
-        FETCH_BYTE(modrm);
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         break;
       default:
@@ -180,28 +195,32 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
       case 0x3c:
       case 0x3d:
       case 0x85:        // test.
-        FETCH_BYTE(modrm);
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         break;
 
       case 0x80:        // group 1, byte immediate.
       case 0x83:
       case 0xc6:
-        FETCH_BYTE(modrm);
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         immediate_size = 1;
         break;
 
       case 0x81:        // group 1, word immediate.
       case 0xc7:        // mov
-        FETCH_BYTE(modrm);
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         immediate_size = operand_size_prefix ? 2 : 4;
         break;
 
       case 0xf6:
       case 0xf7:
-        FETCH_BYTE(modrm);
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         switch ((modrm >> 3) & 7) {  // Extract "reg/opcode" from "modr/m".
           case 0:  // test
@@ -236,7 +255,7 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
 
     // Check for SIB.
     if (mod != 3U /* 0b11 */ && (modrm & 7U /* 0b111 */) == 4) {
-      SKIP_BYTE();  // SIB
+      INCREMENT_PC();     // SIB
     }
 
     switch (mod) {
@@ -252,79 +271,86 @@ static uint32_t GetInstructionSize(const uint8_t* pc, size_t bytes) {
   pc += displacement_size + immediate_size;
 
   VLOG(signals) << "x86 instruction length calculated as " << (pc - startpc);
+  if (pc - startpc > bytes) {
+    return 0;
+  }
   return pc - startpc;
-
-#undef SKIP_BYTE
-#undef FETCH_BYTE
-#undef FETCH_OR_SKIP_BYTE
 }
 
-uintptr_t FaultManager::GetFaultPc(siginfo_t* siginfo ATTRIBUTE_UNUSED, void* context) {
+void FaultManager::GetMethodAndReturnPcAndSp(siginfo_t* siginfo, void* context,
+                                             ArtMethod** out_method,
+                                             uintptr_t* out_return_pc,
+                                             uintptr_t* out_sp,
+                                             bool* out_is_stack_overflow) {
   struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  if (uc->CTX_ESP == 0) {
-    VLOG(signals) << "Missing SP";
-    return 0u;
+  *out_sp = static_cast<uintptr_t>(uc->CTX_ESP);
+  VLOG(signals) << "sp: " << std::hex << *out_sp;
+  if (*out_sp == 0) {
+    return;
   }
-  return uc->CTX_EIP;
-}
-
-uintptr_t FaultManager::GetFaultSp(void* context) {
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  return uc->CTX_ESP;
-}
 
-bool NullPointerHandler::Action(int, siginfo_t* sig, void* context) {
-  uintptr_t fault_address = reinterpret_cast<uintptr_t>(sig->si_addr);
-  if (!IsValidFaultAddress(fault_address)) {
-    return false;
+  // In the case of a stack overflow, the stack is not valid and we can't
+  // get the method from the top of the stack.  However it's in EAX(x86)/RDI(x86_64).
+  uintptr_t* fault_addr = reinterpret_cast<uintptr_t*>(siginfo->si_addr);
+  uintptr_t* overflow_addr = reinterpret_cast<uintptr_t*>(
+#if defined(__x86_64__)
+      reinterpret_cast<uint8_t*>(*out_sp) - GetStackOverflowReservedBytes(InstructionSet::kX86_64));
+#else
+      reinterpret_cast<uint8_t*>(*out_sp) - GetStackOverflowReservedBytes(InstructionSet::kX86));
+#endif
+  if (overflow_addr == fault_addr) {
+    *out_method = reinterpret_cast<ArtMethod*>(uc->CTX_METHOD);
+    *out_is_stack_overflow = true;
+  } else {
+    // The method is at the top of the stack.
+    *out_method = *reinterpret_cast<ArtMethod**>(*out_sp);
+    *out_is_stack_overflow = false;
   }
 
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
-  ArtMethod** sp = reinterpret_cast<ArtMethod**>(uc->CTX_ESP);
-  ArtMethod* method = *sp;
-  if (!IsValidMethod(method)) {
-    return false;
+  uint8_t* pc = reinterpret_cast<uint8_t*>(uc->CTX_EIP);
+  VLOG(signals) << HexDump(pc, 32, true, "PC ");
+
+  if (pc == nullptr) {
+    // Somebody jumped to 0x0. Definitely not ours, and will definitely segfault below.
+    *out_method = nullptr;
+    return;
   }
 
-  // For null checks in compiled code we insert a stack map that is immediately
-  // after the load/store instruction that might cause the fault and we need to
-  // pass the return PC to the handler. For null checks in Nterp, we similarly
-  // need the return PC to recognize that this was a null check in Nterp, so
-  // that the handler can get the needed data from the Nterp frame.
-
-  // Note: Allowing nested faults if `IsValidMethod()` returned a false positive.
-  // Note: The `ArtMethod::GetOatQuickMethodHeader()` can acquire locks, which is
-  // essentially unsafe in a signal handler, but we allow that here just like in
-  // `NullPointerHandler::IsValidReturnPc()`. For more details see comments there.
-  uintptr_t pc = uc->CTX_EIP;
-  const OatQuickMethodHeader* method_header = method->GetOatQuickMethodHeader(pc);
-  if (method_header == nullptr) {
-    VLOG(signals) << "No method header.";
-    return false;
+  uint32_t instr_size = GetInstructionSize(pc);
+  if (instr_size == 0) {
+    // Unknown instruction, tell caller it's not ours.
+    *out_method = nullptr;
+    return;
   }
-  const uint8_t* pc_ptr = reinterpret_cast<const uint8_t*>(pc);
-  size_t offset = pc_ptr - method_header->GetCode();
-  size_t code_size = method_header->GetCodeSize();
-  CHECK_LT(offset, code_size);
-  size_t max_instr_size = code_size - offset;
-  uint32_t instr_size = GetInstructionSize(pc_ptr, max_instr_size);
-  if (instr_size == 0u) {
-    // Unknown instruction (can't really happen) or not enough bytes until end of method code.
+  *out_return_pc = reinterpret_cast<uintptr_t>(pc + instr_size);
+}
+
+bool NullPointerHandler::Action(int, siginfo_t* sig, void* context) {
+  if (!IsValidImplicitCheck(sig)) {
     return false;
   }
+  struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
+  uint8_t* pc = reinterpret_cast<uint8_t*>(uc->CTX_EIP);
+  uint8_t* sp = reinterpret_cast<uint8_t*>(uc->CTX_ESP);
 
-  uintptr_t return_pc = reinterpret_cast<uintptr_t>(pc + instr_size);
-  if (!IsValidReturnPc(sp, return_pc)) {
+  uint32_t instr_size = GetInstructionSize(pc);
+  if (instr_size == 0) {
+    // Unknown instruction, can't really happen.
     return false;
   }
 
-  // Push the return PC and fault address onto the stack.
-  uintptr_t* next_sp = reinterpret_cast<uintptr_t*>(sp) - 2;
-  next_sp[1] = return_pc;
-  next_sp[0] = fault_address;
+  // We need to arrange for the signal handler to return to the null pointer
+  // exception generator.  The return address must be the address of the
+  // next instruction (this instruction + instruction size).  The return address
+  // is on the stack at the top address of the current frame.
+
+  // Push the return address and fault address onto the stack.
+  uintptr_t retaddr = reinterpret_cast<uintptr_t>(pc + instr_size);
+  uintptr_t* next_sp = reinterpret_cast<uintptr_t*>(sp - 2 * sizeof(uintptr_t));
+  next_sp[1] = retaddr;
+  next_sp[0] = reinterpret_cast<uintptr_t>(sig->si_addr);
   uc->CTX_ESP = reinterpret_cast<uintptr_t>(next_sp);
 
-  // Arrange for the signal handler to return to the NPE entrypoint.
   uc->CTX_EIP = reinterpret_cast<uintptr_t>(
       art_quick_throw_null_pointer_exception_from_signal);
   VLOG(signals) << "Generating null pointer exception";
@@ -359,7 +385,7 @@ bool SuspensionHandler::Action(int, siginfo_t*, void* context) {
 #endif
   uint8_t checkinst2[] = {0x85, 0x00};
 
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
+  struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
   uint8_t* pc = reinterpret_cast<uint8_t*>(uc->CTX_EIP);
   uint8_t* sp = reinterpret_cast<uint8_t*>(uc->CTX_ESP);
 
@@ -415,7 +441,7 @@ bool SuspensionHandler::Action(int, siginfo_t*, void* context) {
 // address for the previous method is on the stack at ESP.
 
 bool StackOverflowHandler::Action(int, siginfo_t* info, void* context) {
-  struct ucontext* uc = reinterpret_cast<struct ucontext*>(context);
+  struct ucontext *uc = reinterpret_cast<struct ucontext*>(context);
   uintptr_t sp = static_cast<uintptr_t>(uc->CTX_ESP);
 
   uintptr_t fault_addr = reinterpret_cast<uintptr_t>(info->si_addr);
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index dfe6257b0f..6480dfd249 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -3884,8 +3884,7 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file,
   std::string dex_file_location = dex_file.GetLocation();
   // The following paths checks don't work on preopt when using boot dex files, where the dex
   // cache location is the one on device, and the dex_file's location is the one on host.
-  Runtime* runtime = Runtime::Current();
-  if (!(runtime->IsAotCompiler() && class_loader == nullptr && !kIsTargetBuild)) {
+  if (!(Runtime::Current()->IsAotCompiler() && class_loader == nullptr && !kIsTargetBuild)) {
     CHECK_GE(dex_file_location.length(), dex_cache_length)
         << dex_cache_location << " " << dex_file.GetLocation();
     const std::string dex_file_suffix = dex_file_location.substr(
@@ -3897,7 +3896,7 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file,
   }
 
   // Check if we need to initialize OatFile data (.data.bimg.rel.ro and .bss
-  // sections) needed for code execution and register the oat code range.
+  // sections) needed for code execution.
   const OatFile* oat_file =
       (dex_file.GetOatDexFile() != nullptr) ? dex_file.GetOatDexFile()->GetOatFile() : nullptr;
   bool initialize_oat_file_data = (oat_file != nullptr) && oat_file->IsExecutable();
@@ -3913,13 +3912,6 @@ void ClassLinker::RegisterDexFileLocked(const DexFile& dex_file,
   }
   if (initialize_oat_file_data) {
     oat_file->InitializeRelocations();
-    // Notify the fault handler about the new executable code range if needed.
-    size_t exec_offset = oat_file->GetOatHeader().GetExecutableOffset();
-    DCHECK_LE(exec_offset, oat_file->Size());
-    size_t exec_size = oat_file->Size() - exec_offset;
-    if (exec_size != 0u) {
-      runtime->AddGeneratedCodeRange(oat_file->Begin() + exec_offset, exec_size);
-    }
   }
 
   // Let hiddenapi assign a domain to the newly registered dex file.
@@ -10342,23 +10334,16 @@ void ClassLinker::CleanupClassLoaders() {
       }
     }
   }
-  std::set<const OatFile*> unregistered_oat_files;
   if (!to_delete.empty()) {
     JavaVMExt* vm = self->GetJniEnv()->GetVm();
     WriterMutexLock mu(self, *Locks::dex_lock_);
     for (auto it = dex_caches_.begin(), end = dex_caches_.end(); it != end; ) {
-      const DexFile* dex_file = it->first;
       const DexCacheData& data = it->second;
       if (self->DecodeJObject(data.weak_root) == nullptr) {
         DCHECK(to_delete.end() != std::find_if(
             to_delete.begin(),
             to_delete.end(),
             [&](const ClassLoaderData& cld) { return cld.class_table == data.class_table; }));
-        if (dex_file->GetOatDexFile() != nullptr &&
-            dex_file->GetOatDexFile()->GetOatFile() != nullptr &&
-            dex_file->GetOatDexFile()->GetOatFile()->IsExecutable()) {
-          unregistered_oat_files.insert(dex_file->GetOatDexFile()->GetOatFile());
-        }
         vm->DeleteWeakGlobalRef(self, data.weak_root);
         it = dex_caches_.erase(it);
       } else {
@@ -10370,33 +10355,6 @@ void ClassLinker::CleanupClassLoaders() {
     // CHA unloading analysis and SingleImplementaion cleanups are required.
     DeleteClassLoader(self, data, /*cleanup_cha=*/ true);
   }
-  if (!unregistered_oat_files.empty()) {
-    // Removing the code range requires running an empty checkpoint and we cannot do
-    // that while holding the mutator lock. This thread is not currently `Runnable`
-    // for CC GC, so we need explicit unlock/lock instead of `ScopedThreadStateChange`.
-    // TODO: Clean up state transition in CC GC and possibly other GC types. b/259440389
-    bool runnable = (self->GetState() == ThreadState::kRunnable);
-    if (runnable) {
-      self->TransitionFromRunnableToSuspended(ThreadState::kNative);
-    } else {
-      Locks::mutator_lock_->SharedUnlock(self);
-    }
-    for (const OatFile* oat_file : unregistered_oat_files) {
-      // Notify the fault handler about removal of the executable code range if needed.
-      DCHECK(oat_file->IsExecutable());
-      size_t exec_offset = oat_file->GetOatHeader().GetExecutableOffset();
-      DCHECK_LE(exec_offset, oat_file->Size());
-      size_t exec_size = oat_file->Size() - exec_offset;
-      if (exec_size != 0u) {
-        Runtime::Current()->RemoveGeneratedCodeRange(oat_file->Begin() + exec_offset, exec_size);
-      }
-    }
-    if (runnable) {
-      self->TransitionFromSuspendedToRunnable();
-    } else {
-      Locks::mutator_lock_->SharedLock(self);
-    }
-  }
 }
 
 class ClassLinker::FindVirtualMethodHolderVisitor : public ClassVisitor {
diff --git a/runtime/fault_handler.cc b/runtime/fault_handler.cc
index 08bbdb7210..f8bd213d53 100644
--- a/runtime/fault_handler.cc
+++ b/runtime/fault_handler.cc
@@ -16,14 +16,12 @@
 
 #include "fault_handler.h"
 
-#include <atomic>
 #include <string.h>
 #include <sys/mman.h>
 #include <sys/ucontext.h>
 
 #include "art_method-inl.h"
 #include "base/logging.h"  // For VLOG
-#include "base/membarrier.h"
 #include "base/safe_copy.h"
 #include "base/stl_util.h"
 #include "dex/dex_file_types.h"
@@ -53,16 +51,82 @@ static bool art_fault_handler(int sig, siginfo_t* info, void* context) {
   return fault_manager.HandleFault(sig, info, context);
 }
 
-struct FaultManager::GeneratedCodeRange {
-  std::atomic<GeneratedCodeRange*> next;
-  const void* start;
-  size_t size;
-};
+#if defined(__linux__)
 
-FaultManager::FaultManager()
-    : generated_code_ranges_lock_("FaultHandler generated code ranges lock",
-                                  LockLevel::kGenericBottomLock),
-      initialized_(false) {
+// Change to verify the safe implementations against the original ones.
+constexpr bool kVerifySafeImpls = false;
+
+// Provide implementations of ArtMethod::GetDeclaringClass and VerifyClassClass that use SafeCopy
+// to safely dereference pointers which are potentially garbage.
+// Only available on Linux due to availability of SafeCopy.
+
+static mirror::Class* SafeGetDeclaringClass(ArtMethod* method)
+    REQUIRES_SHARED(Locks::mutator_lock_) {
+  char* method_declaring_class =
+      reinterpret_cast<char*>(method) + ArtMethod::DeclaringClassOffset().SizeValue();
+
+  // ArtMethod::declaring_class_ is a GcRoot<mirror::Class>.
+  // Read it out into as a CompressedReference directly for simplicity's sake.
+  mirror::CompressedReference<mirror::Class> cls;
+  ssize_t rc = SafeCopy(&cls, method_declaring_class, sizeof(cls));
+  CHECK_NE(-1, rc);
+
+  if (kVerifySafeImpls) {
+    ObjPtr<mirror::Class> actual_class = method->GetDeclaringClassUnchecked<kWithoutReadBarrier>();
+    CHECK_EQ(actual_class, cls.AsMirrorPtr());
+  }
+
+  if (rc != sizeof(cls)) {
+    return nullptr;
+  }
+
+  return cls.AsMirrorPtr();
+}
+
+static mirror::Class* SafeGetClass(mirror::Object* obj) REQUIRES_SHARED(Locks::mutator_lock_) {
+  char* obj_cls = reinterpret_cast<char*>(obj) + mirror::Object::ClassOffset().SizeValue();
+
+  mirror::HeapReference<mirror::Class> cls;
+  ssize_t rc = SafeCopy(&cls, obj_cls, sizeof(cls));
+  CHECK_NE(-1, rc);
+
+  if (kVerifySafeImpls) {
+    mirror::Class* actual_class = obj->GetClass<kVerifyNone>();
+    CHECK_EQ(actual_class, cls.AsMirrorPtr());
+  }
+
+  if (rc != sizeof(cls)) {
+    return nullptr;
+  }
+
+  return cls.AsMirrorPtr();
+}
+
+static bool SafeVerifyClassClass(mirror::Class* cls) REQUIRES_SHARED(Locks::mutator_lock_) {
+  mirror::Class* c_c = SafeGetClass(cls);
+  bool result = c_c != nullptr && c_c == SafeGetClass(c_c);
+
+  if (kVerifySafeImpls) {
+    CHECK_EQ(VerifyClassClass(cls), result);
+  }
+
+  return result;
+}
+
+#else
+
+static mirror::Class* SafeGetDeclaringClass(ArtMethod* method_obj)
+    REQUIRES_SHARED(Locks::mutator_lock_) {
+  return method_obj->GetDeclaringClassUnchecked<kWithoutReadBarrier>().Ptr();
+}
+
+static bool SafeVerifyClassClass(mirror::Class* cls) REQUIRES_SHARED(Locks::mutator_lock_) {
+  return VerifyClassClass(cls);
+}
+#endif
+
+
+FaultManager::FaultManager() : initialized_(false) {
   sigaction(SIGSEGV, nullptr, &oldaction_);
 }
 
@@ -86,14 +150,6 @@ void FaultManager::Init() {
   };
 
   AddSpecialSignalHandlerFn(SIGSEGV, &sa);
-
-  // Notify the kernel that we intend to use a specific `membarrier()` command.
-  int result = art::membarrier(MembarrierCommand::kRegisterPrivateExpedited);
-  if (result != 0) {
-    LOG(WARNING) << "FaultHandler: MEMBARRIER_CMD_REGISTER_PRIVATE_EXPEDITED failed: "
-                 << errno << " " << strerror(errno);
-  }
-
   initialized_ = true;
 }
 
@@ -111,20 +167,6 @@ void FaultManager::Shutdown() {
     // Free all handlers.
     STLDeleteElements(&generated_code_handlers_);
     STLDeleteElements(&other_handlers_);
-
-    // Delete remaining code ranges if any (such as nterp code or oat code from
-    // oat files that have not been unloaded, including boot image oat files).
-    GeneratedCodeRange* range;
-    {
-      MutexLock lock(Thread::Current(), generated_code_ranges_lock_);
-      range = generated_code_ranges_.load(std::memory_order_acquire);
-      generated_code_ranges_.store(nullptr, std::memory_order_release);
-    }
-    while (range != nullptr) {
-      GeneratedCodeRange* next_range = range->next.load(std::memory_order_relaxed);
-      delete range;
-      range = next_range;
-    }
   }
 }
 
@@ -179,7 +221,7 @@ bool FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
   raise(SIGSEGV);
 #endif
 
-  if (IsInGeneratedCode(info, context)) {
+  if (IsInGeneratedCode(info, context, true)) {
     VLOG(signals) << "in generated code, looking for handler";
     for (const auto& handler : generated_code_handlers_) {
       VLOG(signals) << "invoking Action on handler " << handler;
@@ -226,84 +268,37 @@ void FaultManager::RemoveHandler(FaultHandler* handler) {
   LOG(FATAL) << "Attempted to remove non existent handler " << handler;
 }
 
-void FaultManager::AddGeneratedCodeRange(const void* start, size_t size) {
-  GeneratedCodeRange* new_range = new GeneratedCodeRange{nullptr, start, size};
-  {
-    MutexLock lock(Thread::Current(), generated_code_ranges_lock_);
-    GeneratedCodeRange* old_head = generated_code_ranges_.load(std::memory_order_relaxed);
-    new_range->next.store(old_head, std::memory_order_relaxed);
-    generated_code_ranges_.store(new_range, std::memory_order_release);
-  }
-
-  // The above release operation on `generated_code_ranges_` with an acquire operation
-  // on the same atomic object in `IsInGeneratedCode()` ensures the correct memory
-  // visibility for the contents of `*new_range` for any thread that loads the value
-  // written above (or a value written by a release sequence headed by that write).
-  //
-  // However, we also need to ensure that any thread that encounters a segmentation
-  // fault in the provided range shall actually see the written value. For JIT code
-  // cache and nterp, the registration happens while the process is single-threaded
-  // but the synchronization is more complicated for code in oat files.
-  //
-  // Threads that load classes register dex files under the `Locks::dex_lock_` and
-  // the first one to register a dex file with a given oat file shall add the oat
-  // code range; the memory visibility for these threads is guaranteed by the lock.
-  // However a thread that did not try to load a class with oat code can execute the
-  // code if a direct or indirect reference to such class escapes from one of the
-  // threads that loaded it. Use `membarrier()` for memory visibility in this case.
-  art::membarrier(MembarrierCommand::kPrivateExpedited);
-}
+static bool IsKnownPc(uintptr_t pc, ArtMethod* method) REQUIRES_SHARED(Locks::mutator_lock_) {
+  // Check whether the pc is within nterp range.
+  if (OatQuickMethodHeader::IsNterpPc(pc)) {
+    return true;
+  }
 
-void FaultManager::RemoveGeneratedCodeRange(const void* start, size_t size) {
-  Thread* self = Thread::Current();
-  GeneratedCodeRange* range = nullptr;
-  {
-    MutexLock lock(self, generated_code_ranges_lock_);
-    std::atomic<GeneratedCodeRange*>* before = &generated_code_ranges_;
-    range = before->load(std::memory_order_relaxed);
-    while (range != nullptr && range->start != start) {
-      before = &range->next;
-      range = before->load(std::memory_order_relaxed);
-    }
-    if (range != nullptr) {
-      GeneratedCodeRange* next = range->next.load(std::memory_order_relaxed);
-      if (before == &generated_code_ranges_) {
-        // Relaxed store directly to `generated_code_ranges_` would not satisfy
-        // conditions for a release sequence, so we need to use store-release.
-        before->store(next, std::memory_order_release);
-      } else {
-        // In the middle of the list, we can use a relaxed store as we're not
-        // publishing any newly written memory to potential reader threads.
-        // Whether they see the removed node or not is unimportant as we should
-        // not execute that code anymore. We're keeping the `next` link of the
-        // removed node, so that concurrent walk can use it to reach remaining
-        // retained nodes, if any.
-        before->store(next, std::memory_order_relaxed);
-      }
-    }
+  // Check whether the pc is in the JIT code cache.
+  jit::Jit* jit = Runtime::Current()->GetJit();
+  if (jit != nullptr && jit->GetCodeCache()->ContainsPc(reinterpret_cast<const void*>(pc))) {
+    return true;
+  }
+
+  if (method->IsObsolete()) {
+    // Obsolete methods never happen on AOT code.
+    return false;
   }
-  CHECK(range != nullptr);
-  DCHECK_EQ(range->start, start);
-  CHECK_EQ(range->size, size);
-
-  Runtime* runtime = Runtime::Current();
-  CHECK(runtime != nullptr);
-  if (runtime->IsStarted() && runtime->GetThreadList() != nullptr) {
-    // Run a checkpoint before deleting the range to ensure that no thread holds a
-    // pointer to the removed range while walking the list in `IsInGeneratedCode()`.
-    // That walk is guarded by checking that the thread is `Runnable`, so any walk
-    // started before the removal shall be done when running the checkpoint and the
-    // checkpoint also ensures the correct memory visibility of `next` links,
-    // so the thread shall not see the pointer during future walks.
-    runtime->GetThreadList()->RunEmptyCheckpoint();
-  }
-  delete range;
+
+  // Note: at this point, we trust it's truly an ArtMethod we found at the bottom of the stack,
+  // and we can find its oat file through it.
+  const OatDexFile* oat_dex_file = method->GetDeclaringClass()->GetDexFile().GetOatDexFile();
+  if (oat_dex_file != nullptr &&
+      oat_dex_file->GetOatFile()->Contains(reinterpret_cast<const void*>(pc))) {
+    return true;
+  }
+
+  return false;
 }
 
-// This function is called within the signal handler. It checks that the thread
-// is `Runnable`, the `mutator_lock_` is held (shared) and the fault PC is in one
-// of the registered generated code ranges. No annotalysis is done.
-bool FaultManager::IsInGeneratedCode(siginfo_t* siginfo, void* context) {
+// This function is called within the signal handler.  It checks that
+// the mutator_lock is held (shared).  No annotalysis is done.
+bool FaultManager::IsInGeneratedCode(siginfo_t* siginfo, void* context, bool check_dex_pc) {
   // We can only be running Java code in the current thread if it
   // is in Runnable state.
   VLOG(signals) << "Checking for generated code";
@@ -326,109 +321,86 @@ bool FaultManager::IsInGeneratedCode(siginfo_t* siginfo, void* context) {
     return false;
   }
 
-  uintptr_t fault_pc = GetFaultPc(siginfo, context);
-  if (fault_pc == 0u) {
-    VLOG(signals) << "no fault PC";
+  ArtMethod* method_obj = nullptr;
+  uintptr_t return_pc = 0;
+  uintptr_t sp = 0;
+  bool is_stack_overflow = false;
+
+  // Get the architecture specific method address and return address.  These
+  // are in architecture specific files in arch/<arch>/fault_handler_<arch>.
+  GetMethodAndReturnPcAndSp(siginfo, context, &method_obj, &return_pc, &sp, &is_stack_overflow);
+
+  // If we don't have a potential method, we're outta here.
+  VLOG(signals) << "potential method: " << method_obj;
+  // TODO: Check linear alloc and image.
+  DCHECK_ALIGNED(ArtMethod::Size(kRuntimePointerSize), sizeof(void*))
+      << "ArtMethod is not pointer aligned";
+  if (method_obj == nullptr || !IsAligned<sizeof(void*)>(method_obj)) {
+    VLOG(signals) << "no method";
     return false;
   }
 
-  // Walk over the list of registered code ranges.
-  GeneratedCodeRange* range = generated_code_ranges_.load(std::memory_order_acquire);
-  while (range != nullptr) {
-    if (fault_pc - reinterpret_cast<uintptr_t>(range->start) < range->size) {
-      return true;
-    }
-    // We may or may not see ranges that were concurrently removed, depending
-    // on when the relaxed writes of the `next` links become visible. However,
-    // even if we're currently at a node that is being removed, we shall visit
-    // all remaining ranges that are not being removed as the removed nodes
-    // retain the `next` link at the time of removal (which may lead to other
-    // removed nodes before reaching remaining retained nodes, if any). Correct
-    // memory visibility of `start` and `size` fields of the visited ranges is
-    // ensured by the release and acquire operations on `generated_code_ranges_`.
-    range = range->next.load(std::memory_order_relaxed);
-  }
-  return false;
-}
-
-FaultHandler::FaultHandler(FaultManager* manager) : manager_(manager) {
-}
-
-//
-// Null pointer fault handler
-//
-NullPointerHandler::NullPointerHandler(FaultManager* manager) : FaultHandler(manager) {
-  manager_->AddHandler(this, true);
-}
-
-bool NullPointerHandler::IsValidMethod(ArtMethod* method) {
-  // At this point we know that the thread is `Runnable` and the PC is in one of
-  // the registered code ranges. The `method` was read from the top of the stack
-  // and should really point to an actual `ArtMethod`, unless we're crashing during
-  // prologue or epilogue, or somehow managed to jump to the compiled code by some
-  // unexpected path, other than method invoke or exception delivery. We do a few
-  // quick checks without guarding from another fault.
-  VLOG(signals) << "potential method: " << method;
-
-  static_assert(IsAligned<sizeof(void*)>(ArtMethod::Size(kRuntimePointerSize)));
-  if (method == nullptr || !IsAligned<sizeof(void*)>(method)) {
-    VLOG(signals) << ((method == nullptr) ? "null method" : "unaligned method");
+  // Verify that the potential method is indeed a method.
+  // TODO: check the GC maps to make sure it's an object.
+  // Check that the class pointer inside the object is not null and is aligned.
+  // No read barrier because method_obj may not be a real object.
+  mirror::Class* cls = SafeGetDeclaringClass(method_obj);
+  if (cls == nullptr) {
+    VLOG(signals) << "not a class";
     return false;
   }
 
-  // Check that the presumed method actually points to a class. Read barriers
-  // are not needed (and would be undesirable in a signal handler) when reading
-  // a chain of constant references to get to a non-movable `Class.class` object.
-
-  // Note: Allowing nested faults. Checking that the method is in one of the
-  // `LinearAlloc` spaces, or that objects we look at are in the `Heap` would be
-  // slow and require locking a mutex, which is undesirable in a signal handler.
-  // (Though we could register valid ranges similarly to the generated code ranges.)
-
-  mirror::Object* klass =
-      method->GetDeclaringClassAddressWithoutBarrier()->AsMirrorPtr();
-  if (klass == nullptr || !IsAligned<kObjectAlignment>(klass)) {
-    VLOG(signals) << ((klass == nullptr) ? "null class" : "unaligned class");
+  if (!IsAligned<kObjectAlignment>(cls)) {
+    VLOG(signals) << "not aligned";
     return false;
   }
 
-  mirror::Class* class_class = klass->GetClass<kVerifyNone, kWithoutReadBarrier>();
-  if (class_class == nullptr || !IsAligned<kObjectAlignment>(class_class)) {
-    VLOG(signals) << ((klass == nullptr) ? "null class_class" : "unaligned class_class");
+  if (!SafeVerifyClassClass(cls)) {
+    VLOG(signals) << "not a class class";
     return false;
   }
 
-  if (class_class != class_class->GetClass<kVerifyNone, kWithoutReadBarrier>()) {
-    VLOG(signals) << "invalid class_class";
+  if (!IsKnownPc(return_pc, method_obj)) {
+    VLOG(signals) << "PC not in Java code";
     return false;
   }
 
-  return true;
-}
+  const OatQuickMethodHeader* method_header = method_obj->GetOatQuickMethodHeader(return_pc);
 
-bool NullPointerHandler::IsValidReturnPc(ArtMethod** sp, uintptr_t return_pc) {
-  // Check if we can associate a dex PC with the return PC, whether from Nterp,
-  // or with an existing stack map entry for a compiled method.
-  // Note: Allowing nested faults if `IsValidMethod()` returned a false positive.
-  // Note: The `ArtMethod::GetOatQuickMethodHeader()` can acquire locks (at least
-  // `Locks::jit_lock_`) and if the thread already held such a lock, the signal
-  // handler would deadlock. However, if a thread is holding one of the locks
-  // below the mutator lock, the PC should be somewhere in ART code and should
-  // not match any registered generated code range, so such as a deadlock is
-  // unlikely. If it happens anyway, the worst case is that an internal ART crash
-  // would be reported as ANR.
-  ArtMethod* method = *sp;
-  const OatQuickMethodHeader* method_header = method->GetOatQuickMethodHeader(return_pc);
   if (method_header == nullptr) {
-    VLOG(signals) << "No method header.";
+    VLOG(signals) << "no compiled code";
     return false;
   }
-  VLOG(signals) << "looking for dex pc for return pc 0x" << std::hex << return_pc
-                << " pc offset: 0x" << std::hex
-                << (return_pc - reinterpret_cast<uintptr_t>(method_header->GetEntryPoint()));
-  uint32_t dexpc = method_header->ToDexPc(reinterpret_cast<ArtMethod**>(sp), return_pc, false);
+
+  // We can be certain that this is a method now.  Check if we have a GC map
+  // at the return PC address.
+  if (true || kIsDebugBuild) {
+    VLOG(signals) << "looking for dex pc for return pc " << std::hex << return_pc;
+    uint32_t sought_offset = return_pc -
+        reinterpret_cast<uintptr_t>(method_header->GetEntryPoint());
+    VLOG(signals) << "pc offset: " << std::hex << sought_offset;
+  }
+  uint32_t dexpc = dex::kDexNoIndex;
+  if (is_stack_overflow) {
+    // If it's an implicit stack overflow check, the frame is not setup, so we
+    // just infer the dex PC as zero.
+    dexpc = 0;
+  } else {
+    CHECK_EQ(*reinterpret_cast<ArtMethod**>(sp), method_obj);
+    dexpc = method_header->ToDexPc(reinterpret_cast<ArtMethod**>(sp), return_pc, false);
+  }
   VLOG(signals) << "dexpc: " << dexpc;
-  return dexpc != dex::kDexNoIndex;
+  return !check_dex_pc || dexpc != dex::kDexNoIndex;
+}
+
+FaultHandler::FaultHandler(FaultManager* manager) : manager_(manager) {
+}
+
+//
+// Null pointer fault handler
+//
+NullPointerHandler::NullPointerHandler(FaultManager* manager) : FaultHandler(manager) {
+  manager_->AddHandler(this, true);
 }
 
 //
@@ -454,13 +426,17 @@ JavaStackTraceHandler::JavaStackTraceHandler(FaultManager* manager) : FaultHandl
 
 bool JavaStackTraceHandler::Action(int sig ATTRIBUTE_UNUSED, siginfo_t* siginfo, void* context) {
   // Make sure that we are in the generated code, but we may not have a dex pc.
-  bool in_generated_code = manager_->IsInGeneratedCode(siginfo, context);
+  bool in_generated_code = manager_->IsInGeneratedCode(siginfo, context, false);
   if (in_generated_code) {
     LOG(ERROR) << "Dumping java stack trace for crash in generated code";
+    ArtMethod* method = nullptr;
+    uintptr_t return_pc = 0;
+    uintptr_t sp = 0;
+    bool is_stack_overflow = false;
     Thread* self = Thread::Current();
 
-    uintptr_t sp = FaultManager::GetFaultSp(context);
-    CHECK_NE(sp, 0u);  // Otherwise we should not have reached this handler.
+    manager_->GetMethodAndReturnPcAndSp(
+        siginfo, context, &method, &return_pc, &sp, &is_stack_overflow);
     // Inside of generated code, sp[0] is the method, so sp is the frame.
     self->SetTopOfStack(reinterpret_cast<ArtMethod**>(sp));
     self->DumpJavaStack(LOG_STREAM(ERROR));
diff --git a/runtime/fault_handler.h b/runtime/fault_handler.h
index b11b9d9ece..8b89c22a0f 100644
--- a/runtime/fault_handler.h
+++ b/runtime/fault_handler.h
@@ -21,11 +21,9 @@
 #include <signal.h>
 #include <stdint.h>
 
-#include <atomic>
 #include <vector>
 
 #include "base/locks.h"  // For annotalysis.
-#include "base/mutex.h"
 #include "runtime_globals.h"  // For CanDoImplicitNullCheckOn.
 
 namespace art {
@@ -53,33 +51,25 @@ class FaultManager {
   void AddHandler(FaultHandler* handler, bool generated_code);
   void RemoveHandler(FaultHandler* handler);
 
-  void AddGeneratedCodeRange(const void* start, size_t size);
-  void RemoveGeneratedCodeRange(const void* start, size_t size) REQUIRES(!Locks::mutator_lock_);
-
-  // Retrieves fault PC from architecture-dependent `context`, returns 0 on failure.
-  // Called in the context of a signal handler.
-  static uintptr_t GetFaultPc(siginfo_t* siginfo, void* context);
-
-  // Retrieves SP from architecture-dependent `context`.
-  // Called in the context of a signal handler.
-  static uintptr_t GetFaultSp(void* context);
-
-  // Checks if the fault happened while running generated code.
-  // Called in the context of a signal handler.
-  bool IsInGeneratedCode(siginfo_t* siginfo, void *context) NO_THREAD_SAFETY_ANALYSIS;
+  // Note that the following two functions are called in the context of a signal handler.
+  // The IsInGeneratedCode() function checks that the mutator lock is held before it
+  // calls GetMethodAndReturnPCAndSP().
+  // TODO: think about adding lock assertions and fake lock and unlock functions.
+  void GetMethodAndReturnPcAndSp(siginfo_t* siginfo,
+                                 void* context,
+                                 ArtMethod** out_method,
+                                 uintptr_t* out_return_pc,
+                                 uintptr_t* out_sp,
+                                 bool* out_is_stack_overflow)
+                                 NO_THREAD_SAFETY_ANALYSIS;
+  bool IsInGeneratedCode(siginfo_t* siginfo, void *context, bool check_dex_pc)
+                         NO_THREAD_SAFETY_ANALYSIS;
 
  private:
-  struct GeneratedCodeRange;
-
   // The HandleFaultByOtherHandlers function is only called by HandleFault function for generated code.
   bool HandleFaultByOtherHandlers(int sig, siginfo_t* info, void* context)
                                   NO_THREAD_SAFETY_ANALYSIS;
 
-  // Note: The lock guards modifications of the ranges but the function `IsInGeneratedCode()`
-  // walks the list in the context of a signal handler without holding the lock.
-  Mutex generated_code_ranges_lock_;
-  std::atomic<GeneratedCodeRange*> generated_code_ranges_ GUARDED_BY(generated_code_ranges_lock_);
-
   std::vector<FaultHandler*> generated_code_handlers_;
   std::vector<FaultHandler*> other_handlers_;
   struct sigaction oldaction_;
@@ -108,29 +98,17 @@ class NullPointerHandler final : public FaultHandler {
  public:
   explicit NullPointerHandler(FaultManager* manager);
 
-  // NO_THREAD_SAFETY_ANALYSIS: Called after the fault manager determined that
-  // the thread is `Runnable` and holds the mutator lock (shared) but without
-  // telling annotalysis that we actually hold the lock.
-  bool Action(int sig, siginfo_t* siginfo, void* context) override
-      NO_THREAD_SAFETY_ANALYSIS;
-
- private:
-  // Helper functions for checking whether the signal can be interpreted
-  // as implicit NPE check. Note that the runtime will do more exhaustive
-  // checks (that we cannot reasonably do in signal processing code) based
-  // on the dex instruction faulting.
+  bool Action(int sig, siginfo_t* siginfo, void* context) override;
 
-  static bool IsValidFaultAddress(uintptr_t fault_address) {
+  static bool IsValidImplicitCheck(siginfo_t* siginfo) {
     // Our implicit NPE checks always limit the range to a page.
-    return CanDoImplicitNullCheckOn(fault_address);
+    // Note that the runtime will do more exhaustive checks (that we cannot
+    // reasonably do in signal processing code) based on the dex instruction
+    // faulting.
+    return CanDoImplicitNullCheckOn(reinterpret_cast<uintptr_t>(siginfo->si_addr));
   }
 
-  static bool IsValidMethod(ArtMethod* method)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
-  static bool IsValidReturnPc(ArtMethod** sp, uintptr_t return_pc)
-      REQUIRES_SHARED(Locks::mutator_lock_);
-
+ private:
   DISALLOW_COPY_AND_ASSIGN(NullPointerHandler);
 };
 
diff --git a/runtime/jit/jit_code_cache.cc b/runtime/jit/jit_code_cache.cc
index f4b26284e1..ee5d5c7ce3 100644
--- a/runtime/jit/jit_code_cache.cc
+++ b/runtime/jit/jit_code_cache.cc
@@ -220,10 +220,9 @@ JitCodeCache* JitCodeCache::Create(bool used_only_for_profile_data,
     }
   }
 
-  Runtime* runtime = Runtime::Current();
-  size_t initial_capacity = runtime->GetJITOptions()->GetCodeCacheInitialCapacity();
+  size_t initial_capacity = Runtime::Current()->GetJITOptions()->GetCodeCacheInitialCapacity();
   // Check whether the provided max capacity in options is below 1GB.
-  size_t max_capacity = runtime->GetJITOptions()->GetCodeCacheMaxCapacity();
+  size_t max_capacity = Runtime::Current()->GetJITOptions()->GetCodeCacheMaxCapacity();
   // We need to have 32 bit offsets from method headers in code cache which point to things
   // in the data cache. If the maps are more than 4G apart, having multiple maps wouldn't work.
   // Ensure we're below 1 GB to be safe.
@@ -245,11 +244,6 @@ JitCodeCache* JitCodeCache::Create(bool used_only_for_profile_data,
     return nullptr;
   }
 
-  if (region.HasCodeMapping()) {
-    const MemMap* exec_pages = region.GetExecPages();
-    runtime->AddGeneratedCodeRange(exec_pages->Begin(), exec_pages->Size());
-  }
-
   std::unique_ptr<JitCodeCache> jit_code_cache(new JitCodeCache());
   if (is_zygote) {
     // Zygote should never collect code to share the memory with the children.
@@ -284,16 +278,7 @@ JitCodeCache::JitCodeCache()
       histogram_profiling_info_memory_use_("Memory used for profiling info", 16) {
 }
 
-JitCodeCache::~JitCodeCache() {
-  if (private_region_.HasCodeMapping()) {
-    const MemMap* exec_pages = private_region_.GetExecPages();
-    Runtime::Current()->RemoveGeneratedCodeRange(exec_pages->Begin(), exec_pages->Size());
-  }
-  if (shared_region_.HasCodeMapping()) {
-    const MemMap* exec_pages = shared_region_.GetExecPages();
-    Runtime::Current()->RemoveGeneratedCodeRange(exec_pages->Begin(), exec_pages->Size());
-  }
-}
+JitCodeCache::~JitCodeCache() {}
 
 bool JitCodeCache::PrivateRegionContainsPc(const void* ptr) const {
   return private_region_.IsInExecSpace(ptr);
@@ -1859,8 +1844,7 @@ void JitCodeCache::PostForkChildAction(bool is_system_server, bool is_zygote) {
   // We do this now and not in Jit::PostForkChildAction, as system server calls
   // JitCodeCache::PostForkChildAction first, and then does some code loading
   // that may result in new JIT tasks that we want to keep.
-  Runtime* runtime = Runtime::Current();
-  ThreadPool* pool = runtime->GetJit()->GetThreadPool();
+  ThreadPool* pool = Runtime::Current()->GetJit()->GetThreadPool();
   if (pool != nullptr) {
     pool->RemoveAllTasks(self);
   }
@@ -1871,7 +1855,7 @@ void JitCodeCache::PostForkChildAction(bool is_system_server, bool is_zygote) {
   // to write to them.
   shared_region_.ResetWritableMappings();
 
-  if (is_zygote || runtime->IsSafeMode()) {
+  if (is_zygote || Runtime::Current()->IsSafeMode()) {
     // Don't create a private region for a child zygote. Regions are usually map shared
     // (to satisfy dual-view), and we don't want children of a child zygote to inherit it.
     return;
@@ -1886,8 +1870,8 @@ void JitCodeCache::PostForkChildAction(bool is_system_server, bool is_zygote) {
   histogram_code_memory_use_.Reset();
   histogram_profiling_info_memory_use_.Reset();
 
-  size_t initial_capacity = runtime->GetJITOptions()->GetCodeCacheInitialCapacity();
-  size_t max_capacity = runtime->GetJITOptions()->GetCodeCacheMaxCapacity();
+  size_t initial_capacity = Runtime::Current()->GetJITOptions()->GetCodeCacheInitialCapacity();
+  size_t max_capacity = Runtime::Current()->GetJITOptions()->GetCodeCacheMaxCapacity();
   std::string error_msg;
   if (!private_region_.Initialize(initial_capacity,
                                   max_capacity,
@@ -1896,10 +1880,6 @@ void JitCodeCache::PostForkChildAction(bool is_system_server, bool is_zygote) {
                                   &error_msg)) {
     LOG(WARNING) << "Could not create private region after zygote fork: " << error_msg;
   }
-  if (private_region_.HasCodeMapping()) {
-    const MemMap* exec_pages = private_region_.GetExecPages();
-    runtime->AddGeneratedCodeRange(exec_pages->Begin(), exec_pages->Size());
-  }
 }
 
 JitMemoryRegion* JitCodeCache::GetCurrentRegion() {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index d81b8fdc17..66a6d32ab1 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -842,18 +842,6 @@ static bool IsSafeToCallAbort() NO_THREAD_SAFETY_ANALYSIS {
   return runtime != nullptr && runtime->IsStarted() && !runtime->IsShuttingDownLocked();
 }
 
-void Runtime::AddGeneratedCodeRange(const void* start, size_t size) {
-  if (HandlesSignalsInCompiledCode()) {
-    fault_manager.AddGeneratedCodeRange(start, size);
-  }
-}
-
-void Runtime::RemoveGeneratedCodeRange(const void* start, size_t size) {
-  if (HandlesSignalsInCompiledCode()) {
-    fault_manager.RemoveGeneratedCodeRange(start, size);
-  }
-}
-
 bool Runtime::Create(RuntimeArgumentMap&& runtime_options) {
   // TODO: acquire a static mutex on Runtime to avoid racing.
   if (Runtime::instance_ != nullptr) {
@@ -998,6 +986,7 @@ bool Runtime::Start() {
     if (!jit::Jit::LoadCompilerLibrary(&error_msg)) {
       LOG(WARNING) << "Failed to load JIT compiler with error " << error_msg;
     }
+    CreateJitCodeCache(/*rwx_memory_allowed=*/true);
     CreateJit();
 #ifdef ADDRESS_SANITIZER
     // (b/238730394): In older implementations of sanitizer + glibc there is a race between
@@ -1585,8 +1574,6 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
         << (core_platform_api_policy_ == hiddenapi::EnforcementPolicy::kEnabled ? "true" : "false");
   }
 
-  // Dex2Oat's Runtime does not need the signal chain or the fault handler
-  // and it passes the `NoSigChain` option to `Runtime` to indicate this.
   no_sig_chain_ = runtime_options.Exists(Opt::NoSigChain);
   force_native_bridge_ = runtime_options.Exists(Opt::ForceNativeBridge);
 
@@ -1778,34 +1765,31 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) {
       break;
   }
 
-  if (HandlesSignalsInCompiledCode()) {
-    fault_manager.Init();
-
-    // These need to be in a specific order.  The null point check handler must be
-    // after the suspend check and stack overflow check handlers.
-    //
-    // Note: the instances attach themselves to the fault manager and are handled by it. The
-    //       manager will delete the instance on Shutdown().
-    if (implicit_suspend_checks_) {
-      new SuspensionHandler(&fault_manager);
-    }
+  if (!no_sig_chain_) {
+    // Dex2Oat's Runtime does not need the signal chain or the fault handler.
+    if (implicit_null_checks_ || implicit_so_checks_ || implicit_suspend_checks_) {
+      fault_manager.Init();
 
-    if (implicit_so_checks_) {
-      new StackOverflowHandler(&fault_manager);
-    }
+      // These need to be in a specific order.  The null point check handler must be
+      // after the suspend check and stack overflow check handlers.
+      //
+      // Note: the instances attach themselves to the fault manager and are handled by it. The
+      //       manager will delete the instance on Shutdown().
+      if (implicit_suspend_checks_) {
+        new SuspensionHandler(&fault_manager);
+      }
 
-    if (implicit_null_checks_) {
-      new NullPointerHandler(&fault_manager);
-    }
+      if (implicit_so_checks_) {
+        new StackOverflowHandler(&fault_manager);
+      }
 
-    if (kEnableJavaStackTraceHandler) {
-      new JavaStackTraceHandler(&fault_manager);
-    }
+      if (implicit_null_checks_) {
+        new NullPointerHandler(&fault_manager);
+      }
 
-    if (interpreter::CanRuntimeUseNterp()) {
-      // Nterp code can use signal handling just like the compiled managed code.
-      OatQuickMethodHeader* nterp_header = OatQuickMethodHeader::NterpMethodHeader;
-      fault_manager.AddGeneratedCodeRange(nterp_header->GetCode(), nterp_header->GetCodeSize());
+      if (kEnableJavaStackTraceHandler) {
+        new JavaStackTraceHandler(&fault_manager);
+      }
     }
   }
 
@@ -3028,9 +3012,7 @@ void Runtime::AddCurrentRuntimeFeaturesAsDex2OatArguments(std::vector<std::strin
   }
 }
 
-void Runtime::CreateJit() {
-  DCHECK(jit_code_cache_ == nullptr);
-  DCHECK(jit_ == nullptr);
+void Runtime::CreateJitCodeCache(bool rwx_memory_allowed) {
   if (kIsDebugBuild && GetInstrumentation()->IsForcedInterpretOnly()) {
     DCHECK(!jit_options_->UseJitCompilation());
   }
@@ -3039,19 +3021,28 @@ void Runtime::CreateJit() {
     return;
   }
 
-  if (IsSafeMode()) {
-    LOG(INFO) << "Not creating JIT because of SafeMode.";
-    return;
-  }
-
   std::string error_msg;
   bool profiling_only = !jit_options_->UseJitCompilation();
   jit_code_cache_.reset(jit::JitCodeCache::Create(profiling_only,
-                                                  /*rwx_memory_allowed=*/ true,
+                                                  rwx_memory_allowed,
                                                   IsZygote(),
                                                   &error_msg));
   if (jit_code_cache_.get() == nullptr) {
     LOG(WARNING) << "Failed to create JIT Code Cache: " << error_msg;
+  }
+}
+
+void Runtime::CreateJit() {
+  DCHECK(jit_ == nullptr);
+  if (jit_code_cache_.get() == nullptr) {
+    if (!IsSafeMode()) {
+      LOG(WARNING) << "Missing code cache, cannot create JIT.";
+    }
+    return;
+  }
+  if (IsSafeMode()) {
+    LOG(INFO) << "Not creating JIT because of SafeMode.";
+    jit_code_cache_.reset();
     return;
   }
 
diff --git a/runtime/runtime.h b/runtime/runtime.h
index cf2b437f97..0cebdab5dc 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -1147,9 +1147,6 @@ class Runtime {
     return no_sig_chain_;
   }
 
-  void AddGeneratedCodeRange(const void* start, size_t size);
-  void RemoveGeneratedCodeRange(const void* start, size_t size);
-
   // Trigger a flag reload from system properties or device congfigs.
   //
   // Should only be called from runtime init and zygote post fork as
@@ -1172,11 +1169,6 @@ class Runtime {
 
   Runtime();
 
-  bool HandlesSignalsInCompiledCode() const {
-    return !no_sig_chain_ &&
-           (implicit_null_checks_ || implicit_so_checks_ || implicit_suspend_checks_);
-  }
-
   void BlockSignals();
 
   bool Init(RuntimeArgumentMap&& runtime_options)