Skip samples where top function's stack frame is not setup properly

src/profile-generator.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 11 matching lines @@
 // LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 // DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 // THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "v8.h"
 
 #include "profile-generator-inl.h"
 
+#include "compiler.h"
 #include "global-handles.h"
 #include "scopeinfo.h"
 #include "unicode.h"
 #include "zone-inl.h"
 #include "debug.h"
 
 namespace v8 {
 namespace internal {
 
 
@@ skipping 137 matching lines @@
   size += sizeof(HashMap::Entry) * names_.capacity();
   for (HashMap::Entry* p = names_.Start(); p != NULL; p = names_.Next(p)) {
     size += strlen(reinterpret_cast<const char*>(p->value)) + 1;
   }
   return size;
 }
 
 const char* const CodeEntry::kEmptyNamePrefix = "";
 
 
+CodeEntry::~CodeEntry() {
+  delete no_frame_ranges_;
+}
+
+
 void CodeEntry::CopyData(const CodeEntry& source) {
   tag_ = source.tag_;
   name_prefix_ = source.name_prefix_;
   name_ = source.name_;
   resource_name_ = source.resource_name_;
   line_number_ = source.line_number_;
 }
 
 
 uint32_t CodeEntry::GetCallUid() const {
@@ skipping 325 matching lines @@
     CodeTree::Locator locator;
     if (!tree_.FindGreatestLessThan(addr, &locator)) break;
     Address start2 = locator.key(), end2 = start2 + locator.value().size;
     if (start2 < end && start < end2) to_delete.Add(start2);
     addr = start2 - 1;
   }
   for (int i = 0; i < to_delete.length(); ++i) tree_.Remove(to_delete[i]);
 }
 
 
-CodeEntry* CodeMap::FindEntry(Address addr) {
+CodeEntry* CodeMap::FindEntry(Address addr, Address* start) {
   CodeTree::Locator locator;
   if (tree_.FindGreatestLessThan(addr, &locator)) {
     // locator.key() <= addr. Need to check that addr is within entry.
     const CodeEntryInfo& entry = locator.value();
-    if (addr < (locator.key() + entry.size))
+    if (addr < (locator.key() + entry.size)) {
+      if (start)
+        *start = locator.key();

[Jakob Kummerow, 2013/05/14 12:38:51]

nit: {} please

[yurys, 2013/05/14 22:51:19]

Done.

       return entry.entry;
+    }
   }
   return NULL;
 }
 
 
 int CodeMap::GetSharedId(Address addr) {
   CodeTree::Locator locator;
   // For shared function entries, 'size' field is used to store their IDs.
   if (tree_.Find(addr, &locator)) {
     const CodeEntryInfo& entry = locator.value();
@@ skipping 340 matching lines @@
 
 
 void ProfileGenerator::RecordTickSample(const TickSample& sample) {
   // Allocate space for stack frames + pc + function + vm-state.
   ScopedVector<CodeEntry*> entries(sample.frames_count + 3);
   // As actual number of decoded code entries may vary, initialize
   // entries vector with NULL values.
   CodeEntry** entry = entries.start();
   memset(entry, 0, entries.length() * sizeof(*entry));
   if (sample.pc != NULL) {
-    *entry++ = code_map_.FindEntry(sample.pc);
+    Address start;
+    CodeEntry* pc_entry = code_map_.FindEntry(sample.pc, &start);
+    // If pc is in the function code before it setup stack frame or after the

[Jakob Kummerow, 2013/05/14 12:38:51]

nit: s/setup/set up/

[yurys, 2013/05/14 22:51:19]

Done.

+    // frame was destroyed SafeStackTraceFrameIterator incorrectly thinks that
+    // ebp cotains return address of the current function and skips caller's

[Jakob Kummerow, 2013/05/14 12:38:51]

nit: contains

[yurys, 2013/05/14 22:51:19]

Done.

+    // frame. Check for this case and just skip such samples.
+    if (pc_entry) {
+      List<OffsetRange>* ranges = pc_entry->no_frame_ranges();
+      if (ranges) {
+        Code* code = Code::cast(HeapObject::FromAddress(start));
+        int pc_offset = sample.pc - code->instruction_start();
+        for (int i = 0; i < ranges->length(); i++) {
+          OffsetRange& range = ranges->at(i);
+          if (range.from <= pc_offset && pc_offset < range.to) {
+            return;
+          }
+        }
+      }
+    }
+    *entry++ = pc_entry;
 
     if (sample.has_external_callback) {
       // Don't use PC when in external callback code, as it can point
       // inside callback's code, and we will erroneously report
       // that a callback calls itself.
       *(entries.start()) = NULL;
       *entry++ = code_map_.FindEntry(sample.external_callback);
     }
 
     for (const Address* stack_pos = sample.stack,
@@ skipping 16 matching lines @@
     if (no_symbolized_entries) {
       *entry++ = EntryForVMState(sample.state);
     }
   }
 
   profiles_->AddPathToCurrentProfiles(entries);
 }
 
 
 } }  // namespace v8::internal