OSDN Git Service

simpleperf: fix according to unwindstack change.
authorYabin Cui <yabinc@google.com>
Thu, 23 Jan 2020 20:32:26 +0000 (12:32 -0800)
committerYabin Cui <yabinc@google.com>
Thu, 23 Jan 2020 20:39:01 +0000 (12:39 -0800)
unwindstack::Maps uses prev_real_map instead of prev_map, which
breaks simpleperf code building up prev_map links. So switch to
use unwindstack::Maps::Sort() to build up prev_real_map links.

Also add more tests.

Bug: 148075852
Test: run simpleperf_unit_test.
Change-Id: Ica0551e2d5a84aa8a0671489821fed1d263324f4

simpleperf/Android.bp
simpleperf/OfflineUnwinder.cpp
simpleperf/OfflineUnwinder_impl.h [new file with mode: 0644]
simpleperf/OfflineUnwinder_test.cpp [new file with mode: 0644]

index f3e1320..b8b59c0 100644 (file)
@@ -536,6 +536,7 @@ cc_defaults {
                 "cmd_trace_sched_test.cpp",
                 "environment_test.cpp",
                 "IOEventLoop_test.cpp",
+                "OfflineUnwinder_test.cpp",
                 "read_dex_file_test.cpp",
                 "record_file_test.cpp",
                 "RecordReadThread_test.cpp",
index 131b5e5..e166d84 100644 (file)
@@ -38,6 +38,7 @@
 #include <unwindstack/UserX86_64.h>
 
 #include "environment.h"
+#include "OfflineUnwinder_impl.h"
 #include "perf_regs.h"
 #include "read_apk.h"
 #include "thread_tree.h"
@@ -135,15 +136,6 @@ static unwindstack::MapInfo* CreateMapInfo(const MapEntry* entry) {
                                   PROT_READ | entry->flags, name);
 }
 
-class UnwindMaps : public unwindstack::Maps {
- public:
-  void UpdateMaps(const MapSet& map_set);
-
- private:
-  uint64_t version_ = 0u;
-  std::vector<const MapEntry*> entries_;
-};
-
 void UnwindMaps::UpdateMaps(const MapSet& map_set) {
   if (version_ == map_set.version) {
     return;
@@ -151,6 +143,7 @@ void UnwindMaps::UpdateMaps(const MapSet& map_set) {
   version_ = map_set.version;
   size_t i = 0;
   size_t old_size = entries_.size();
+  bool has_removed_entry = false;
   for (auto it = map_set.maps.begin(); it != map_set.maps.end();) {
     const MapEntry* entry = it->second;
     if (i < old_size && entry == entries_[i]) {
@@ -163,34 +156,30 @@ void UnwindMaps::UpdateMaps(const MapSet& map_set) {
       ++it;
     } else {
       // Remove an entry.
+      has_removed_entry = true;
       entries_[i] = nullptr;
       maps_[i++] = nullptr;
     }
   }
   while (i < old_size) {
+    has_removed_entry = true;
     entries_[i] = nullptr;
     maps_[i++] = nullptr;
   }
+
+  if (has_removed_entry) {
+    entries_.resize(std::remove(entries_.begin(), entries_.end(), nullptr) - entries_.begin());
+    maps_.resize(std::remove(maps_.begin(), maps_.end(), std::unique_ptr<unwindstack::MapInfo>()) -
+                 maps_.begin());
+  }
+
   std::sort(entries_.begin(), entries_.end(), [](const auto& e1, const auto& e2) {
-    if (e1 == nullptr || e2 == nullptr) {
-      return e1 != nullptr;
-    }
     return e1->start_addr < e2->start_addr;
   });
-  std::sort(maps_.begin(), maps_.end(),
-            [](const auto& m1, const auto& m2) {
-    if (m1 == nullptr || m2 == nullptr) {
-      return m1 != nullptr;
-    }
-    return m1->start < m2->start;
-  });
-  entries_.resize(map_set.maps.size());
-  maps_.resize(map_set.maps.size());
-  // prev_map is needed by libunwindstack to find the start of an embedded lib in an apk.
+  // Use Sort() to sort maps_ and create prev_real_map links.
+  // prev_real_map is needed by libunwindstack to find the start of an embedded lib in an apk.
   // See http://b/120981155.
-  for (size_t i = 1; i < maps_.size(); ++i) {
-    maps_[i]->prev_map = maps_[i-1].get();
-  }
+  Sort();
 }
 
 class OfflineUnwinderImpl : public OfflineUnwinder {
diff --git a/simpleperf/OfflineUnwinder_impl.h b/simpleperf/OfflineUnwinder_impl.h
new file mode 100644 (file)
index 0000000..01f0336
--- /dev/null
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include <unwindstack/Maps.h>
+
+#include "thread_tree.h"
+
+namespace simpleperf {
+
+class UnwindMaps : public unwindstack::Maps {
+ public:
+  void UpdateMaps(const MapSet& map_set);
+
+ private:
+  uint64_t version_ = 0u;
+  std::vector<const MapEntry*> entries_;
+};
+
+}  // namespace simpleperf
diff --git a/simpleperf/OfflineUnwinder_test.cpp b/simpleperf/OfflineUnwinder_test.cpp
new file mode 100644 (file)
index 0000000..79ba64c
--- /dev/null
@@ -0,0 +1,87 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "OfflineUnwinder_impl.h"
+
+#include <gtest/gtest.h>
+
+using namespace simpleperf;
+
+bool CheckUnwindMaps(UnwindMaps& maps, const MapSet& map_set) {
+  if (maps.Total() != map_set.maps.size()) {
+    return false;
+  }
+  unwindstack::MapInfo* prev_real_map = nullptr;
+  for (size_t i = 0; i < maps.Total(); i++) {
+    unwindstack::MapInfo* info = maps.Get(i);
+    if (info == nullptr || map_set.maps.find(info->start) == map_set.maps.end()) {
+      return false;
+    }
+    if (info->prev_real_map != prev_real_map) {
+      return false;
+    }
+    if (!info->IsBlank()) {
+      prev_real_map = info;
+    }
+  }
+  return true;
+}
+
+TEST(OfflineUnwinder, UnwindMaps) {
+  // 1. Create fake map entries.
+  std::unique_ptr<Dso> fake_dso = Dso::CreateDso(DSO_UNKNOWN_FILE, "unknown");
+  std::vector<MapEntry> map_entries;
+  for (size_t i = 0; i < 10; i++) {
+    map_entries.emplace_back(i, 1, i, fake_dso.get(), false);
+  }
+
+  // 2. Init with empty maps.
+  MapSet map_set;
+  UnwindMaps maps;
+  maps.UpdateMaps(map_set);
+  ASSERT_TRUE(CheckUnwindMaps(maps, map_set));
+
+  // 3. Add maps starting from even addr.
+  map_set.version = 1;
+  for (size_t i = 0; i < map_entries.size(); i += 2) {
+    map_set.maps.insert(std::make_pair(map_entries[i].start_addr, &map_entries[i]));
+  }
+
+  maps.UpdateMaps(map_set);
+  ASSERT_TRUE(CheckUnwindMaps(maps, map_set));
+
+  // 4. Add maps starting from odd addr.
+  map_set.version = 2;
+  for (size_t i = 1; i < 10; i += 2) {
+    map_set.maps.insert(std::make_pair(map_entries[i].start_addr, &map_entries[i]));
+  }
+  maps.UpdateMaps(map_set);
+  ASSERT_TRUE(CheckUnwindMaps(maps, map_set));
+
+  // 5. Remove maps starting from even addr.
+  map_set.version = 3;
+  for (size_t i = 0; i < 10; i += 2) {
+    map_set.maps.erase(map_entries[i].start_addr);
+  }
+  maps.UpdateMaps(map_set);
+  ASSERT_TRUE(CheckUnwindMaps(maps, map_set));
+
+  // 6. Remove all maps.
+  map_set.version = 4;
+  map_set.maps.clear();
+  maps.UpdateMaps(map_set);
+  ASSERT_TRUE(CheckUnwindMaps(maps, map_set));
+}