[compiler-rt] r330355 - [sanitizer] Minor refactor of some ReservedAddressRange functions

[Kostya Kortchinsky via llvm-commits]

Author: cryptoad
Date: Thu Apr 19 11:38:15 2018
New Revision: 330355

URL: http://llvm.org/viewvc/llvm-project?rev=330355&view=rev
Log:
[sanitizer] Minor refactor of some ReservedAddressRange functions

Summary:
Some of the functions had spurious conditional statements and checks, and some
intermediary variables that I feel made the code more complicated than it needs
to be. Also, when unmapping the whole range, the range size would be 0, but
the base was set to the address of the end of the range, which sounds prone to
error. I think nulling out the base in this scenario is a better way to go.

Reviewers: alekseyshl, flowerhack

Reviewed By: alekseyshl

Subscribers: kubamracek, delcypher, #sanitizers, llvm-commits

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

Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc?rev=330355&r1=330354&r2=330355&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_fuchsia.cc Thu Apr 19 11:38:15 2018
@@ -274,20 +274,16 @@ void UnmapOrDieVmar(void *addr, uptr siz
   DecreaseTotalMmap(size);
 }
 
-void ReservedAddressRange::Unmap(uptr fixed_addr, uptr size) {
-  uptr offset = fixed_addr - reinterpret_cast<uptr>(base_);
-  uptr addr = reinterpret_cast<uptr>(base_) + offset;
-  void *addr_as_void = reinterpret_cast<void *>(addr);
-  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
-  // Only unmap at the beginning or end of the range.
-  CHECK((addr_as_void == base_) || (addr + size == base_as_uptr + size_));
+void ReservedAddressRange::Unmap(uptr addr, uptr size) {
   CHECK_LE(size, size_);
+  if (addr == reinterpret_cast<uptr>(base_))
+    // If we unmap the whole range, just null out the base.
+    base_ = (size == size_) ? nullptr : reinterpret_cast<void*>(addr + size);
+  else
+    CHECK_EQ(addr + size, reinterpret_cast<uptr>(base_) + size_);
+  size_ -= size;
   UnmapOrDieVmar(reinterpret_cast<void *>(addr), size,
                  static_cast<zx_handle_t>(os_handle_));
-  if (addr_as_void == base_) {
-    base_ = reinterpret_cast<void *>(addr + size);
-  }
-  size_ = size_ - size;
 }
 
 // This should never be called.

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc?rev=330355&r1=330354&r2=330355&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc Thu Apr 19 11:38:15 2018
@@ -346,11 +346,7 @@ uptr ReservedAddressRange::Init(uptr siz
   // `open` (e.g. TSAN, ESAN), then you'll get a failure during initialization.
   // TODO(flowerhack): Fix the implementation of GetNamedMappingFd to solve
   // this problem.
-  if (fixed_addr) {
-    base_ = MmapFixedNoAccess(fixed_addr, size);
-  } else {
-    base_ = MmapNoAccess(size);
-  }
+  base_ = fixed_addr ? MmapFixedNoAccess(fixed_addr, size) : MmapNoAccess(size);
   size_ = size;
   name_ = name;
   (void)os_handle_;  // unsupported
@@ -368,16 +364,14 @@ uptr ReservedAddressRange::MapOrDie(uptr
 }
 
 void ReservedAddressRange::Unmap(uptr addr, uptr size) {
-  void* addr_as_void = reinterpret_cast<void*>(addr);
-  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
-  // Only unmap at the beginning or end of the range.
-  CHECK((addr_as_void == base_) || (addr + size == base_as_uptr + size_));
   CHECK_LE(size, size_);
+  if (addr == reinterpret_cast<uptr>(base_))
+    // If we unmap the whole range, just null out the base.
+    base_ = (size == size_) ? nullptr : reinterpret_cast<void*>(addr + size);
+  else
+    CHECK_EQ(addr + size, reinterpret_cast<uptr>(base_) + size_);
+  size_ -= size;
   UnmapOrDie(reinterpret_cast<void*>(addr), size);
-  if (addr_as_void == base_) {
-    base_ = reinterpret_cast<void*>(addr + size);
-  }
-  size_ = size_ - size;
 }
 
 void *MmapFixedNoAccess(uptr fixed_addr, uptr size, const char *name) {

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc?rev=330355&r1=330354&r2=330355&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc Thu Apr 19 11:38:15 2018
@@ -247,15 +247,12 @@ uptr ReservedAddressRange::MapOrDie(uptr
 }
 
 void ReservedAddressRange::Unmap(uptr addr, uptr size) {
-  void* addr_as_void = reinterpret_cast<void*>(addr);
-  uptr base_as_uptr = reinterpret_cast<uptr>(base_);
   // Only unmap if it covers the entire range.
-  CHECK((addr == base_as_uptr) && (size == size_));
-  UnmapOrDie(addr_as_void, size);
-  if (addr_as_void == base_) {
-    base_ = reinterpret_cast<void*>(addr + size);
-  }
-  size_ = size_ - size;
+  CHECK((addr == reinterpret_cast<uptr>(base_)) && (size == size_));
+  // We unmap the whole range, just null out the base.
+  base_ = nullptr;
+  size_ = 0;
+  UnmapOrDie(reinterpret_cast<void*>(addr), size);
 }
 
 void *MmapFixedOrDieOnFatalError(uptr fixed_addr, uptr size) {
@@ -276,11 +273,7 @@ void *MmapNoReserveOrDie(uptr size, cons
 }
 
 uptr ReservedAddressRange::Init(uptr size, const char *name, uptr fixed_addr) {
-  if (fixed_addr) {
-    base_ = MmapFixedNoAccess(fixed_addr, size, name);
-  } else {
-    base_ = MmapNoAccess(size);
-  }
+  base_ = fixed_addr ? MmapFixedNoAccess(fixed_addr, size) : MmapNoAccess(size);
   size_ = size;
   name_ = name;
   (void)os_handle_;  // unsupported