[arm64] Fix CFI issue with short builtin calls

The allowlist used for `Deoptimizer::IsValidReturnAddress` depends on
fixed embedded builtin addresses. Pass a pointer to the isolate to
this method, so that it can discover the actual builtin code start
(which may have been remapped) and calculate the offset from the start
of the builtins' code in order to check if the return address is
allowed.

After this change, do not disable short builtin calls when CFI is
enabled.

There's an important TODO for this change:
Since the builtin code pointer that's used to check whether a return
address is allowed is now writable, we should use pointer authentication
to protect it.

Bug: v8:10026
Change-Id: Iafd31d3ad7e10cb17faf33e76e78d3df36edeefd
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3667506Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Georgia Kouveli <georgia.kouveli@arm.com>
Cr-Commit-Position: refs/heads/main@{#81049}

BUILD.gn

@@ -485,10 +485,8 @@ if (build_with_chromium && v8_current_cpu == "arm64" &&
 }
 
 if (v8_enable_short_builtin_calls &&
-    ((!v8_enable_pointer_compression && v8_current_cpu != "x64") ||
-     v8_control_flow_integrity)) {
-  # Disable short calls when pointer compression is not enabled.
-  # Or when CFI is enabled (until the CFI-related issues are fixed), except x64,
+    (!v8_enable_pointer_compression && v8_current_cpu != "x64")) {
+  # Disable short calls when pointer compression is not enabled, except x64,
   # where short builtin calls can still be enabled if the code range is
   # guaranteed to be close enough to embedded builtins.
   v8_enable_short_builtin_calls = false

src/deoptimizer/arm64/deoptimizer-arm64.cc

@@ -24,7 +24,7 @@ Float32 RegisterValues::GetFloatRegister(unsigned n) const {
 void FrameDescription::SetCallerPc(unsigned offset, intptr_t value) {
   Address new_context =
       static_cast<Address>(GetTop()) + offset + kPCOnStackSize;
-  value = PointerAuthentication::SignAndCheckPC(value, new_context);
+  value = PointerAuthentication::SignAndCheckPC(isolate_, value, new_context);
   SetFrameSlot(offset, value);
 }
 
@@ -38,9 +38,11 @@ void FrameDescription::SetCallerConstantPool(unsigned offset, intptr_t value) {
 }
 
 void FrameDescription::SetPc(intptr_t pc) {
+  // TODO(v8:10026): We need to sign pointers to the embedded blob, which are
+  // stored in the isolate and code range objects.
   if (ENABLE_CONTROL_FLOW_INTEGRITY_BOOL) {
-    CHECK(
-        Deoptimizer::IsValidReturnAddress(PointerAuthentication::StripPAC(pc)));
+    CHECK(Deoptimizer::IsValidReturnAddress(PointerAuthentication::StripPAC(pc),
+                                            isolate_));
   }
   pc_ = pc;
 }

src/deoptimizer/deoptimizer-cfi-builtins.cc

@@ -4,6 +4,7 @@
 
 #include "src/builtins/builtins.h"
 #include "src/deoptimizer/deoptimizer.h"
+#include "src/snapshot/embedded/embedded-data.h"
 
 extern "C" {
 void Builtins_InterpreterEnterAtBytecode();
@@ -23,6 +24,9 @@ typedef void (*function_ptr)();
 namespace v8 {
 namespace internal {
 
+extern "C" const uint8_t v8_Default_embedded_blob_code_[];
+extern "C" uint32_t v8_Default_embedded_blob_code_size_;
+
 // List of allowed builtin addresses that we can return to in the deoptimizer.
 constexpr function_ptr builtins[] = {
     &Builtins_InterpreterEnterAtBytecode,
@@ -38,9 +42,16 @@ constexpr function_ptr builtins[] = {
     &Builtins_RestartFrameTrampoline,
 };
 
-bool Deoptimizer::IsValidReturnAddress(Address address) {
+bool Deoptimizer::IsValidReturnAddress(Address address, Isolate* isolate) {
+  EmbeddedData d = EmbeddedData::GetEmbeddedDataForPC(isolate, address);
+  Address code_start = reinterpret_cast<Address>(d.code());
+  Address offset = address - code_start;
+  if (offset >= v8_Default_embedded_blob_code_size_) return false;
+  Address blob_start =
+      reinterpret_cast<Address>(v8_Default_embedded_blob_code_);
+  Address original_address = blob_start + offset;
   for (function_ptr builtin : builtins) {
-    if (address == FUNCTION_ADDR(builtin)) {
+    if (original_address == FUNCTION_ADDR(builtin)) {
       return true;
     }
   }

src/deoptimizer/deoptimizer-cfi-empty.cc

@@ -8,7 +8,9 @@ namespace v8 {
 namespace internal {
 
 // Dummy implementation when building mksnapshot.
-bool Deoptimizer::IsValidReturnAddress(Address address) { return false; }
+bool Deoptimizer::IsValidReturnAddress(Address address, Isolate* isolate) {
+  return false;
+}
 
 }  // namespace internal
 }  // namespace v8

src/deoptimizer/deoptimizer.cc

@@ -513,7 +513,7 @@ Deoptimizer::Deoptimizer(Isolate* isolate, JSFunction function,
   unsigned size = ComputeInputFrameSize();
   const int parameter_count =
       function.shared().internal_formal_parameter_count_with_receiver();
-  input_ = new (size) FrameDescription(size, parameter_count);
+  input_ = new (size) FrameDescription(size, parameter_count, isolate_);
 
   DCHECK_EQ(deopt_exit_index_, kFixedExitSizeMarker);
   // Calculate the deopt exit index from return address.
@@ -984,7 +984,7 @@ void Deoptimizer::DoComputeUnoptimizedFrame(TranslatedFrame* translated_frame,
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame = new (output_frame_size)
-      FrameDescription(output_frame_size, parameters_count);
+      FrameDescription(output_frame_size, parameters_count, isolate());
   FrameWriter frame_writer(this, output_frame, verbose_trace_scope());
 
   CHECK(frame_index >= 0 && frame_index < output_count_);
@@ -1220,7 +1220,7 @@ void Deoptimizer::DoComputeUnoptimizedFrame(TranslatedFrame* translated_frame,
     // Only the pc of the topmost frame needs to be signed since it is
     // authenticated at the end of the DeoptimizationEntry builtin.
     const intptr_t top_most_pc = PointerAuthentication::SignAndCheckPC(
-        pc, frame_writer.frame()->GetTop());
+        isolate(), pc, frame_writer.frame()->GetTop());
     output_frame->SetPc(top_most_pc);
   } else {
     output_frame->SetPc(pc);
@@ -1285,7 +1285,8 @@ void Deoptimizer::DoComputeInlinedExtraArguments(
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame = new (output_frame_size) FrameDescription(
-      output_frame_size, JSParameterCount(argument_count_without_receiver));
+      output_frame_size, JSParameterCount(argument_count_without_receiver),
+      isolate());
   // The top address of the frame is computed from the previous frame's top and
   // this frame's size.
   const intptr_t top_address =
@@ -1347,7 +1348,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
 
   // Allocate and store the output frame description.
   FrameDescription* output_frame = new (output_frame_size)
-      FrameDescription(output_frame_size, parameters_count);
+      FrameDescription(output_frame_size, parameters_count, isolate());
   FrameWriter frame_writer(this, output_frame, verbose_trace_scope());
 
   // Construct stub can not be topmost.
@@ -1451,7 +1452,7 @@ void Deoptimizer::DoComputeConstructStubFrame(TranslatedFrame* translated_frame,
     // Only the pc of the topmost frame needs to be signed since it is
     // authenticated at the end of the DeoptimizationEntry builtin.
     output_frame->SetPc(PointerAuthentication::SignAndCheckPC(
-        pc_value, frame_writer.frame()->GetTop()));
+        isolate(), pc_value, frame_writer.frame()->GetTop()));
   } else {
     output_frame->SetPc(pc_value);
   }
@@ -1695,8 +1696,8 @@ void Deoptimizer::DoComputeBuiltinContinuation(
            frame_info.stack_parameter_count(), output_frame_size);
   }
 
-  FrameDescription* output_frame = new (output_frame_size)
-      FrameDescription(output_frame_size, frame_info.stack_parameter_count());
+  FrameDescription* output_frame = new (output_frame_size) FrameDescription(
+      output_frame_size, frame_info.stack_parameter_count(), isolate());
   output_[frame_index] = output_frame;
   FrameWriter frame_writer(this, output_frame, verbose_trace_scope());
 
@@ -1912,6 +1913,7 @@ void Deoptimizer::DoComputeBuiltinContinuation(
     // Only the pc of the topmost frame needs to be signed since it is
     // authenticated at the end of the DeoptimizationEntry builtin.
     const intptr_t top_most_pc = PointerAuthentication::SignAndCheckPC(
+        isolate(),
         static_cast<intptr_t>(continue_to_builtin.InstructionStart()),
         frame_writer.frame()->GetTop());
     output_frame->SetPc(top_most_pc);

src/deoptimizer/deoptimizer.h

@@ -91,7 +91,7 @@ class Deoptimizer : public Malloced {
   // deoptimizer, in particular the signing process, to gain control over the
   // program.
   // When building mksnapshot, always return false.
-  static bool IsValidReturnAddress(Address address);
+  static bool IsValidReturnAddress(Address pc, Isolate* isolate);
 
   ~Deoptimizer();
 

src/deoptimizer/frame-description.h

@@ -57,14 +57,16 @@ class RegisterValues {
 
 class FrameDescription {
  public:
-  FrameDescription(uint32_t frame_size, int parameter_count)
+  FrameDescription(uint32_t frame_size, int parameter_count, Isolate* isolate)
       : frame_size_(frame_size),
         parameter_count_(parameter_count),
         top_(kZapUint32),
         pc_(kZapUint32),
         fp_(kZapUint32),
         context_(kZapUint32),
-        constant_pool_(kZapUint32) {
+        constant_pool_(kZapUint32),
+        isolate_(isolate) {
+    USE(isolate_);
     // Zap all the registers.
     for (int r = 0; r < Register::kNumRegisters; r++) {
       // TODO(jbramley): It isn't safe to use kZapUint32 here. If the register
@@ -211,6 +213,8 @@ class FrameDescription {
   intptr_t context_;
   intptr_t constant_pool_;
 
+  Isolate* isolate_;
+
   // Continuation is the PC where the execution continues after
   // deoptimizing.
   intptr_t continuation_;

src/execution/arm64/pointer-authentication-arm64.h

@@ -16,9 +16,6 @@ namespace internal {
 // The following functions execute on the host and therefore need a different
 // path based on whether we are simulating arm64 or not.
 
-// clang-format fails to detect this file as C++, turn it off.
-// clang-format off
-
 // Authenticate the address stored in {pc_address}. {offset_from_sp} is the
 // offset between {pc_address} and the pointer used as a context for signing.
 V8_INLINE Address PointerAuthentication::AuthenticatePC(
@@ -99,29 +96,27 @@ V8_INLINE void PointerAuthentication::ReplacePC(Address* pc_address,
 
 
 // Sign {pc} using {sp}.
-V8_INLINE Address PointerAuthentication::SignAndCheckPC(Address pc,
-                                                          Address sp) {
+V8_INLINE Address PointerAuthentication::SignAndCheckPC(Isolate* isolate,
+                                                        Address pc,
+                                                        Address sp) {
 #ifdef USE_SIMULATOR
   pc = Simulator::AddPAC(pc, sp, Simulator::kPACKeyIB,
-                        Simulator::kInstructionPointer);
-  CHECK(Deoptimizer::IsValidReturnAddress(PointerAuthentication::StripPAC(pc)));
-  return pc;
+                         Simulator::kInstructionPointer);
 #else
   asm volatile(
-    "  mov x17, %[pc]\n"
-    "  mov x16, %[sp]\n"
-    "  pacib1716\n"
-    "  mov %[pc], x17\n"
-    : [pc] "+r"(pc)
-    : [sp] "r"(sp)
-    : "x16", "x17");
-  CHECK(Deoptimizer::IsValidReturnAddress(PointerAuthentication::StripPAC(pc)));
-  return pc;
+      "  mov x17, %[pc]\n"
+      "  mov x16, %[sp]\n"
+      "  pacib1716\n"
+      "  mov %[pc], x17\n"
+      : [pc] "+r"(pc)
+      : [sp] "r"(sp)
+      : "x16", "x17");
 #endif
+  CHECK(Deoptimizer::IsValidReturnAddress(PointerAuthentication::StripPAC(pc),
+                                          isolate));
+  return pc;
 }
 
-// clang-format on
-
 }  // namespace internal
 }  // namespace v8
 #endif  // V8_EXECUTION_ARM64_POINTER_AUTHENTICATION_ARM64_H_

src/execution/pointer-authentication-dummy.h

@@ -16,9 +16,8 @@ namespace internal {
 // when CFI is not enabled.
 
 // Load return address from {pc_address} and return it.
-V8_INLINE Address PointerAuthentication::AuthenticatePC(
-    Address* pc_address, unsigned offset_from_sp) {
-  USE(offset_from_sp);
+V8_INLINE Address PointerAuthentication::AuthenticatePC(Address* pc_address,
+                                                        unsigned) {
   return *pc_address;
 }
 
@@ -27,16 +26,13 @@ V8_INLINE Address PointerAuthentication::StripPAC(Address pc) { return pc; }
 
 // Store {new_pc} to {pc_address} without signing.
 V8_INLINE void PointerAuthentication::ReplacePC(Address* pc_address,
-                                                Address new_pc,
-                                                int offset_from_sp) {
-  USE(offset_from_sp);
+                                                Address new_pc, int) {
   *pc_address = new_pc;
 }
 
 // Return {pc} unmodified.
-V8_INLINE Address PointerAuthentication::SignAndCheckPC(Address pc,
-                                                        Address sp) {
-  USE(sp);
+V8_INLINE Address PointerAuthentication::SignAndCheckPC(Isolate*, Address pc,
+                                                        Address) {
   return pc;
 }
 

src/execution/pointer-authentication.h

@@ -37,7 +37,8 @@ class PointerAuthentication : public AllStatic {
   // When CFI is enabled, sign {pc} using {sp}, check the address and return the
   // signed value. When CFI is not enabled, return {pc} unmodified. This method
   // only applies in the deoptimizer.
-  V8_INLINE static Address SignAndCheckPC(Address pc, Address sp);
+  V8_INLINE static Address SignAndCheckPC(Isolate* isolate, Address pc,
+                                          Address sp);
 };
 
 }  // namespace internal