From f41b9946e299ba7c2c779c25ddb16dea6ffecdc8 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 16 Nov 2020 09:33:30 +0100 Subject: [PATCH] UserspaceEmulator: Hang malloc metadata on malloc block MmapRegions Instead of tracking known malloc blocks in a separate hash table, add an optional malloc metadata pointer to MmapRegion. This makes finding the malloc metadata for a given pointer extremely fast since it can piggyback on the page table array. :^) --- DevTools/UserspaceEmulator/MallocTracer.cpp | 79 +++++++++++---------- DevTools/UserspaceEmulator/MallocTracer.h | 68 +++++++----------- DevTools/UserspaceEmulator/MmapRegion.cpp | 5 -- DevTools/UserspaceEmulator/MmapRegion.h | 8 +++ 4 files changed, 77 insertions(+), 83 deletions(-) diff --git a/DevTools/UserspaceEmulator/MallocTracer.cpp b/DevTools/UserspaceEmulator/MallocTracer.cpp index 9bc7aa3491..7e4676142a 100644 --- a/DevTools/UserspaceEmulator/MallocTracer.cpp +++ b/DevTools/UserspaceEmulator/MallocTracer.cpp @@ -40,12 +40,23 @@ MallocTracer::MallocTracer() { } -void MallocTracer::notify_malloc_block_was_released(Badge, MmapRegion& region) +template +inline void MallocTracer::for_each_mallocation(Callback callback) const { - // FIXME: It's sad that we may lose a bunch of free() backtraces here, - // but if the address is reused for a new ChunkedBlock, things will - // get extremely confused. - m_chunked_blocks.remove(region.base()); + Emulator::the().mmu().for_each_region([&](auto& region) { + if (region.is_mmap() && static_cast(region).is_malloc_block()) { + auto* malloc_data = static_cast(region).malloc_metadata(); + for (auto& mallocation : malloc_data->mallocations) { + if (mallocation.used && callback(mallocation) == IterationDecision::Break) + return IterationDecision::Break; + } + } + return IterationDecision::Continue; + }); + for (auto& big_mallocation : m_big_mallocations) { + if (callback(big_mallocation) == IterationDecision::Break) + return; + } } void MallocTracer::target_did_malloc(Badge, FlatPtr address, size_t size) @@ -70,35 +81,30 @@ void MallocTracer::target_did_malloc(Badge, FlatPtr address, size_t siz return; } - if (size <= size_classes[num_size_classes - 1]) { - FlatPtr chunked_block_address = address & ChunkedBlock::block_mask; - TrackedChunkedBlock* block = nullptr; - auto tracked_chunked_block = m_chunked_blocks.get(chunked_block_address); - if (!tracked_chunked_block.has_value()) { - auto new_block = make(); - block = new_block.ptr(); - m_chunked_blocks.set(chunked_block_address, move(new_block)); - tracked_chunked_block = m_chunked_blocks.get(chunked_block_address); - auto& block = const_cast(*tracked_chunked_block.value()); - block.address = chunked_block_address; - block.chunk_size = mmap_region.read32(offsetof(CommonHeader, m_size)).value(); - block.mallocations.resize((ChunkedBlock::block_size - sizeof(ChunkedBlock)) / block.chunk_size); - dbgln("Tracking ChunkedBlock @ {:p} with chunk_size={}, chunk_count={}", block.address, block.chunk_size, block.mallocations.size()); - } else { - block = const_cast(tracked_chunked_block.value()); + bool is_chunked_allocation = size <= size_classes[num_size_classes - 1]; + if (is_chunked_allocation) { + MallocRegionMetadata* malloc_data = static_cast(*region).malloc_metadata(); + if (!malloc_data) { + auto new_malloc_data = make(); + malloc_data = new_malloc_data.ptr(); + static_cast(*region).set_malloc_metadata({}, move(new_malloc_data)); + malloc_data->address = region->base(); + malloc_data->chunk_size = mmap_region.read32(offsetof(CommonHeader, m_size)).value(); + malloc_data->mallocations.resize((ChunkedBlock::block_size - sizeof(ChunkedBlock)) / malloc_data->chunk_size); + dbgln("Tracking ChunkedBlock @ {:p} with chunk_size={}, chunk_count={}", malloc_data->address, malloc_data->chunk_size, malloc_data->mallocations.size()); } - block->mallocation_for_address(address) = { address, size, true, false, Emulator::the().raw_backtrace(), Vector() }; + malloc_data->mallocation_for_address(address) = { address, size, true, false, Emulator::the().raw_backtrace(), Vector() }; } else { m_big_mallocations.append({ address, size, true, false, Emulator::the().raw_backtrace(), Vector() }); } } -ALWAYS_INLINE MallocTracer::Mallocation& MallocTracer::TrackedChunkedBlock::mallocation_for_address(FlatPtr address) const +ALWAYS_INLINE Mallocation& MallocRegionMetadata::mallocation_for_address(FlatPtr address) const { return const_cast(this->mallocations[chunk_index_for_address(address)]); } -ALWAYS_INLINE size_t MallocTracer::TrackedChunkedBlock::chunk_index_for_address(FlatPtr address) const +ALWAYS_INLINE size_t MallocRegionMetadata::chunk_index_for_address(FlatPtr address) const { auto chunk_offset = address - (this->address + sizeof(ChunkedBlock)); return chunk_offset / this->chunk_size; @@ -154,19 +160,18 @@ void MallocTracer::target_did_realloc(Badge, FlatPtr address, size_t si existing_mallocation->malloc_backtrace = Emulator::the().raw_backtrace(); } -MallocTracer::Mallocation* MallocTracer::find_mallocation(FlatPtr address) +Mallocation* MallocTracer::find_mallocation(FlatPtr address) { - FlatPtr possible_chunked_block = address & ChunkedBlock::block_mask; - - auto chunked_block = m_chunked_blocks.get(possible_chunked_block); - if (chunked_block.has_value()) { - auto& block = *chunked_block.value(); - auto& mallocation = block.mallocation_for_address(address); - if (mallocation.used) { - ASSERT(mallocation.contains(address)); - return const_cast(&mallocation); + if (auto* region = Emulator::the().mmu().find_region({ 0x23, address })) { + if (region->is_mmap() && static_cast(*region).malloc_metadata()) { + auto& malloc_data = *static_cast(*region).malloc_metadata(); + auto& mallocation = malloc_data.mallocation_for_address(address); + if (mallocation.used) { + ASSERT(mallocation.contains(address)); + return &mallocation; + } + return nullptr; } - return nullptr; } for (auto& mallocation : m_big_mallocations) { @@ -177,7 +182,7 @@ MallocTracer::Mallocation* MallocTracer::find_mallocation(FlatPtr address) return nullptr; } -MallocTracer::Mallocation* MallocTracer::find_mallocation_before(FlatPtr address) +Mallocation* MallocTracer::find_mallocation_before(FlatPtr address) { Mallocation* found_mallocation = nullptr; for_each_mallocation([&](auto& mallocation) { @@ -190,7 +195,7 @@ MallocTracer::Mallocation* MallocTracer::find_mallocation_before(FlatPtr address return found_mallocation; } -MallocTracer::Mallocation* MallocTracer::find_mallocation_after(FlatPtr address) +Mallocation* MallocTracer::find_mallocation_after(FlatPtr address) { Mallocation* found_mallocation = nullptr; for_each_mallocation([&](auto& mallocation) { diff --git a/DevTools/UserspaceEmulator/MallocTracer.h b/DevTools/UserspaceEmulator/MallocTracer.h index 290cd58a7c..01a0f7f700 100644 --- a/DevTools/UserspaceEmulator/MallocTracer.h +++ b/DevTools/UserspaceEmulator/MallocTracer.h @@ -37,6 +37,32 @@ namespace UserspaceEmulator { class MmapRegion; class SoftCPU; +struct Mallocation { + bool contains(FlatPtr a) const + { + return a >= address && a < (address + size); + } + + FlatPtr address { 0 }; + size_t size { 0 }; + bool used { false }; + bool freed { false }; + + Vector malloc_backtrace; + Vector free_backtrace; +}; + +class MallocRegionMetadata { +public: + FlatPtr address { 0 }; + size_t chunk_size { 0 }; + + size_t chunk_index_for_address(FlatPtr) const; + Mallocation& mallocation_for_address(FlatPtr) const; + + Vector mallocations; +}; + class MallocTracer { public: MallocTracer(); @@ -45,60 +71,20 @@ public: void target_did_free(Badge, FlatPtr address); void target_did_realloc(Badge, FlatPtr address, size_t); - void notify_malloc_block_was_released(Badge, MmapRegion&); - void audit_read(FlatPtr address, size_t); void audit_write(FlatPtr address, size_t); void dump_leak_report(); private: - struct Mallocation { - bool contains(FlatPtr a) const - { - return a >= address && a < (address + size); - } - - FlatPtr address { 0 }; - size_t size { 0 }; - bool used { false }; - bool freed { false }; - - Vector malloc_backtrace; - Vector free_backtrace; - }; - - struct TrackedChunkedBlock { - FlatPtr address { 0 }; - size_t chunk_size { 0 }; - - size_t chunk_index_for_address(FlatPtr) const; - Mallocation& mallocation_for_address(FlatPtr) const; - - Vector mallocations; - }; - template - void for_each_mallocation(Callback callback) const - { - for (auto& it : m_chunked_blocks) { - for (auto& mallocation : it.value->mallocations) { - if (mallocation.used && callback(mallocation) == IterationDecision::Break) - return; - } - } - for (auto& big_mallocation : m_big_mallocations) { - if (callback(big_mallocation) == IterationDecision::Break) - return; - } - } + void for_each_mallocation(Callback callback) const; Mallocation* find_mallocation(FlatPtr); Mallocation* find_mallocation_before(FlatPtr); Mallocation* find_mallocation_after(FlatPtr); bool is_reachable(const Mallocation&) const; - HashMap> m_chunked_blocks; Vector m_big_mallocations; bool m_auditing_enabled { true }; diff --git a/DevTools/UserspaceEmulator/MmapRegion.cpp b/DevTools/UserspaceEmulator/MmapRegion.cpp index a01d837082..f0036f729c 100644 --- a/DevTools/UserspaceEmulator/MmapRegion.cpp +++ b/DevTools/UserspaceEmulator/MmapRegion.cpp @@ -58,11 +58,6 @@ MmapRegion::MmapRegion(u32 base, u32 size, int prot) MmapRegion::~MmapRegion() { - if (is_malloc_block()) { - if (auto* tracer = Emulator::the().malloc_tracer()) - tracer->notify_malloc_block_was_released({}, *this); - } - free(m_shadow_data); if (m_file_backed) munmap(m_data, size()); diff --git a/DevTools/UserspaceEmulator/MmapRegion.h b/DevTools/UserspaceEmulator/MmapRegion.h index f1e1220707..dacc2f7d9f 100644 --- a/DevTools/UserspaceEmulator/MmapRegion.h +++ b/DevTools/UserspaceEmulator/MmapRegion.h @@ -31,6 +31,9 @@ namespace UserspaceEmulator { +class MallocRegionMetadata; +class MallocTracer; + class MmapRegion final : public SoftMMU::Region { public: static NonnullOwnPtr create_anonymous(u32 base, u32 size, u32 prot); @@ -59,6 +62,9 @@ public: void set_prot(int prot) { m_prot = prot; } + MallocRegionMetadata* malloc_metadata() { return m_malloc_metadata; } + void set_malloc_metadata(Badge, NonnullOwnPtr metadata) { m_malloc_metadata = move(metadata); } + private: MmapRegion(u32 base, u32 size, int prot); virtual bool is_mmap() const override { return true; } @@ -68,6 +74,8 @@ private: int m_prot { 0 }; bool m_file_backed { false }; bool m_malloc { false }; + + OwnPtr m_malloc_metadata; }; }