Reland of Make profiler no frame region detection code more robust [ia86/x64]

Upon collection of the stack trace if the current PC falls into
the frame building code, the top frame might be in a non-consistent
state. That leads to some of the frames could be missing from the
stack trace.

The patch makes it check instructions under current PC and if they
look like the frame setup/destroy code, it skips the entire sample.

Support for x86/x64

CG_INCLUDE_TRYBOTS=tryserver.v8:v8_linux64_msan_rel
BUG=chromium:529931
LOG=N

Review URL: https://codereview.chromium.org/1348533005

Cr-Commit-Position: refs/heads/master@{#30841}

diff --git a/src/sampler.cc b/src/sampler.cc
index 4e669ca6dc13611fdda5bacc728d2f1227592646..88e429b9fba56e97bcaa6f74175e4359880cfff4 100644 (file)
--- a/src/sampler.cc
+++ b/src/sampler.cc
@@ -173,6 +173,69 @@ class PlatformDataCommon : public Malloced {
   ThreadId profiled_thread_id_;
 };
 
+
+bool IsSamePage(byte* ptr1, byte* ptr2) {
+  const uint32_t kPageSize = 4096;
+  uintptr_t mask = ~static_cast<uintptr_t>(kPageSize - 1);
+  return (reinterpret_cast<uintptr_t>(ptr1) & mask) ==
+         (reinterpret_cast<uintptr_t>(ptr2) & mask);
+}
+
+
+// Check if the code at specified address could potentially be a
+// frame setup code.
+bool IsNoFrameRegion(Address address) {
+  struct Pattern {
+    int bytes_count;
+    byte bytes[8];
+    int offsets[4];
+  };
+  byte* pc = reinterpret_cast<byte*>(address);
+  static Pattern patterns[] = {
+#if V8_HOST_ARCH_IA32
+    // push %ebp
+    // mov %esp,%ebp
+    {3, {0x55, 0x89, 0xe5}, {0, 1, -1}},
+    // pop %ebp
+    // ret N
+    {2, {0x5d, 0xc2}, {0, 1, -1}},
+    // pop %ebp
+    // ret
+    {2, {0x5d, 0xc3}, {0, 1, -1}},
+#elif V8_HOST_ARCH_X64
+    // pushq %rbp
+    // movq %rsp,%rbp
+    {4, {0x55, 0x48, 0x89, 0xe5}, {0, 1, -1}},
+    // popq %rbp
+    // ret N
+    {2, {0x5d, 0xc2}, {0, 1, -1}},
+    // popq %rbp
+    // ret
+    {2, {0x5d, 0xc3}, {0, 1, -1}},
+#endif
+    {0, {}, {}}
+  };
+  for (Pattern* pattern = patterns; pattern->bytes_count; ++pattern) {
+    for (int* offset_ptr = pattern->offsets; *offset_ptr != -1; ++offset_ptr) {
+      int offset = *offset_ptr;
+      if (!offset || IsSamePage(pc, pc - offset)) {
+        MSAN_MEMORY_IS_INITIALIZED(pc - offset, pattern->bytes_count);
+        if (!memcmp(pc - offset, pattern->bytes, pattern->bytes_count))
+          return true;
+      } else {
+        // It is not safe to examine bytes on another page as it might not be
+        // allocated thus causing a SEGFAULT.
+        // Check the pattern part that's on the same page and
+        // pessimistically assume it could be the entire pattern match.
+        MSAN_MEMORY_IS_INITIALIZED(pc, pattern->bytes_count - offset);
+        if (!memcmp(pc, pattern->bytes + offset, pattern->bytes_count - offset))
+          return true;
+      }
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 #if defined(USE_SIGNALS)
@@ -232,7 +295,9 @@ class SimulatorHelper {
 
   inline void FillRegisters(v8::RegisterState* state) {
 #if V8_TARGET_ARCH_ARM
-    state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    if (!simulator_->has_bad_pc()) {
+      state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    }
     state->sp = reinterpret_cast<Address>(simulator_->get_register(
         Simulator::sp));
     state->fp = reinterpret_cast<Address>(simulator_->get_register(
@@ -243,19 +308,30 @@ class SimulatorHelper {
       // the sp or fp register. ARM64 simulator does this in two steps:
       // first setting it to zero and then setting it to a new value.
       // Bailout if sp/fp doesn't contain the new value.
+      //
+      // FIXME: The above doesn't really solve the issue.
+      // If a 64-bit target is executed on a 32-bit host even the final
+      // write is non-atomic, so it might obtain a half of the result.
+      // Moreover as long as the register set code uses memcpy (as of now),
+      // it is not guaranteed to be atomic even when both host and target
+      // are of same bitness.
       return;
     }
     state->pc = reinterpret_cast<Address>(simulator_->pc());
     state->sp = reinterpret_cast<Address>(simulator_->sp());
     state->fp = reinterpret_cast<Address>(simulator_->fp());
 #elif V8_TARGET_ARCH_MIPS || V8_TARGET_ARCH_MIPS64
-    state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    if (!simulator_->has_bad_pc()) {
+      state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    }
     state->sp = reinterpret_cast<Address>(simulator_->get_register(
         Simulator::sp));
     state->fp = reinterpret_cast<Address>(simulator_->get_register(
         Simulator::fp));
 #elif V8_TARGET_ARCH_PPC
-    state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    if (!simulator_->has_bad_pc()) {
+      state->pc = reinterpret_cast<Address>(simulator_->get_pc());
+    }
     state->sp =
         reinterpret_cast<Address>(simulator_->get_register(Simulator::sp));
     state->fp =
@@ -592,6 +668,11 @@ DISABLE_ASAN void TickSample::Init(Isolate* isolate,
   Address js_entry_sp = isolate->js_entry_sp();
   if (js_entry_sp == 0) return;  // Not executing JS now.
 
+  if (pc && IsNoFrameRegion(pc)) {
+    pc = 0;
+    return;
+  }
+
   ExternalCallbackScope* scope = isolate->external_callback_scope();
   Address handler = Isolate::handler(isolate->thread_local_top());
   // If there is a handler on top of the external callback scope then