From 486932a225b21ffce9ba8557990d5ad4d135963d Mon Sep 17 00:00:00 2001 From: Mathieu Chartier Date: Wed, 24 Feb 2016 10:09:23 -0800 Subject: [PATCH] Add MapAnonymous handling for null error_str We use MapAnonymous with null error_str for app image loading when we want to fail quickly. Also avoid doing CheckNonOverlapping in CheckMapRequest if error_msg is null. The result from CheckNonOverlapping is unused and CheckNonOverlapping is slow since it creates a backtrace map. Bug: 22858531 Bug: 26746779 (cherry picked from commit 83723aedac536fd8a3cd6e1662dbd6260e576194) Change-Id: I0ff03a778b36303aa1e256fe7238dece5b3bbfab --- runtime/mem_map.cc | 34 ++++++++++++++++++++-------------- runtime/mem_map_test.cc | 13 +++++++++++++ 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/runtime/mem_map.cc b/runtime/mem_map.cc index c908b3920..11156c622 100644 --- a/runtime/mem_map.cc +++ b/runtime/mem_map.cc @@ -230,17 +230,16 @@ static bool CheckMapRequest(uint8_t* expected_ptr, void* actual_ptr, size_t byte PLOG(WARNING) << StringPrintf("munmap(%p, %zd) failed", actual_ptr, byte_count); } - // We call this here so that we can try and generate a full error - // message with the overlapping mapping. There's no guarantee that - // that there will be an overlap though, since - // - The kernel is not *required* to honor expected_ptr unless MAP_FIXED is - // true, even if there is no overlap - // - There might have been an overlap at the point of mmap, but the - // overlapping region has since been unmapped. - std::string error_detail; - CheckNonOverlapping(expected, limit, &error_detail); - if (error_msg != nullptr) { + // We call this here so that we can try and generate a full error + // message with the overlapping mapping. There's no guarantee that + // that there will be an overlap though, since + // - The kernel is not *required* to honor expected_ptr unless MAP_FIXED is + // true, even if there is no overlap + // - There might have been an overlap at the point of mmap, but the + // overlapping region has since been unmapped. + std::string error_detail; + CheckNonOverlapping(expected, limit, &error_detail); std::ostringstream os; os << StringPrintf("Failed to mmap at expected address, mapped at " "0x%08" PRIxPTR " instead of 0x%08" PRIxPTR, @@ -338,11 +337,18 @@ MemMap* MemMap::MapAnonymous(const char* name, saved_errno = errno; if (actual == MAP_FAILED) { - PrintFileToLog("/proc/self/maps", LogSeverity::WARNING); + if (error_msg != nullptr) { + PrintFileToLog("/proc/self/maps", LogSeverity::WARNING); - *error_msg = StringPrintf("Failed anonymous mmap(%p, %zd, 0x%x, 0x%x, %d, 0): %s. See process " - "maps in the log.", expected_ptr, page_aligned_byte_count, prot, - flags, fd.get(), strerror(saved_errno)); + *error_msg = StringPrintf("Failed anonymous mmap(%p, %zd, 0x%x, 0x%x, %d, 0): %s. " + "See process maps in the log.", + expected_ptr, + page_aligned_byte_count, + prot, + flags, + fd.get(), + strerror(saved_errno)); + } return nullptr; } std::ostringstream check_map_request_error_msg; diff --git a/runtime/mem_map_test.cc b/runtime/mem_map_test.cc index 81c855e73..e703b78cf 100644 --- a/runtime/mem_map_test.cc +++ b/runtime/mem_map_test.cc @@ -164,6 +164,19 @@ TEST_F(MemMapTest, MapAnonymousEmpty) { ASSERT_TRUE(error_msg.empty()); } +TEST_F(MemMapTest, MapAnonymousFailNullError) { + CommonInit(); + // Test that we don't crash with a null error_str when mapping at an invalid location. + std::unique_ptr map(MemMap::MapAnonymous("MapAnonymousInvalid", + reinterpret_cast(kPageSize), + 0x20000, + PROT_READ | PROT_WRITE, + false, + false, + nullptr)); + ASSERT_EQ(nullptr, map.get()); +} + #ifdef __LP64__ TEST_F(MemMapTest, MapAnonymousEmpty32bit) { CommonInit(); -- 2.11.0