[compiler-rt] r350136 - Introduce `LocalAddressSpaceView::LoadWritable(...)` and make the `Load(...)` method return a const pointer.

[Dan Liew via llvm-commits]

Author: delcypher
Date: Fri Dec 28 11:30:51 2018
New Revision: 350136

URL: http://llvm.org/viewvc/llvm-project?rev=350136&view=rev
Log:
Introduce `LocalAddressSpaceView::LoadWritable(...)` and make the `Load(...)` method return a const pointer.

Summary:
This is a follow-up to r346956 (https://reviews.llvm.org/D53975).

The purpose of this change to allow implementers of the
`AddressSpaceView` to be able to distinguish between when a caller wants
read-only memory and when a caller wants writable memory. Being able
distinguish these cases allows implementations to optimize for the
different cases and also provides a way to workaround possible platform
restrictions (e.g. the low level platform interface for reading
out-of-process memory may place memory in read-only pages).

For allocator enumeration in almost all cases read-only is sufficient so
we make `Load(...)` take on this new requirement and introduce the
`LoadWritable(...)` variants for cases where memory needs to be
writable.

The behaviour of `LoadWritable(...)` documented in comments are
deliberately very restrictive so that it will be possible in the future
to implement a simple write-cache (i.e. just a map from target address
to a writable region of memory). These restrictions can be loosened in
the future if necessary by implementing a more sophisticated
write-cache.

rdar://problem/45284065

Reviewers: kcc, cryptoad, eugenis, kubamracek, george.karpenkov

Subscribers: #sanitizers, llvm-commits

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

Modified:
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h
    compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h?rev=350136&r1=350135&r2=350136&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h Fri Dec 28 11:30:51 2018
@@ -204,10 +204,10 @@ class LargeMmapAllocator {
 
   void EnsureSortedChunks() {
     if (chunks_sorted_) return;
-    Header **chunks = AddressSpaceView::Load(chunks_, n_chunks_);
+    Header **chunks = AddressSpaceView::LoadWritable(chunks_, n_chunks_);
     Sort(reinterpret_cast<uptr *>(chunks), n_chunks_);
     for (uptr i = 0; i < n_chunks_; i++)
-      AddressSpaceView::Load(chunks[i])->chunk_idx = i;
+      AddressSpaceView::LoadWritable(chunks[i])->chunk_idx = i;
     chunks_sorted_ = true;
   }
 
@@ -275,9 +275,9 @@ class LargeMmapAllocator {
   // The allocator must be locked when calling this function.
   void ForEachChunk(ForEachChunkCallback callback, void *arg) {
     EnsureSortedChunks();  // Avoid doing the sort while iterating.
-    Header **chunks = AddressSpaceView::Load(chunks_, n_chunks_);
+    const Header *const *chunks = AddressSpaceView::Load(chunks_, n_chunks_);
     for (uptr i = 0; i < n_chunks_; i++) {
-      Header *t = chunks[i];
+      const Header *t = chunks[i];
       callback(reinterpret_cast<uptr>(GetUser(t)), arg);
       // Consistency check: verify that the array did not change.
       CHECK_EQ(chunks[i], t);
@@ -301,7 +301,7 @@ class LargeMmapAllocator {
     return GetHeader(reinterpret_cast<uptr>(p));
   }
 
-  void *GetUser(Header *h) {
+  void *GetUser(const Header *h) {
     CHECK(IsAligned((uptr)h, page_size_));
     return reinterpret_cast<void*>(reinterpret_cast<uptr>(h) + page_size_);
   }

Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h?rev=350136&r1=350135&r2=350136&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h Fri Dec 28 11:30:51 2018
@@ -31,18 +31,42 @@
 
 namespace __sanitizer {
 struct LocalAddressSpaceView {
-  // Load memory `sizeof(T) * num_elements` bytes of memory
-  // from the target process (always local for this implementation)
-  // starting at address `target_address`. The local copy of
-  // this memory is returned as a pointer. It is guaranteed that
+  // Load memory `sizeof(T) * num_elements` bytes of memory from the target
+  // process (always local for this implementation) starting at address
+  // `target_address`. The local copy of this memory is returned as a pointer.
+  // The caller should not write to this memory. The behaviour when doing so is
+  // undefined. Callers should use `LoadWritable()` to get access to memory
+  // that is writable.
   //
-  // * That the function will always return the same value
-  //   for a given set of arguments.
-  // * That the memory returned is writable and that writes will persist.
+  // The lifetime of loaded memory is implementation defined.
+  template <typename T>
+  static const T *Load(const T *target_address, uptr num_elements = 1) {
+    // The target address space is the local address space so
+    // nothing needs to be copied. Just return the pointer.
+    return target_address;
+  }
+
+  // Load memory `sizeof(T) * num_elements` bytes of memory from the target
+  // process (always local for this implementation) starting at address
+  // `target_address`. The local copy of this memory is returned as a pointer.
+  // The memory returned may be written to.
+  //
+  // Writes made to the returned memory will be visible in the memory returned
+  // by subsequent `Load()` or `LoadWritable()` calls provided the
+  // `target_address` parameter is the same. It is not guaranteed that the
+  // memory returned by previous calls to `Load()` will contain any performed
+  // writes.  If two or more overlapping regions of memory are loaded via
+  // separate calls to `LoadWritable()`, it is implementation defined whether
+  // writes made to the region returned by one call are visible in the regions
+  // returned by other calls.
+  //
+  // Given the above it is recommended to load the largest possible object
+  // that requires modification (e.g. a class) rather than individual fields
+  // from a class to avoid issues with overlapping writable regions.
   //
   // The lifetime of loaded memory is implementation defined.
   template <typename T>
-  static T *Load(T *target_address, uptr num_elements = 1) {
+  static T *LoadWritable(T *target_address, uptr num_elements = 1) {
     // The target address space is the local address space so
     // nothing needs to be copied. Just return the pointer.
     return target_address;