[PATCH] D54879: Introduce `LocalAddressSpaceView::LoadWritable(...)` and make the `Load(...)` method return a const pointer.

[Dan Liew via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL350136: Introduce `LocalAddressSpaceView::LoadWritable(...)` and make the `Load(...)`… (authored by delcypher, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54879?vs=178202&id=179650#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54879/new/

https://reviews.llvm.org/D54879

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


Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h
===================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_allocator_secondary.h
@@ -204,10 +204,10 @@
 
   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 @@
   // 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 @@
     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_);
   }
Index: compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h
===================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_local_address_space_view.h
@@ -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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54879.179650.patch
Type: text/x-patch
Size: 4696 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181228/fc53dcf8/attachment.bin>