From 8bf8303f84623a9346997d71d828a2e843c685d3 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Tue, 4 Jun 2019 01:10:37 +0200 Subject: [PATCH] Simplify mc::Region::read() - Inline a function, and simplify the flow now that we only have Chunked regions - Make it a method (it was a C function) --- src/mc/compare.cpp | 30 +++++++------- src/mc/sosp/Region.cpp | 78 +++++++++++++++++++++++++++++++++++ src/mc/sosp/Region.hpp | 18 ++++++++ src/mc/sosp/Snapshot.cpp | 75 +-------------------------------- src/mc/sosp/Snapshot.hpp | 49 ---------------------- src/mc/sosp/Snapshot_test.cpp | 4 +- 6 files changed, 115 insertions(+), 139 deletions(-) diff --git a/src/mc/compare.cpp b/src/mc/compare.cpp index dda78ad75d..e4eb505635 100644 --- a/src/mc/compare.cpp +++ b/src/mc/compare.cpp @@ -308,8 +308,10 @@ int mmalloc_compare_heap( while (i1 < state.heaplimit) { - const malloc_info* heapinfo1 = (const malloc_info*) MC_region_read(heap_region1, &heapinfo_temp1, &heapinfos1[i1], sizeof(malloc_info)); - const malloc_info* heapinfo2 = (const malloc_info*) MC_region_read(heap_region2, &heapinfo_temp2, &heapinfos2[i1], sizeof(malloc_info)); + const malloc_info* heapinfo1 = + (const malloc_info*)heap_region1->read(&heapinfo_temp1, &heapinfos1[i1], sizeof(malloc_info)); + const malloc_info* heapinfo2 = + (const malloc_info*)heap_region2->read(&heapinfo_temp2, &heapinfos2[i1], sizeof(malloc_info)); if (heapinfo1->type == MMALLOC_TYPE_FREE || heapinfo1->type == MMALLOC_TYPE_HEAPINFO) { /* Free block */ i1 ++; @@ -365,7 +367,8 @@ int mmalloc_compare_heap( continue; } - const malloc_info* heapinfo2b = (const malloc_info*) MC_region_read(heap_region2, &heapinfo_temp2b, &heapinfos2[i2], sizeof(malloc_info)); + const malloc_info* heapinfo2b = + (const malloc_info*)heap_region2->read(&heapinfo_temp2b, &heapinfos2[i2], sizeof(malloc_info)); if (heapinfo2b->type != MMALLOC_TYPE_UNFRAGMENTED) { i2++; @@ -423,9 +426,8 @@ int mmalloc_compare_heap( while (i2 < state.heaplimit && not equal) { - const malloc_info* heapinfo2b = (const malloc_info*) MC_region_read( - heap_region2, &heapinfo_temp2b, &heapinfos2[i2], - sizeof(malloc_info)); + const malloc_info* heapinfo2b = + (const malloc_info*)heap_region2->read(&heapinfo_temp2b, &heapinfos2[i2], sizeof(malloc_info)); if (heapinfo2b->type == MMALLOC_TYPE_FREE || heapinfo2b->type == MMALLOC_TYPE_HEAPINFO) { i2 ++; @@ -480,8 +482,8 @@ int mmalloc_compare_heap( /* All blocks/fragments are equal to another block/fragment_ ? */ for (size_t i = 1; i < state.heaplimit; i++) { - const malloc_info* heapinfo1 = (const malloc_info*) MC_region_read( - heap_region1, &heapinfo_temp1, &heapinfos1[i], sizeof(malloc_info)); + const malloc_info* heapinfo1 = + (const malloc_info*)heap_region1->read(&heapinfo_temp1, &heapinfos1[i], sizeof(malloc_info)); if (heapinfo1->type == MMALLOC_TYPE_UNFRAGMENTED && i1 == state.heaplimit && heapinfo1->busy_block.busy_size > 0 && not state.equals_to1_(i, 0).valid_) { @@ -502,8 +504,8 @@ int mmalloc_compare_heap( XBT_DEBUG("Number of blocks/fragments not found in heap1: %d", nb_diff1); for (size_t i = 1; i < state.heaplimit; i++) { - const malloc_info* heapinfo2 = (const malloc_info*) MC_region_read( - heap_region2, &heapinfo_temp2, &heapinfos2[i], sizeof(malloc_info)); + const malloc_info* heapinfo2 = + (const malloc_info*)heap_region2->read(&heapinfo_temp2, &heapinfos2[i], sizeof(malloc_info)); if (heapinfo2->type == MMALLOC_TYPE_UNFRAGMENTED && i1 == state.heaplimit && heapinfo2->busy_block.busy_size > 0 && not state.equals_to2_(i, 0).valid_) { XBT_DEBUG("Block %zu not found (size used = %zu)", i, @@ -938,10 +940,10 @@ static int compare_heap_area(simgrid::mc::StateComparator& state, const void* ar simgrid::mc::Region* heap_region1 = MC_get_heap_region(snapshot1); simgrid::mc::Region* heap_region2 = MC_get_heap_region(snapshot2); - const malloc_info* heapinfo1 = (const malloc_info*) MC_region_read( - heap_region1, &heapinfo_temp1, &heapinfos1[block1], sizeof(malloc_info)); - const malloc_info* heapinfo2 = (const malloc_info*) MC_region_read( - heap_region2, &heapinfo_temp2, &heapinfos2[block2], sizeof(malloc_info)); + const malloc_info* heapinfo1 = + (const malloc_info*)heap_region1->read(&heapinfo_temp1, &heapinfos1[block1], sizeof(malloc_info)); + const malloc_info* heapinfo2 = + (const malloc_info*)heap_region2->read(&heapinfo_temp2, &heapinfos2[block2], sizeof(malloc_info)); if ((heapinfo1->type == MMALLOC_TYPE_FREE || heapinfo1->type==MMALLOC_TYPE_HEAPINFO) && (heapinfo2->type == MMALLOC_TYPE_FREE || heapinfo2->type ==MMALLOC_TYPE_HEAPINFO)) { diff --git a/src/mc/sosp/Region.cpp b/src/mc/sosp/Region.cpp index b7dbbca9e4..4eb5d6ffb0 100644 --- a/src/mc/sosp/Region.cpp +++ b/src/mc/sosp/Region.cpp @@ -46,5 +46,83 @@ void Region::restore() } } +static XBT_ALWAYS_INLINE void* mc_translate_address_region(uintptr_t addr, simgrid::mc::Region* region) +{ + auto split = simgrid::mc::mmu::split(addr - region->start().address()); + auto pageno = split.first; + auto offset = split.second; + const void* snapshot_page = region->get_chunks().page(pageno); + return (char*)snapshot_page + offset; +} + +const void* Region::read(void* target, const void* addr, std::size_t size) +{ + xbt_assert(contain(simgrid::mc::remote(addr)), "Trying to read out of the region boundary."); + + // Last byte of the region: + void* end = (char*)addr + size - 1; + if (simgrid::mc::mmu::same_chunk((std::uintptr_t)addr, (std::uintptr_t)end)) { + // The memory is contained in a single page: + return mc_translate_address_region((uintptr_t)addr, this); + } + // Otherwise, the memory spans several pages. Let's copy it all into the provided buffer + xbt_assert(target != nullptr, "Missing destination buffer for fragmented memory access"); + + // TODO, we assume the chunks are aligned to natural chunk boundaries. + // We should remove this assumption. + + // Page of the last byte of the memory area: + size_t page_end = simgrid::mc::mmu::split((std::uintptr_t)end).first; + + void* dest = target; // iterator in the buffer to where we should copy next + + // Read each page: + while (simgrid::mc::mmu::split((std::uintptr_t)addr).first != page_end) { + void* snapshot_addr = mc_translate_address_region((uintptr_t)addr, this); + void* next_page = (void*)simgrid::mc::mmu::join(simgrid::mc::mmu::split((std::uintptr_t)addr).first + 1, 0); + size_t readable = (char*)next_page - (char*)addr; + memcpy(dest, snapshot_addr, readable); + addr = (char*)addr + readable; + dest = (char*)dest + readable; + size -= readable; + } + + // Read the end: + void* snapshot_addr = mc_translate_address_region((uintptr_t)addr, this); + memcpy(dest, snapshot_addr, size); + + return target; +} + } // namespace mc } // namespace simgrid + +/** Compare memory between snapshots (with known regions) + * + * @param addr1 Address in the first snapshot + * @param region1 Region of the address in the first snapshot + * @param addr2 Address in the second snapshot + * @param region2 Region of the address in the second snapshot + * @return same semantic as memcmp + */ +int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2, + simgrid::mc::Region* region2, size_t size) +{ + // Using alloca() for large allocations may trigger stack overflow: + // use malloc if the buffer is too big. + bool stack_alloc = size < 64; + void* buffer1a = stack_alloc ? alloca(size) : ::operator new(size); + void* buffer2a = stack_alloc ? alloca(size) : ::operator new(size); + const void* buffer1 = region1->read(buffer1a, addr1, size); + const void* buffer2 = region2->read(buffer2a, addr2, size); + int res; + if (buffer1 == buffer2) + res = 0; + else + res = memcmp(buffer1, buffer2, size); + if (not stack_alloc) { + ::operator delete(buffer1a); + ::operator delete(buffer2a); + } + return res; +} diff --git a/src/mc/sosp/Region.hpp b/src/mc/sosp/Region.hpp index 3079060f1b..2ee7e5272a 100644 --- a/src/mc/sosp/Region.hpp +++ b/src/mc/sosp/Region.hpp @@ -61,9 +61,27 @@ public: /** @brief Restore a region from a snapshot */ void restore(); + + /** @brief Read memory that was snapshoted in this region + * + * @param target Buffer to store contiguously the value if it spans over several pages + * @param addr Process (non-snapshot) address of the data + * @param size Size of the data to read in bytes + * @return Pointer where the data is located (either target buffer or original location) + */ + const void* read(void* target, const void* addr, std::size_t size); }; } // namespace mc } // namespace simgrid +int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2, + simgrid::mc::Region* region2, std::size_t size); + +static XBT_ALWAYS_INLINE void* MC_region_read_pointer(simgrid::mc::Region* region, const void* addr) +{ + void* res; + return *(void**)region->read(&res, addr, sizeof(void*)); +} + #endif diff --git a/src/mc/sosp/Snapshot.cpp b/src/mc/sosp/Snapshot.cpp index 6a2716ef6b..bc72a2b751 100644 --- a/src/mc/sosp/Snapshot.cpp +++ b/src/mc/sosp/Snapshot.cpp @@ -11,79 +11,6 @@ #include /* std::size_t */ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(mc_snapshot, mc, "Taking and restoring snapshots"); - -/** @brief Read memory from a snapshot region broken across fragmented pages - * - * @param addr Process (non-snapshot) address of the data - * @param region Snapshot memory region where the data is located - * @param target Buffer to store the value - * @param size Size of the data to read in bytes - * @return Pointer where the data is located (target buffer of original location) - */ -const void* MC_region_read_fragmented(simgrid::mc::Region* region, void* target, const void* addr, size_t size) -{ - // Last byte of the memory area: - void* end = (char*)addr + size - 1; - - // TODO, we assume the chunks are aligned to natural chunk boundaries. - // We should remove this assumption. - - // Page of the last byte of the memory area: - size_t page_end = simgrid::mc::mmu::split((std::uintptr_t)end).first; - - void* dest = target; - - if (dest == nullptr) - xbt_die("Missing destination buffer for fragmented memory access"); - - // Read each page: - while (simgrid::mc::mmu::split((std::uintptr_t)addr).first != page_end) { - void* snapshot_addr = mc_translate_address_region((uintptr_t)addr, region); - void* next_page = (void*)simgrid::mc::mmu::join(simgrid::mc::mmu::split((std::uintptr_t)addr).first + 1, 0); - size_t readable = (char*)next_page - (char*)addr; - memcpy(dest, snapshot_addr, readable); - addr = (char*)addr + readable; - dest = (char*)dest + readable; - size -= readable; - } - - // Read the end: - void* snapshot_addr = mc_translate_address_region((uintptr_t)addr, region); - memcpy(dest, snapshot_addr, size); - - return target; -} - -/** Compare memory between snapshots (with known regions) - * - * @param addr1 Address in the first snapshot - * @param region1 Region of the address in the first snapshot - * @param addr2 Address in the second snapshot - * @param region2 Region of the address in the second snapshot - * @return same semantic as memcmp - */ -int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2, - simgrid::mc::Region* region2, size_t size) -{ - // Using alloca() for large allocations may trigger stack overflow: - // use malloc if the buffer is too big. - bool stack_alloc = size < 64; - void* buffer1a = stack_alloc ? alloca(size) : ::operator new(size); - void* buffer2a = stack_alloc ? alloca(size) : ::operator new(size); - const void* buffer1 = MC_region_read(region1, buffer1a, addr1, size); - const void* buffer2 = MC_region_read(region2, buffer2a, addr2, size); - int res; - if (buffer1 == buffer2) - res = 0; - else - res = memcmp(buffer1, buffer2, size); - if (not stack_alloc) { - ::operator delete(buffer1a); - ::operator delete(buffer2a); - } - return res; -} - namespace simgrid { namespace mc { /************************************* Take Snapshot ************************************/ @@ -313,7 +240,7 @@ const void* Snapshot::read_bytes(void* buffer, std::size_t size, RemotePtr { Region* region = this->get_region((void*)address.address()); if (region) { - const void* res = MC_region_read(region, buffer, (void*)address.address(), size); + const void* res = region->read(buffer, (void*)address.address(), size); if (buffer == res || options & ReadOptions::lazy()) return res; else { diff --git a/src/mc/sosp/Snapshot.hpp b/src/mc/sosp/Snapshot.hpp index 4bdb153840..a91f3ec349 100644 --- a/src/mc/sosp/Snapshot.hpp +++ b/src/mc/sosp/Snapshot.hpp @@ -11,17 +11,6 @@ #include "src/mc/remote/RemoteClient.hpp" #include "src/mc/sosp/Region.hpp" -// ***** Snapshot region - -static XBT_ALWAYS_INLINE void* mc_translate_address_region(uintptr_t addr, simgrid::mc::Region* region) -{ - auto split = simgrid::mc::mmu::split(addr - region->start().address()); - auto pageno = split.first; - auto offset = split.second; - const void* snapshot_page = region->get_chunks().page(pageno); - return (char*)snapshot_page + offset; -} - // ***** MC Snapshot /** Ignored data @@ -99,13 +88,6 @@ private: } // namespace mc } // namespace simgrid -static const void* mc_snapshot_get_heap_end(simgrid::mc::Snapshot* snapshot); - -const void* MC_region_read_fragmented(simgrid::mc::Region* region, void* target, const void* addr, std::size_t size); - -int MC_snapshot_region_memcmp(const void* addr1, simgrid::mc::Region* region1, const void* addr2, - simgrid::mc::Region* region2, std::size_t size); - static XBT_ALWAYS_INLINE const void* mc_snapshot_get_heap_end(simgrid::mc::Snapshot* snapshot) { if (snapshot == nullptr) @@ -113,35 +95,4 @@ static XBT_ALWAYS_INLINE const void* mc_snapshot_get_heap_end(simgrid::mc::Snaps return mc_model_checker->process().get_heap()->breakval; } -/** @brief Read memory from a snapshot region - * - * @param addr Process (non-snapshot) address of the data - * @param region Snapshot memory region where the data is located - * @param target Buffer to store the value - * @param size Size of the data to read in bytes - * @return Pointer where the data is located (target buffer of original location) - */ -static XBT_ALWAYS_INLINE const void* MC_region_read(simgrid::mc::Region* region, void* target, const void* addr, - std::size_t size) -{ - xbt_assert(region); - - xbt_assert(region->contain(simgrid::mc::remote(addr)), "Trying to read out of the region boundary."); - - // Last byte of the region: - void* end = (char*)addr + size - 1; - if (simgrid::mc::mmu::same_chunk((std::uintptr_t)addr, (std::uintptr_t)end)) { - // The memory is contained in a single page: - return mc_translate_address_region((uintptr_t)addr, region); - } - // Otherwise, the memory spans several pages: - return MC_region_read_fragmented(region, target, addr, size); -} - -static XBT_ALWAYS_INLINE void* MC_region_read_pointer(simgrid::mc::Region* region, const void* addr) -{ - void* res; - return *(void**)MC_region_read(region, &res, addr, sizeof(void*)); -} - #endif diff --git a/src/mc/sosp/Snapshot_test.cpp b/src/mc/sosp/Snapshot_test.cpp index 4b30fc94ce..cadb7bdf43 100644 --- a/src/mc/sosp/Snapshot_test.cpp +++ b/src/mc/sosp/Snapshot_test.cpp @@ -95,7 +95,7 @@ void snap_test_helper::read_whole_region() for (int n = 1; n != 32; ++n) { prologue_return ret = prologue(n); - const void* read = MC_region_read(ret.region, ret.dstn, ret.src, ret.size); + const void* read = ret.region->read(ret.dstn, ret.src, ret.size); INFO("Mismatch in MC_region_read()"); REQUIRE(not memcmp(ret.src, read, ret.size)); @@ -115,7 +115,7 @@ void snap_test_helper::read_region_parts() for (int j = 0; j != 100; ++j) { size_t offset = rnd_engine() % ret.size; size_t size = rnd_engine() % (ret.size - offset); - const void* read = MC_region_read(ret.region, ret.dstn, (const char*)ret.src + offset, size); + const void* read = ret.region->read(ret.dstn, (const char*)ret.src + offset, size); INFO("Mismatch in MC_region_read()"); REQUIRE(not memcmp((char*)ret.src + offset, read, size)); } -- 2.20.1