From 08b7b1d06014784edf0e844e488eebcce1537f48 Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Wed, 14 Jun 2017 11:14:19 -0700 Subject: [PATCH] [fuchsia] Make profile processing resilient to bogus overlaps from dladdr. R=zra@google.com Review-Url: https://codereview.chromium.org/2939853002 . --- runtime/vm/profiler_service.cc | 350 ++++++++++++++++++--------------- runtime/vm/profiler_service.h | 47 ++++- runtime/vm/profiler_test.cc | 106 ++++++++++ 3 files changed, 342 insertions(+), 161 deletions(-) diff --git a/runtime/vm/profiler_service.cc b/runtime/vm/profiler_service.cc index f245ed5f296..9a4fe3b6916 100644 --- a/runtime/vm/profiler_service.cc +++ b/runtime/vm/profiler_service.cc @@ -302,10 +302,31 @@ ProfileCode::ProfileCode(Kind kind, address_ticks_(0) {} -void ProfileCode::AdjustExtent(uword start, uword end) { +void ProfileCode::TruncateLower(uword start) { + if (start > start_) { + start_ = start; + } + ASSERT(start_ < end_); +} + + +void ProfileCode::TruncateUpper(uword end) { + if (end < end_) { + end_ = end; + } + ASSERT(start_ < end_); +} + + +void ProfileCode::ExpandLower(uword start) { if (start < start_) { start_ = start; } + ASSERT(start_ < end_); +} + + +void ProfileCode::ExpandUpper(uword end) { if (end > end_) { end_ = end; } @@ -695,172 +716,163 @@ ProfileFunction* ProfileCode::SetFunctionAndName(ProfileFunctionTable* table) { } -typedef bool (*RangeCompare)(uword pc, uword region_start, uword region_end); - -class ProfileCodeTable : public ZoneAllocated { - public: - ProfileCodeTable() : table_(8) {} - - intptr_t length() const { return table_.length(); } - - ProfileCode* At(intptr_t index) const { - ASSERT(index >= 0); - ASSERT(index < length()); - return table_[index]; +intptr_t ProfileCodeTable::FindCodeIndexForPC(uword pc) const { + intptr_t length = table_.length(); + if (length == 0) { + return -1; // Not found. } - - // Find the table index to the ProfileCode containing pc. - // Returns < 0 if not found. - intptr_t FindCodeIndexForPC(uword pc) const { - intptr_t index = FindCodeIndex(pc, &CompareLowerBound); - if (index == length()) { - // Not present. - return -1; - } - const ProfileCode* code = At(index); - if (!code->Contains(pc)) { - // Not present. - return -1; - } - // Found at index. - return index; - } - - ProfileCode* FindCodeForPC(uword pc) const { - intptr_t index = FindCodeIndexForPC(pc); - if (index < 0) { - return NULL; - } - return At(index); - } - - // Insert |new_code| into the table. Returns the table index where |new_code| - // was inserted. Will merge with an overlapping ProfileCode if one is present. - intptr_t InsertCode(ProfileCode* new_code) { - const uword start = new_code->start(); - const uword end = new_code->end(); - const intptr_t length = table_.length(); - if (length == 0) { - table_.Add(new_code); - return length; - } - // Determine the correct place to insert or merge |new_code| into table. - intptr_t lo = FindCodeIndex(start, &CompareLowerBound); - intptr_t hi = FindCodeIndex(end - 1, &CompareUpperBound); - // TODO(johnmccutchan): Simplify below logic. - if ((lo == length) && (hi == length)) { - lo = length - 1; - } - if (lo == length) { - ProfileCode* code = At(hi); - if (code->Overlaps(new_code)) { - HandleOverlap(code, new_code, start, end); - return hi; - } - table_.Add(new_code); - return length; - } else if (hi == length) { - ProfileCode* code = At(lo); - if (code->Overlaps(new_code)) { - HandleOverlap(code, new_code, start, end); - return lo; - } - table_.Add(new_code); - return length; - } else if (lo == hi) { - ProfileCode* code = At(lo); - if (code->Overlaps(new_code)) { - HandleOverlap(code, new_code, start, end); - return lo; - } - table_.InsertAt(lo, new_code); - return lo; + intptr_t lo = 0; + intptr_t hi = length - 1; + while (lo <= hi) { + intptr_t mid = (hi - lo + 1) / 2 + lo; + ASSERT(mid >= lo); + ASSERT(mid <= hi); + ProfileCode* code = At(mid); + if (code->Contains(pc)) { + return mid; + } else if (pc < code->start()) { + hi = mid - 1; } else { - ProfileCode* code = At(lo); - if (code->Overlaps(new_code)) { - HandleOverlap(code, new_code, start, end); - return lo; - } - code = At(hi); - if (code->Overlaps(new_code)) { - HandleOverlap(code, new_code, start, end); - return hi; - } - table_.InsertAt(hi, new_code); - return hi; - } - UNREACHABLE(); - return -1; - } - - private: - intptr_t FindCodeIndex(uword pc, RangeCompare comparator) const { - ASSERT(comparator != NULL); - intptr_t count = table_.length(); - intptr_t first = 0; - while (count > 0) { - intptr_t it = first; - intptr_t step = count / 2; - it += step; - const ProfileCode* code = At(it); - if (comparator(pc, code->start(), code->end())) { - first = ++it; - count -= (step + 1); - } else { - count = step; - } - } - return first; - } - - static bool CompareUpperBound(uword pc, uword start, uword end) { - return pc >= end; - } - - static bool CompareLowerBound(uword pc, uword start, uword end) { - return end <= pc; - } - - void HandleOverlap(ProfileCode* existing, - ProfileCode* code, - uword start, - uword end) { - // We should never see overlapping Dart code regions. - ASSERT(existing->kind() != ProfileCode::kDartCode); - // We should never see overlapping Tag code regions. - ASSERT(existing->kind() != ProfileCode::kTagCode); - // When code regions overlap, they should be of the same kind. - ASSERT(existing->kind() == code->kind()); - existing->AdjustExtent(start, end); - } - - void VerifyOrder() { - const intptr_t length = table_.length(); - if (length == 0) { - return; - } - uword last = table_[0]->end(); - for (intptr_t i = 1; i < length; i++) { - ProfileCode* a = table_[i]; - ASSERT(last <= a->start()); - last = a->end(); + lo = mid + 1; } } + return -1; +} - void VerifyOverlap() { - const intptr_t length = table_.length(); - for (intptr_t i = 0; i < length; i++) { - ProfileCode* a = table_[i]; - for (intptr_t j = i + 1; j < length; j++) { - ProfileCode* b = table_[j]; - ASSERT(!a->Contains(b->start()) && !a->Contains(b->end() - 1) && - !b->Contains(a->start()) && !b->Contains(a->end() - 1)); - } - } + +intptr_t ProfileCodeTable::InsertCode(ProfileCode* new_code) { + const intptr_t length = table_.length(); + if (length == 0) { + table_.Add(new_code); + return length; } - ZoneGrowableArray table_; -}; + // Determine the correct place to insert or merge |new_code| into table. + intptr_t lo = -1; + intptr_t hi = -1; + ProfileCode* lo_code = NULL; + ProfileCode* hi_code = NULL; + const uword pc = new_code->end() - 1; + FindNeighbors(pc, &lo, &hi, &lo_code, &hi_code); + ASSERT((lo_code != NULL) || (hi_code != NULL)); + + if (lo != -1) { + // Has left neighbor. + new_code->TruncateLower(lo_code->end()); + ASSERT(!new_code->Overlaps(lo_code)); + } + if (hi != -1) { + // Has right neighbor. + new_code->TruncateUpper(hi_code->start()); + ASSERT(!new_code->Overlaps(hi_code)); + } + + if ((lo != -1) && (lo_code->kind() == ProfileCode::kNativeCode) && + (new_code->kind() == ProfileCode::kNativeCode) && + (lo_code->end() == new_code->start())) { + // Adjacent left neighbor of the same kind: merge. + // (dladdr doesn't give us symbol size so processing more samples may see + // more PCs we didn't previously know belonged to it.) + lo_code->ExpandUpper(new_code->end()); + return lo; + } + + if ((hi != -1) && (hi_code->kind() == ProfileCode::kNativeCode) && + (new_code->kind() == ProfileCode::kNativeCode) && + (new_code->end() == hi_code->start())) { + // Adjacent right neighbor of the same kind: merge. + // (dladdr doesn't give us symbol size so processing more samples may see + // more PCs we didn't previously know belonged to it.) + hi_code->ExpandLower(new_code->start()); + return hi; + } + + intptr_t insert; + if (lo == -1) { + insert = 0; + } else if (hi == -1) { + insert = length; + } else { + insert = lo + 1; + } + table_.InsertAt(insert, new_code); + return insert; +} + + +void ProfileCodeTable::FindNeighbors(uword pc, + intptr_t* lo, + intptr_t* hi, + ProfileCode** lo_code, + ProfileCode** hi_code) const { + ASSERT(table_.length() >= 1); + + intptr_t length = table_.length(); + + if (pc < At(0)->start()) { + // Lower than any existing code. + *lo = -1; + *lo_code = NULL; + *hi = 0; + *hi_code = At(*hi); + return; + } + + if (pc >= At(length - 1)->end()) { + // Higher than any existing code. + *lo = length - 1; + *lo_code = At(*lo); + *hi = -1; + *hi_code = NULL; + return; + } + + *lo = 0; + *lo_code = At(*lo); + *hi = length - 1; + *hi_code = At(*hi); + + while ((*hi - *lo) > 1) { + intptr_t mid = (*hi - *lo + 1) / 2 + *lo; + ASSERT(*lo <= mid); + ASSERT(*hi >= mid); + ProfileCode* code = At(mid); + if (code->end() <= pc) { + *lo = mid; + *lo_code = code; + } + if (pc < code->end()) { + *hi = mid; + *hi_code = code; + } + } +} + + +void ProfileCodeTable::VerifyOrder() { + const intptr_t length = table_.length(); + if (length == 0) { + return; + } + uword last = table_[0]->end(); + for (intptr_t i = 1; i < length; i++) { + ProfileCode* a = table_[i]; + ASSERT(last <= a->start()); + last = a->end(); + } +} + +void ProfileCodeTable::VerifyOverlap() { + const intptr_t length = table_.length(); + for (intptr_t i = 0; i < length; i++) { + ProfileCode* a = table_[i]; + for (intptr_t j = i + 1; j < length; j++) { + ProfileCode* b = table_[j]; + ASSERT(!a->Contains(b->start()) && !a->Contains(b->end() - 1) && + !b->Contains(a->start()) && !b->Contains(a->end() - 1)); + } + } +} ProfileTrieNode::ProfileTrieNode(intptr_t table_index) @@ -2239,6 +2251,24 @@ class ProfileBuilder : public ValueObject { native_start &= ~1; #endif + if (native_start > pc) { + // Bogus lookup result. + if (native_name != NULL) { + NativeSymbolResolver::FreeSymbolName(native_name); + native_name = NULL; + } + native_start = pc; + } + if ((pc - native_start) > (32 * KB)) { + // Suspect lookup result. More likely dladdr going off the rails than a + // jumbo function. + if (native_name != NULL) { + NativeSymbolResolver::FreeSymbolName(native_name); + native_name = NULL; + } + native_start = pc; + } + ASSERT(pc >= native_start); profile_code = new ProfileCode(ProfileCode::kNativeCode, native_start, pc + 1, 0, code); diff --git a/runtime/vm/profiler_service.h b/runtime/vm/profiler_service.h index 8045906e144..b746f510626 100644 --- a/runtime/vm/profiler_service.h +++ b/runtime/vm/profiler_service.h @@ -171,7 +171,10 @@ class ProfileCode : public ZoneAllocated { uword end() const { return end_; } void set_end(uword end) { end_ = end; } - void AdjustExtent(uword start, uword end); + void ExpandLower(uword start); + void ExpandUpper(uword end); + void TruncateLower(uword start); + void TruncateUpper(uword end); bool Contains(uword pc) const { return (pc >= start_) && (pc < end_); } @@ -239,6 +242,48 @@ class ProfileCode : public ZoneAllocated { }; +class ProfileCodeTable : public ZoneAllocated { + public: + ProfileCodeTable() : table_(8) {} + + intptr_t length() const { return table_.length(); } + + ProfileCode* At(intptr_t index) const { + ASSERT(index >= 0); + ASSERT(index < length()); + return table_[index]; + } + + // Find the table index to the ProfileCode containing pc. + // Returns < 0 if not found. + intptr_t FindCodeIndexForPC(uword pc) const; + + ProfileCode* FindCodeForPC(uword pc) const { + intptr_t index = FindCodeIndexForPC(pc); + if (index < 0) { + return NULL; + } + return At(index); + } + + // Insert |new_code| into the table. Returns the table index where |new_code| + // was inserted. Will merge with an overlapping ProfileCode if one is present. + intptr_t InsertCode(ProfileCode* new_code); + + private: + void FindNeighbors(uword pc, + intptr_t* lo, + intptr_t* hi, + ProfileCode** lo_code, + ProfileCode** hi_code) const; + + void VerifyOrder(); + void VerifyOverlap(); + + ZoneGrowableArray table_; +}; + + // Stack traces are organized in a trie. This holds information about one node // in the trie. A node in a tree represents a stack frame and a path in the tree // represents a stack trace. Each unique stack trace appears in the tree once diff --git a/runtime/vm/profiler_test.cc b/runtime/vm/profiler_test.cc index 8dc7aea3266..17f0fdc41ee 100644 --- a/runtime/vm/profiler_test.cc +++ b/runtime/vm/profiler_test.cc @@ -2809,6 +2809,112 @@ TEST_CASE(Profiler_GetSourceReport) { EXPECT_SUBSTRING("\"inclusiveTicks\":[1,2]", js.ToCString()); } + +TEST_CASE(Profiler_ProfileCodeTableTest) { + Zone* Z = Thread::Current()->zone(); + + ProfileCodeTable* table = new (Z) ProfileCodeTable(); + EXPECT_EQ(table->length(), 0); + EXPECT_EQ(table->FindCodeForPC(42), static_cast(NULL)); + + int64_t timestamp = 0; + Code& null_code = Code::Handle(Z); + + ProfileCode* code1 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 50, 60, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code1), 0); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(55), code1); + EXPECT_EQ(table->FindCodeForPC(59), code1); + EXPECT_EQ(table->FindCodeForPC(60), static_cast(NULL)); + + // Insert below all. + ProfileCode* code2 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 10, 20, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code2), 0); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(19), code2); + EXPECT_EQ(table->FindCodeForPC(20), static_cast(NULL)); + + // Insert above all. + ProfileCode* code3 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 80, 90, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code3), 2); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(80), code3); + EXPECT_EQ(table->FindCodeForPC(89), code3); + EXPECT_EQ(table->FindCodeForPC(90), static_cast(NULL)); + + // Insert between. + ProfileCode* code4 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 65, 75, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code4), 2); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(80), code3); + EXPECT_EQ(table->FindCodeForPC(65), code4); + EXPECT_EQ(table->FindCodeForPC(74), code4); + EXPECT_EQ(table->FindCodeForPC(75), static_cast(NULL)); + + // Insert overlapping left. + ProfileCode* code5 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 15, 25, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code5), 0); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(80), code3); + EXPECT_EQ(table->FindCodeForPC(65), code4); + EXPECT_EQ(table->FindCodeForPC(15), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(24), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(25), static_cast(NULL)); + + // Insert overlapping right. + ProfileCode* code6 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 45, 55, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code6), 1); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(80), code3); + EXPECT_EQ(table->FindCodeForPC(65), code4); + EXPECT_EQ(table->FindCodeForPC(15), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(24), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(45), code1); // Merged right. + EXPECT_EQ(table->FindCodeForPC(54), code1); // Merged right. + EXPECT_EQ(table->FindCodeForPC(55), code1); + + // Insert overlapping both. + ProfileCode* code7 = new (Z) + ProfileCode(ProfileCode::kNativeCode, 20, 50, timestamp, null_code); + EXPECT_EQ(table->InsertCode(code7), 0); + EXPECT_EQ(table->FindCodeForPC(0), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(100), static_cast(NULL)); + EXPECT_EQ(table->FindCodeForPC(50), code1); + EXPECT_EQ(table->FindCodeForPC(10), code2); + EXPECT_EQ(table->FindCodeForPC(80), code3); + EXPECT_EQ(table->FindCodeForPC(65), code4); + EXPECT_EQ(table->FindCodeForPC(15), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(24), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(45), code1); // Merged right. + EXPECT_EQ(table->FindCodeForPC(54), code1); // Merged right. + EXPECT_EQ(table->FindCodeForPC(20), code2); // Merged left. + EXPECT_EQ(table->FindCodeForPC(49), code1); // Truncated. + EXPECT_EQ(table->FindCodeForPC(50), code1); +} + #endif // !PRODUCT } // namespace dart