diff options
author | Narayan Kamath <narayan@google.com> | 2014-08-21 17:38:09 +0100 |
---|---|---|
committer | Brian Carlstrom <bdc@google.com> | 2014-08-21 14:29:14 -0700 |
commit | b89c3da592de1a2741a08cc3c8ed2908e8bdd15a (patch) | |
tree | 3a55d3f92f7dfd476706bff2c53955ef8ebd217f /runtime/mem_map.cc | |
parent | ca0ceb06d1b82714b834727a33f82fe6512effa9 (diff) | |
download | art-b89c3da592de1a2741a08cc3c8ed2908e8bdd15a.tar.gz art-b89c3da592de1a2741a08cc3c8ed2908e8bdd15a.tar.bz2 art-b89c3da592de1a2741a08cc3c8ed2908e8bdd15a.zip |
Make a couple of map checks debug only.
This cost us close to 80ms in app startup times.
The checks that a reused region was within an already
existent map has been demoted to a debug check.
A couple of other negative checks have been removed
outright because one of them was superflous and the other
wasn't guaranteed to be correct.
bug: 16828525
(cherry picked from commit bddaea2b88b0a19d9cc7a4dea772af8e829323b3)
Change-Id: Ia6f3e69692bb9cb5b4ff6f47946ea38a56d4cdb6
Diffstat (limited to 'runtime/mem_map.cc')
-rw-r--r-- | runtime/mem_map.cc | 40 |
1 files changed, 27 insertions, 13 deletions
diff --git a/runtime/mem_map.cc b/runtime/mem_map.cc index c281b2200f..6cda258847 100644 --- a/runtime/mem_map.cc +++ b/runtime/mem_map.cc @@ -131,9 +131,9 @@ uintptr_t MemMap::next_mem_pos_ = GenerateNextMemPos(); #endif // Return true if the address range is contained in a single /proc/self/map entry. -static bool CheckOverlapping(uintptr_t begin, - uintptr_t end, - std::string* error_msg) { +static bool ContainedWithinExistingMap(uintptr_t begin, + uintptr_t end, + std::string* error_msg) { std::unique_ptr<BacktraceMap> map(BacktraceMap::Create(getpid(), true)); if (map.get() == nullptr) { *error_msg = StringPrintf("Failed to build process map"); @@ -212,12 +212,25 @@ static bool CheckMapRequest(byte* expected_ptr, void* actual_ptr, size_t byte_co PLOG(WARNING) << StringPrintf("munmap(%p, %zd) failed", actual_ptr, byte_count); } - if (!CheckNonOverlapping(expected, limit, error_msg)) { - return false; + // 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 honour 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, + actual, expected); + if (!error_detail.empty()) { + os << " : " << error_detail; } - *error_msg = StringPrintf("Failed to mmap at expected address, mapped at " - "0x%08" PRIxPTR " instead of 0x%08" PRIxPTR, actual, expected); + *error_msg = os.str(); return false; } @@ -375,19 +388,20 @@ MemMap* MemMap::MapFileAtAddress(byte* expected_ptr, size_t byte_count, int prot CHECK_NE(0, flags & (MAP_SHARED | MAP_PRIVATE)); uintptr_t expected = reinterpret_cast<uintptr_t>(expected_ptr); uintptr_t limit = expected + byte_count; + + // Note that we do not allow MAP_FIXED unless reuse == true, i.e we + // expect his mapping to be contained within an existing map. if (reuse) { // reuse means it is okay that it overlaps an existing page mapping. // Only use this if you actually made the page reservation yourself. CHECK(expected_ptr != nullptr); - if (!CheckOverlapping(expected, limit, error_msg)) { - return nullptr; - } + + DCHECK(ContainedWithinExistingMap(expected, limit, error_msg)); flags |= MAP_FIXED; } else { CHECK_EQ(0, flags & MAP_FIXED); - if (expected_ptr != nullptr && !CheckNonOverlapping(expected, limit, error_msg)) { - return nullptr; - } + // Don't bother checking for an overlapping region here. We'll + // check this if required after the fact inside CheckMapRequest. } if (byte_count == 0) { |