unbreak MallocExtension::GetAllocatedSize() for debug allocator

Some years back we fixed memalign vs realloc bug, but we preserved
'wrong' malloc_size/GetAllocatedSize implementation for debug
allocator.

This commit refactors old code making sure we always use right
data_size and it fixes GetAllocatedSize. We update our unittest
accordingly.

Closes #738
This commit is contained in:
Aliaksey Kandratsenka 2023-07-31 16:27:33 -04:00
parent a51e08b06a
commit 909fa3e649
2 changed files with 36 additions and 17 deletions

View File

@ -486,7 +486,21 @@ class MallocBlock {
static size_t data_offset() { return OFFSETOF_MEMBER(MallocBlock, size_and_magic2_); }
size_t data_size() const { return size1_; }
size_t raw_data_size() const { return size1_; }
// Note, this allocation might actually be from memalign, so raw_ptr
// might be >= data_addr() (see FromRawPointer and do_debug_memalign
// for how it works). So in order to get data size we should be
// careful.
size_t actual_data_size(const void* raw_ptr) const {
const char* raw_begin = static_cast<const char*>(data_addr());
const char* raw_end = raw_begin + raw_data_size();
CHECK_CONDITION(raw_begin <= raw_end);
CHECK_CONDITION(raw_begin <= raw_ptr);
CHECK_CONDITION(raw_ptr <= raw_end);
return raw_end - static_cast<const char*>(raw_ptr);
}
void set_offset(int offset) { this->offset_ = offset; }
@ -1037,7 +1051,7 @@ static inline void* DebugAllocate(size_t size, int type) {
static inline void DebugDeallocate(void* ptr, int type, size_t given_size) {
MALLOC_TRACE("free",
(ptr != 0 ? MallocBlock::FromRawPointer(ptr)->data_size() : 0),
(ptr != 0 ? MallocBlock::FromRawPointer(ptr)->actual_data_size(ptr) : 0),
ptr);
if (ptr) MallocBlock::FromRawPointer(ptr)->Deallocate(type, given_size);
}
@ -1092,7 +1106,7 @@ class DebugMallocImplementation : public TCMallocImplementation {
if (p) {
RAW_CHECK(GetOwnership(p) != MallocExtension::kNotOwned,
"ptr not allocated by tcmalloc");
return MallocBlock::FromRawPointer(p)->data_size();
return MallocBlock::FromRawPointer(p)->actual_data_size(p);
}
return 0;
}
@ -1297,22 +1311,13 @@ extern "C" PERFTOOLS_DLL_DECL void* tc_realloc(void* ptr, size_t size) PERFTOOLS
// return null
if (p == NULL) return NULL;
// if ptr was allocated via memalign, then old->data_size() is not
// start of user data. So we must be careful to copy only user-data
char *old_begin = (char *)old->data_addr();
char *old_end = old_begin + old->data_size();
ssize_t old_ssize = old_end - (char *)ptr;
CHECK_CONDITION(old_ssize >= 0);
size_t old_size = (size_t)old_ssize;
CHECK_CONDITION(old_size <= old->data_size());
size_t old_size = old->actual_data_size(ptr);
memcpy(p->data_addr(), ptr, (old_size < size) ? old_size : size);
MallocHook::InvokeDeleteHook(ptr);
MallocHook::InvokeNewHook(p->data_addr(), size);
DebugDeallocate(ptr, MallocBlock::kMallocType, 0);
MALLOC_TRACE("realloc", p->data_size(), p->data_addr());
MALLOC_TRACE("realloc", p->actual_data_size(p->data_addr()), p->data_addr());
return p->data_addr();
}

View File

@ -261,7 +261,6 @@ TEST(DebugAllocationTest, CurrentlyAllocated) {
}
TEST(DebugAllocationTest, GetAllocatedSizeTest) {
#if 1
// When debug_allocation is in effect, GetAllocatedSize should return
// exactly requested size, since debug_allocation doesn't allow users
// to write more than that.
@ -272,14 +271,29 @@ TEST(DebugAllocationTest, GetAllocatedSizeTest) {
void *p = noopt(malloc(i));
EXPECT_EQ(i, MallocExtension::instance()->GetAllocatedSize(p));
free(p);
p = tc_memalign(16, i);
auto amount = MallocExtension::instance()->GetAllocatedSize(p);
EXPECT_LE(i, amount);
memset(p, 0xff, amount);
tc_free(p);
}
#endif
void* a = noopt(malloc(1000));
EXPECT_GE(MallocExtension::instance()->GetAllocatedSize(a), 1000);
// This is just a sanity check. If we allocated too much, alloc is broken
EXPECT_LE(MallocExtension::instance()->GetAllocatedSize(a), 5000);
EXPECT_GE(MallocExtension::instance()->GetEstimatedAllocatedSize(1000), 1000);
free(a);
EXPECT_GE(MallocExtension::instance()->GetEstimatedAllocatedSize(1000), 1000);
a = tc_memalign(16, 1000);
auto amount = MallocExtension::instance()->GetAllocatedSize(a);
EXPECT_GE(amount, 1000);
// This is just a sanity check. If we allocated too much, alloc is broken
EXPECT_LE(amount, 5000);
memset(a, 0xff, amount);
tc_free(a);
}
TEST(DebugAllocationTest, HugeAlloc) {