From 5bdee65f7a63f18c6145a92963f0f044165d4d7f Mon Sep 17 00:00:00 2001 From: "ivan.penkov@gmail.com" Date: Thu, 23 May 2013 18:47:49 +0000 Subject: Thanks to Matthew Riley who noticed this issue and provided the initial proposal for the fix. There's a bug in the new allocator implementation used by wasteful_vector. It inherits the base class' implementation of allocator and doesn't implement allocate() so it goes to the heap instead of the PageAllocator -- the very thing wasteful_vector was trying to avoid! As a side effect it was also leaking heap memory. Thanks, -Ivan Review URL: https://breakpad.appspot.com/599002 git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@1188 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/common/memory.h | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) (limited to 'src/common/memory.h') diff --git a/src/common/memory.h b/src/common/memory.h index cb37b5fa..949bd8b6 100644 --- a/src/common/memory.h +++ b/src/common/memory.h @@ -30,13 +30,14 @@ #ifndef GOOGLE_BREAKPAD_COMMON_MEMORY_H_ #define GOOGLE_BREAKPAD_COMMON_MEMORY_H_ -#include -#include #include #include #include #include +#include +#include + #ifdef __APPLE__ #define sys_mmap mmap #define sys_mmap2 mmap @@ -87,12 +88,27 @@ class PageAllocator { if (!ret) return NULL; - page_offset_ = (page_size_ - (page_size_ * pages - (bytes + sizeof(PageHeader)))) % page_size_; + page_offset_ = + (page_size_ - (page_size_ * pages - (bytes + sizeof(PageHeader)))) % + page_size_; current_page_ = page_offset_ ? ret + page_size_ * (pages - 1) : NULL; return ret + sizeof(PageHeader); } + // Checks whether the page allocator owns the passed-in pointer. + // This method exists for testing pursposes only. + bool OwnsPointer(const void* p) { + PageHeader* header = last_; + for (header = last_; header; header = header->next) { + const char* current = reinterpret_cast(header); + if ((p >= current) && (p < current + header->num_pages * page_size_)) + return true; + } + + return false; + } + private: uint8_t *GetNPages(unsigned num_pages) { #ifdef __x86_64 @@ -135,16 +151,16 @@ class PageAllocator { // Wrapper to use with STL containers template -struct PageStdAllocator: public std::allocator { +struct PageStdAllocator : public std::allocator { typedef typename std::allocator::pointer pointer; typedef typename std::allocator::size_type size_type; - PageStdAllocator(PageAllocator& allocator): allocator_(allocator) {} - template PageStdAllocator(const PageStdAllocator& other): - allocator_(other.allocator_) {} + explicit PageStdAllocator(PageAllocator& allocator): allocator_(allocator) {} + template PageStdAllocator(const PageStdAllocator& other) + : allocator_(other.allocator_) {} - inline pointer allocator(size_type n, const void* = 0) { - return allocator_.Alloc(sizeof(T) * n); + inline pointer allocate(size_type n, const void* = 0) { + return static_cast(allocator_.Alloc(sizeof(T) * n)); } inline void deallocate(pointer, size_type) { @@ -155,7 +171,7 @@ struct PageStdAllocator: public std::allocator { typedef PageStdAllocator other; }; -private: + private: PageAllocator& allocator_; }; @@ -163,7 +179,7 @@ private: // PageAllocator. It's wasteful because, when resizing, it always allocates a // whole new array since the PageAllocator doesn't support realloc. template -class wasteful_vector: public std::vector > { +class wasteful_vector : public std::vector > { public: wasteful_vector(PageAllocator* allocator, unsigned size_hint = 16) : std::vector >(PageStdAllocator(*allocator)) { @@ -175,7 +191,7 @@ class wasteful_vector: public std::vector > { inline void* operator new(size_t nbytes, google_breakpad::PageAllocator& allocator) { - return allocator.Alloc(nbytes); + return allocator.Alloc(nbytes); } #endif // GOOGLE_BREAKPAD_COMMON_MEMORY_H_ -- cgit v1.2.1