From 15b71ea6466dd17eabf51d33fedf004fb7e9f852 Mon Sep 17 00:00:00 2001
From: Alex Shlyapnikov <alekseys@google.com>
Date: Tue, 28 Nov 2017 22:15:27 +0000
Subject: [PATCH] [LSan] Fix one source of stale segments in the process memory
 mapping.

Summary:
Load process memory map after updating the same cache to reflect the
umap happening in the process of updating.
Also clear out the buffer in case of failed read of /proc/self/maps (not
the source of stale segments, but can lead to the similar crash).

Reviewers: eugenis

Subscribers: llvm-commits, kubamracek

Differential Revision: https://reviews.llvm.org/D40529

llvm-svn: 319237
---
 .../sanitizer_common/sanitizer_procmaps_common.cc  | 50 ++++++++++------------
 .../sanitizer_common/sanitizer_procmaps_linux.cc   |  8 +++-
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cc b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cc
index b9298d4..3982230 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_common.cc
@@ -70,53 +70,49 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) {
 }
 
 MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) {
-  ReadProcMaps(&data_.proc_self_maps);
-  if (cache_enabled) {
-    if (data_.proc_self_maps.mmaped_size == 0) {
-      LoadFromCache();
-      CHECK_GT(data_.proc_self_maps.len, 0);
-    }
-  } else {
-    CHECK_GT(data_.proc_self_maps.mmaped_size, 0);
-  }
-  Reset();
   // FIXME: in the future we may want to cache the mappings on demand only.
   if (cache_enabled)
     CacheMemoryMappings();
+
+  // Read maps after the cache update to capture the maps/unmaps happening in
+  // the process of updating.
+  ReadProcMaps(&data_.proc_self_maps);
+  if (cache_enabled && data_.proc_self_maps.mmaped_size == 0)
+    LoadFromCache();
+  CHECK_GT(data_.proc_self_maps.mmaped_size, 0);
+  CHECK_GT(data_.proc_self_maps.len, 0);
+
+  Reset();
 }
 
 MemoryMappingLayout::~MemoryMappingLayout() {
   // Only unmap the buffer if it is different from the cached one. Otherwise
   // it will be unmapped when the cache is refreshed.
-  if (data_.proc_self_maps.data != cached_proc_self_maps.data) {
+  if (data_.proc_self_maps.data != cached_proc_self_maps.data)
     UnmapOrDie(data_.proc_self_maps.data, data_.proc_self_maps.mmaped_size);
-  }
 }
 
-void MemoryMappingLayout::Reset() { data_.current = data_.proc_self_maps.data; }
+void MemoryMappingLayout::Reset() {
+  data_.current = data_.proc_self_maps.data;
+}
 
 // static
 void MemoryMappingLayout::CacheMemoryMappings() {
-  SpinMutexLock l(&cache_lock);
+  ProcSelfMapsBuff new_proc_self_maps;
+  ReadProcMaps(&new_proc_self_maps);
   // Don't invalidate the cache if the mappings are unavailable.
-  ProcSelfMapsBuff old_proc_self_maps;
-  old_proc_self_maps = cached_proc_self_maps;
-  ReadProcMaps(&cached_proc_self_maps);
-  if (cached_proc_self_maps.mmaped_size == 0) {
-    cached_proc_self_maps = old_proc_self_maps;
-  } else {
-    if (old_proc_self_maps.mmaped_size) {
-      UnmapOrDie(old_proc_self_maps.data,
-                 old_proc_self_maps.mmaped_size);
-    }
-  }
+  if (new_proc_self_maps.mmaped_size == 0)
+    return;
+  SpinMutexLock l(&cache_lock);
+  if (cached_proc_self_maps.mmaped_size)
+    UnmapOrDie(cached_proc_self_maps.data, cached_proc_self_maps.mmaped_size);
+  cached_proc_self_maps = new_proc_self_maps;
 }
 
 void MemoryMappingLayout::LoadFromCache() {
   SpinMutexLock l(&cache_lock);
-  if (cached_proc_self_maps.data) {
+  if (cached_proc_self_maps.data)
     data_.proc_self_maps = cached_proc_self_maps;
-  }
 }
 
 void MemoryMappingLayout::DumpListOfModules(
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_linux.cc b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_linux.cc
index ac894eb..633e939 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_linux.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_linux.cc
@@ -18,8 +18,12 @@
 namespace __sanitizer {
 
 void ReadProcMaps(ProcSelfMapsBuff *proc_maps) {
-  ReadFileToBuffer("/proc/self/maps", &proc_maps->data, &proc_maps->mmaped_size,
-                   &proc_maps->len);
+  if (!ReadFileToBuffer("/proc/self/maps", &proc_maps->data,
+                        &proc_maps->mmaped_size, &proc_maps->len)) {
+    proc_maps->data = nullptr;
+    proc_maps->mmaped_size = 0;
+    proc_maps->len = 0;
+  }
 }
 
 static bool IsOneOf(char c, char c1, char c2) {
-- 
2.7.4