[profiler] Make no frame region detection code more robust [x86/x64]

src/sampler.cc

Index: src/sampler.cc
diff --git a/src/sampler.cc b/src/sampler.cc
index 4e669ca6dc13611fdda5bacc728d2f1227592646..55269d8714561c2a588d6ce0466095e86f2b14f7 100644
--- a/src/sampler.cc
+++ b/src/sampler.cc
@@ -173,6 +173,49 @@ class PlatformDataCommon : public Malloced {
   ThreadId profiled_thread_id_;
 };
 
+
+// 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 = pattern->offsets; *offset != -1; ++offset) {
+      if (!memcmp(pc - *offset, pattern->bytes, pattern->bytes_count))

[yurys, 2015/09/16 14:42:01]

what about the potential case that we discussed offline, when pc is begin of a
page and we cannot access pc -1 ?

[alph, 2015/09/16 17:28:50]

It should never happen for JS code, because of Code object preceding  and in
fact every heap allocated objects are preceded with the system info.

As for the native code, I looked at the memory map for Chrome process and all
the code segments have ELF prologue, e.g.
(gdb) x/8c 0x7fb6cc498000
0x7fb6cc498000:	127 '\177'	69 'E'	76 'L'	70 'F'	2 '\002'	1 '\001'	1 '\001'	0
'\000'

So it looks like we should never hit an unallocated page. Not sure how the
loading behaves on Windows.

In any case we can monitor it for crashes. Or I can still add a check code just
to be on the safe side, but I don't think it's worth.

Wdyt?

+        return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 #if defined(USE_SIGNALS)
@@ -592,6 +635,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