Closed Bug 704045 Opened 13 years ago Closed 4 years ago

Clarify how moz_malloc_usable_size works on Mac

Categories

(Core :: Memory Allocator, defect)

All
macOS
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: n.nethercote, Unassigned)

Details

This is spun off from 414946 comment 198. moz_malloc_usable_size looks like this: size_t moz_malloc_usable_size(void *ptr) { if (!ptr) return 0; #if defined(XP_MACOSX) return malloc_size(ptr); #elif defined(MOZ_MEMORY) return malloc_usable_size(ptr); #elif defined(XP_WIN) return _msize(ptr); #else return 0; #endif } That looks like it shouldn't work on Mac when jemalloc is enabled. However it does because jemalloc's szone2ozone function does this: default_zone->size = (void *)ozone_size; And ozone_size looks like this: static size_t ozone_size(malloc_zone_t *zone, void *ptr) { size_t ret = isalloc_validate(ptr); if (ret == 0) ret = szone->size(zone, ptr); return ret; } Amusingly enough, jemalloc's malloc_usable_size looks like this: #ifdef MOZ_MEMORY_ANDROID size_t malloc_usable_size(void *ptr) #else size_t malloc_usable_size(const void *ptr) #endif { DARWIN_ONLY(return (szone->size)(szone, ptr)); #ifdef MALLOC_VALIDATE return (isalloc_validate(ptr)); #else assert(ptr != NULL); return (isalloc(ptr)); #endif } But jlebar says: > The mac jemalloc code claims that some objects are allocated with the mac > default allocator before jemalloc loads up. If we called > malloc_usable_size on one of them, jemalloc would likely crash. Yuk. It seems like we should add a comment to moz_malloc_usable_size explaining the ordering, and maybe assert if we call jemalloc's malloc_usable_size on Mac?
> Amusingly enough, jemalloc's malloc_usable_size looks like this: > > #ifdef MOZ_MEMORY_ANDROID > size_t > malloc_usable_size(void *ptr) > #else > size_t > malloc_usable_size(const void *ptr) > #endif > { > DARWIN_ONLY(return (szone->size)(szone, ptr)); It's a trap! DARWIN_ONLY actually means "only if jemalloc is disabled." That macro could be renamed to something less misleading...
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.