From 58e92dce7b74cb42178ca4516e82c3723b3a6cb8 Mon Sep 17 00:00:00 2001 From: Dan Liew Date: Fri, 28 Dec 2018 19:30:51 +0000 Subject: 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 --- .../sanitizer_allocator_secondary.h | 10 +++--- .../sanitizer_local_address_space_view.h | 40 +++++++++++++++++----- 2 files changed, 37 insertions(+), 13 deletions(-) (limited to 'compiler-rt') diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_secondary.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_secondary.h index e628a796471..0c8505c34c8 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_secondary.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_secondary.h @@ -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(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(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(p)); } - void *GetUser(Header *h) { + void *GetUser(const Header *h) { CHECK(IsAligned((uptr)h, page_size_)); return reinterpret_cast(reinterpret_cast(h) + page_size_); } diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_local_address_space_view.h b/compiler-rt/lib/sanitizer_common/sanitizer_local_address_space_view.h index f9507c701e2..ec1847abc53 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_local_address_space_view.h +++ b/compiler-rt/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 + 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 - 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; -- cgit v1.2.3