commit 3d67d347f5322ca6d4aeac790368bff1f50e27b1
author Florian Mayer <fmayer@google.com> Wed Feb 27 18:00:37 2019 +0000
committer Florian Mayer <fmayer@google.com> Tue Mar 05 13:05:36 2019 +0000
tree 7f7972210500803c11055447dbe592fd6db236ee
parent d5345f58fdb11f9f362b00bbfd9d7b19ca659fcc

Fix copy / move behaviour of Maps object.

Currently, moving or copying a Maps object leads to double free of MapInfo.

Even moving a Maps object  did not prevent this, as after a move
the object only has to be in an "unspecified but valid state", which can
be the original state for a vector of raw pointers (but not for a vector
of unique_ptrs).

Changing to unique_ptrs is the most failsafe way to make sure we never
accidentally destruct MapInfo.

Test: atest libuwindstack_test
      Failed LocalUnwinderTest#unwind_after_dlopen which also fails at master.

Change-Id: Id1c9739b334da5c1ba532fd55366e115940a66d3

libunwindstack/tests/MapsTest.cpp

diff --git a/libunwindstack/tests/MapsTest.cpp b/libunwindstack/tests/MapsTest.cpp
index b4197f2..9e7a6ab 100644
--- a/libunwindstack/tests/MapsTest.cpp
+++ b/libunwindstack/tests/MapsTest.cpp
@@ -61,6 +61,26 @@
   ASSERT_EQ(0U, info->load_bias.load());
 }
 
+TEST(MapsTest, map_move) {
+  Maps maps;
+
+  maps.Add(0x1000, 0x2000, 0, PROT_READ, "fake_map", 0);
+  maps.Add(0x3000, 0x4000, 0x10, 0, "", 0x1234);
+  maps.Add(0x5000, 0x6000, 1, 2, "fake_map2", static_cast<uint64_t>(-1));
+
+  Maps maps2 = std::move(maps);
+
+  ASSERT_EQ(3U, maps2.Total());
+  MapInfo* info = maps2.Get(0);
+  ASSERT_EQ(0x1000U, info->start);
+  ASSERT_EQ(0x2000U, info->end);
+  ASSERT_EQ(0U, info->offset);
+  ASSERT_EQ(PROT_READ, info->flags);
+  ASSERT_EQ("fake_map", info->name);
+  ASSERT_EQ(0U, info->elf_offset);
+  ASSERT_EQ(0U, info->load_bias.load());
+}
+
 TEST(MapsTest, verify_parse_line) {
   MapInfo info(nullptr, 0, 0, 0, 0, "");
 

libunwindstack/tests/UnwinderTest.cpp

diff --git a/libunwindstack/tests/UnwinderTest.cpp b/libunwindstack/tests/UnwinderTest.cpp
index d88531f..2dc5118 100644
--- a/libunwindstack/tests/UnwinderTest.cpp
+++ b/libunwindstack/tests/UnwinderTest.cpp
@@ -49,7 +49,7 @@
     std::string str_name(name);
     maps_->Add(start, end, offset, flags, name, static_cast<uint64_t>(-1));
     if (elf != nullptr) {
-      MapInfo* map_info = *--maps_->end();
+      const auto& map_info = *--maps_->end();
       map_info->elf.reset(elf);
     }
   }
@@ -85,7 +85,7 @@
     AddMapInfo(0x53000, 0x54000, 0, PROT_READ | PROT_WRITE, "/fake/fake.oat");
 
     AddMapInfo(0xa3000, 0xa4000, 0, PROT_READ | PROT_WRITE | PROT_EXEC, "/fake/fake.vdex");
-    MapInfo* info = *--maps_->end();
+    const auto& info = *--maps_->end();
     info->load_bias = 0;
 
     elf = new ElfFake(new MemoryFake);
@@ -98,8 +98,8 @@
     elf->FakeSetInterface(new ElfInterfaceFake(nullptr));
     AddMapInfo(0xa7000, 0xa8000, 0, PROT_READ | PROT_WRITE | PROT_EXEC, "/fake/fake_offset.oat",
                elf);
-    info = *--maps_->end();
-    info->elf_offset = 0x8000;
+    const auto& info2 = *--maps_->end();
+    info2->elf_offset = 0x8000;
 
     process_memory_.reset(new MemoryFake);
   }