Closed Bug 1389305 Opened 7 years ago Closed 7 years ago

mozjemalloc function to convert an interior pointer to a base pointer

Categories

(Core :: Memory Allocator, enhancement)

Product:
Core
Component:
Memory Allocator
Version:
54 Branch
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: n.nethercote, Assigned: n.nethercote)

References

Details

Attachments

(1 file, 3 obsolete files)

Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of()
25.88 KB, patch
glandium
: review+
Details | Diff | Splinter Review 
There have now been two cases in Stylo memory reporting where we can easily obtain an interior pointer to a heap block, but it's difficult (bug 1387956) or impossible (bug 1387958) to obtain a base pointer.

One way to address this would be to make mozjemalloc's malloc_usable_size() handle interior pointers. But DMD cannot handle interior pointers.

So a better option is for mozjemalloc to provide a interior-to-base pointer conversion function. We can then call malloc_size_of() on that pointer, and DMD will handle it.
Bug 1300900 has a patch that can serve as a starting point for this.
This patch isn't ready to land. The "njn" comments indicate incomplete things:

- huge allocations aren't handled properly;

- the test needs improvement.

But I might as well get feedback now if there's a chance that this whole
approach will be rejected.
Attachment #8898115 - Flags: feedback?(mh+mozilla)
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Comment on attachment 8898115 [details] [diff] [review]
Add jemalloc_to_base_pointer, a function to convert an interior pointer to a base pointer

Review of attachment 8898115 [details] [diff] [review]:
-----------------------------------------------------------------

Not much to say, this is essentially a conversion of the gdb script in bug 1300900 :)

::: memory/build/mozmemory.h
@@ +92,5 @@
> + * Take a pointer anywhere within a live allocation and return a pointer to the
> + * start of the allocation. May do unpredictable things, including crashing, if
> + * given a pointer that does not lie within a live allocation.
> + */
> +MOZ_JEMALLOC_API const void* jemalloc_to_base_pointer(const void* ptr);

maybe jemalloc_base_of()?

::: memory/gtest/TestJemalloc.cpp
@@ +64,5 @@
> +    }
> +  }
> +
> +  // Similar for large (4KiB--1MiB) allocations.
> +  // njn: need the -8 K; hits the huge case slightly before 1 MiB.

you might as well say "large (4KiB..1MiB-8KiB)"

::: memory/mozjemalloc/mozjemalloc.cpp
@@ +3577,5 @@
> +
> +        // Information about the arena_run_t in that page.
> +        size_t mapbits = chunk->map[pageind].bits;
> +
> +        if (mapbits & CHUNK_MAP_LARGE) {

You should first check if it's CHUNK_MAP_ALLOCATED.

@@ +3587,5 @@
> +                        if (size != 0)
> +                                break;
> +
> +                        pageind--;
> +                        if (pageind <= 0)

pageind being a size_t (unsigned), this can't be smaller than 0.

@@ +3614,5 @@
> +        // Position in the run.
> +        uintptr_t idx = ((uintptr_t)ptr - reg0_addr) / size;
> +
> +        // Pointer to the allocation's base address.
> +        return (const void*)(reg0_addr + idx * size);

You should check whether this is allocated or not.
Attachment #8898115 - Flags: feedback?(mh+mozilla) → feedback+
jemalloc_ptr_info() gives info about any pointer, such as whether it's within a
live or free allocation, and if so, info about that allocation. It's useful for
debugging.

moz_malloc_enclosing_size_of() uses jemalloc_ptr_info() to measure the size of
an allocation from an interior pointer. It's useful for memory reporting,
especially for Rust code.
Attachment #8900636 - Flags: review?(mh+mozilla)
Blocks: 1393384
Comment on attachment 8900636 [details] [diff] [review]
Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of()

Review of attachment 8900636 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/gtest/TestJemalloc.cpp
@@ +61,5 @@
> +}
> +
> +bool InfoEqFreedPage(jemalloc_ptr_info_t& aInfo, void* aAddr)
> +{
> +  static const size_t kPageSize = 4096;

This means the test doesn't work on any platform where that's not true. I'd be in favor of adding the pagesize as jemalloc knows it (which might actually not match what the system page size actually is) to jemalloc_stats_t and use jemalloc_stats() to get the value.

Likewise, it would be better to use small_max, and large_max from there too instead of hardcoding sizes.

@@ +76,5 @@
> +  mozilla::Vector<char*> small, large, huge;
> +
> +  // For small (<= 2KiB) allocations, test every position within many possible
> +  // sizes.
> +  for (size_t n = 0; n <= 2048; n += 8) {

why not n += usable? (true for all loops) (and maybe start with 1)

@@ +111,5 @@
> +
> +  // Free the small allocations and recheck them.
> +  int isFreedSmall = 0, isFreedPage = 0;
> +  while (!small.empty()) {
> +    char* p = small.popCopy();

It would be better to pick them pseudo-randomly. That would allow to detect e.g. off-by-ones that sequentially freeing would miss.

@@ +174,5 @@
> +  ASSERT_TRUE(InfoEq(info, TagNone, nullptr, 0U));
> +
> +  // static memory
> +  jemalloc_ptr_info(&gStaticVar, &info);
> +  ASSERT_TRUE(InfoEq(info, TagNone, nullptr, 0U));

A test picking an address in chunk headers and run headers would be a good addition here.

::: memory/mozjemalloc/mozjemalloc.cpp
@@ +1464,5 @@
>  
> +static inline int
> +extent_bounds_comp(extent_node_t *key, extent_node_t *node)
> +{
> +        uintptr_t key_addr = (uintptr_t)key->addr;

If you're going to use spaces for indentation, please don't use 8.

@@ +3587,5 @@
> +        extent_node_t *node, key;
> +        malloc_mutex_lock(&huge_mtx);
> +        key.addr = const_cast<void*>(ptr);
> +        node = extent_tree_bounds_search(&huge, &key);
> +        if (node)

curly braces?

@@ +3590,5 @@
> +        node = extent_tree_bounds_search(&huge, &key);
> +        if (node)
> +                *info = { TagLiveHuge, node->addr, node->size };
> +        malloc_mutex_unlock(&huge_mtx);
> +        if (node)

likewise

@@ +3603,5 @@
> +        MOZ_DIAGNOSTIC_ASSERT(chunk->arena->magic == ARENA_MAGIC);
> +
> +        // Get the page number within the chunk. Zero isn't used.
> +        size_t pageind = (((uintptr_t)ptr - (uintptr_t)chunk) >> pagesize_2pow);
> +        MOZ_DIAGNOSTIC_ASSERT(pageind != 0);

note that technically, pageind needs to be less than arena_chunk_header_npages, not != 0. And it would probably be better to fail gracefully here.

@@ +3670,5 @@
> +        // Address of the first possible pointer in the run after its headers.
> +        uintptr_t reg0_addr = (uintptr_t)run + run->bin->reg0_offset;
> +
> +        // Position in the run.
> +        unsigned regind = ((uintptr_t)ptr - reg0_addr) / size;

You'll end up with a weird result if for some reason the pointer is in the run header.

@@ +3678,5 @@
> +
> +        // Check if the allocation has been freed.
> +        unsigned elm = regind >> (SIZEOF_INT_2POW + 3);
> +        if (elm < run->regs_minelm)
> +                run->regs_minelm = elm;

You copied this from arena_run_reg_dalloc, but that's only valid there because we're actually making new things being freed, so regs_minelm /might/ need an update. That's not the case here.

::: memory/mozjemalloc/mozjemalloc_types.h
@@ +82,5 @@
> +        /*
> +         * The pointer is not currently known to the allocator.
> +         * 'addr' and 'size' are always 0.
> +         */
> +        TagNone,

TagUnknown?
Attachment #8900636 - Flags: review?(mh+mozilla) → feedback+
> This means the test doesn't work on any platform where that's not true. I'd
> be in favor of adding the pagesize as jemalloc knows it (which might
> actually not match what the system page size actually is) to
> jemalloc_stats_t and use jemalloc_stats() to get the value.
> 
> Likewise, it would be better to use small_max, and large_max from there too
> instead of hardcoding sizes.

Ok. small_max isn't actually the small max (it's the quantum-sized max) but I can use pagesize to compute the real small max.

> > +  for (size_t n = 0; n <= 2048; n += 8) {
> 
> why not n += usable? (true for all loops) (and maybe start with 1)

I assume you're basically suggesting one allocation per size class, but adding usable won't do that. It'll miss heaps of size classes in all three categories. I'll stick with the constant increment, it does cause some sizes classes to be repeated, but that doesn't hurt; it's just more stress testing and the test doesn't take long to run.

> It would be better to pick them pseudo-randomly. That would allow to detect
> e.g. off-by-ones that sequentially freeing would miss.

Ok.

> A test picking an address in chunk headers and run headers would be a good
> addition here.

I've added testing for a chunk header. I'm having trouble working out how to get a pointer into a run header from outside mozjemalloc.cpp.

> If you're going to use spaces for indentation, please don't use 8.

I'll convert the new code to use Gecko style.

> @@ +3603,5 @@
> > +        MOZ_DIAGNOSTIC_ASSERT(chunk->arena->magic == ARENA_MAGIC);
> > +
> > +        // Get the page number within the chunk. Zero isn't used.
> > +        size_t pageind = (((uintptr_t)ptr - (uintptr_t)chunk) >> pagesize_2pow);
> > +        MOZ_DIAGNOSTIC_ASSERT(pageind != 0);
> 
> note that technically, pageind needs to be less than
> arena_chunk_header_npages, not != 0. And it would probably be better to fail
> gracefully here.

I guess you mean "greater than arena_chunk_header_npages"? I will make that change.

> You'll end up with a weird result if for some reason the pointer is in the
> run header.

True, I'll fix. 

> You copied this from arena_run_reg_dalloc, but that's only valid there
> because we're actually making new things being freed, so regs_minelm /might/
> need an update. That's not the case here.

I will fix.

> > +        TagNone,
> 
> TagUnknown?

Ok.
I've addressed all the comments except for testing a pointer within run header,
because I'm not sure how to get one.
Attachment #8901658 - Flags: review?(mh+mozilla)
Attachment #8900636 - Attachment is obsolete: true
Comment on attachment 8901658 [details] [diff] [review]
Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of()

Review of attachment 8901658 [details] [diff] [review]:
-----------------------------------------------------------------

You should be able to find a run header by taking the address of a very small allocation and aligning it to the page size (e.g. malloc(8) & ~(stats.page_size - 1))

::: memory/gtest/TestJemalloc.cpp
@@ +92,5 @@
> +      ASSERT_TRUE(InfoEq(info, TagLiveSmall, p, usable));
> +    }
> +  }
> +
> +  // Similar for large (2KiB + 1 .. 1MiB - 8KiB) allocations.

+ 1 KiB. Or maybe you should actually start from 2KiB + 1 (since that's going to be returning a full page, and considered a large allocation)

@@ +143,5 @@
> +  ASSERT_TRUE(isFreedSmall / isFreedPage > 10);
> +
> +  // Free the large allocations and recheck them.
> +  len = large.length();
> +  for (size_t i = 0, j = 0; i < len; i++, j = (j + 19) % len) {

large.length() should be larger than small.length(), why use a smaller prime here?

@@ +202,5 @@
> +  }
> +
> +  // Entire chunk. It's impossible to check what is put into |info| for all of
> +  // these addresses; this is more about checking that we don't crash.
> +  for (size_t i = 0; i < chunkHeaderSize; i += 256) {

ITYM stats.chunksize, but that won't work as a test, because you'll likely get some TagFreed stuff in there.

::: memory/mozjemalloc/mozjemalloc.cpp
@@ +1464,5 @@
>  
> +static inline int
> +extent_bounds_comp(extent_node_t *key, extent_node_t *node)
> +{
> +        uintptr_t key_addr = (uintptr_t)key->addr;

You missed this function when reindenting.

@@ +3648,5 @@
> +      // practice unless there is heap corruption.
> +
> +      pageind--;
> +      MOZ_DIAGNOSTIC_ASSERT(pageind != 0);
> +      if (pageind == 0) {

< arena_chunk_header_npages here too?
Attachment #8901658 - Flags: review?(mh+mozilla) → feedback+
> You should be able to find a run header by taking the address of a very
> small allocation and aligning it to the page size (e.g. malloc(8) &
> ~(stats.page_size - 1))

Great, thank you.

> > +  // Similar for large (2KiB + 1 .. 1MiB - 8KiB) allocations.
> 
> + 1 KiB. Or maybe you should actually start from 2KiB + 1 (since that's
> going to be returning a full page, and considered a large allocation)

Hmm, that would mess up the incrementing, and I think it's well established that 2049 and 3072 both get rounded up to 4096, so I'll just fix the comment.

> large.length() should be larger than small.length(), why use a smaller prime
> here?

Ok, I swapped the step sizes for these two vectors.
 
> > +  // Entire chunk. It's impossible to check what is put into |info| for all of
> > +  // these addresses; this is more about checking that we don't crash.
> > +  for (size_t i = 0; i < chunkHeaderSize; i += 256) {
> 
> ITYM stats.chunksize, but that won't work as a test, because you'll likely
> get some TagFreed stuff in there.

You're right about stats.chunksize, good catch. Oh, and there's not supposed to be an ASSERT_TRUE within the loop; I have removed it.

> You missed this function when reindenting.

True, fixed.

> > +      MOZ_DIAGNOSTIC_ASSERT(pageind != 0);
> > +      if (pageind == 0) {
> 
> < arena_chunk_header_npages here too?

Fixed.
jemalloc_ptr_info() gives info about any pointer, such as whether it's within a
live or free allocation, and if so, info about that allocation. It's useful for
debugging.

moz_malloc_enclosing_size_of() uses jemalloc_ptr_info() to measure the size of
an allocation from an interior pointer. It's useful for memory reporting,
especially for Rust code.
Attachment #8903020 - Flags: review?(mh+mozilla)
Attachment #8901658 - Attachment is obsolete: true
Comment on attachment 8903020 [details] [diff] [review]
Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of()

Review of attachment 8903020 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/gtest/TestJemalloc.cpp
@@ +92,5 @@
> +      ASSERT_TRUE(InfoEq(info, TagLiveSmall, p, usable));
> +    }
> +  }
> +
> +  // Similar for large (2KiB + 1 K .. 1MiB - 8KiB) allocations.

Weird to have K here where on the same line you have KiB twice.

@@ +203,5 @@
> +
> +  // Run header.
> +  size_t page_sizeMask = stats.page_size - 1;
> +  char* run = (char*)(uintptr_t(p.get()) & ~page_sizeMask);
> +  for (size_t i = 0; i < 16; i++) {

you could even go as far as 4 * sizeof(void*)
Attachment #8903020 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8903020 [details] [diff] [review]
Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of()

Review of attachment 8903020 [details] [diff] [review]:
-----------------------------------------------------------------

::: memory/mozjemalloc/mozjemalloc_types.h
@@ +115,5 @@
> +  void* addr;     // meaning depends on tag; see above
> +  size_t size;    // meaning depends on tag; see above
> +} jemalloc_ptr_info_t;
> +
> +static inline jemalloc_bool

Note I bitrotted you with bug 1395088
https://hg.mozilla.org/integration/mozilla-inbound/rev/f232b5b1a0c74b84c5d7f4ecb131d25a92601015
Bug 1389305 - Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of(). r=glandium.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f232b5b1a0c7
Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of(). r=glandium.
Backed out bug 1393384 and bug 1389305 for frequently failing (25% of the test runs) GTest Jemalloc.PtrInfo on Linux opt:

https://hg.mozilla.org/integration/mozilla-inbound/rev/584579605f5f4e7f46434149f0c2e1e87cc49111
https://hg.mozilla.org/integration/mozilla-inbound/rev/0922fc9b3ae605ba93fa0036017c72afeb4e0f0a

Push with failures https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=5b30f38210e155ef046b55ef22e19d771c13540e&filter-searchStr=711f49d9771acf7fa2767abd6f05979f3a534a39
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=127368910&repo=mozilla-inbound

[task 2017-08-31T11:36:30.809277Z] 11:36:30     INFO -  TEST-START | Jemalloc.PtrInfo
[task 2017-08-31T11:36:31.095072Z] 11:36:31     INFO -  ExceptionHandler::GenerateDump cloned child 10221
[task 2017-08-31T11:36:31.095507Z] 11:36:31     INFO -  ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2017-08-31T11:36:31.095897Z] 11:36:31     INFO -  ExceptionHandler::WaitForContinueSignal waiting for continue signal...
[task 2017-08-31T11:36:31.479065Z] 11:36:31     INFO -  mozcrash INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/MFuOeItiQBCW47agEur-jA/artifacts/public/build/target.crashreporter-symbols.zip
[task 2017-08-31T11:36:38.507304Z] 11:36:38     INFO -  mozcrash INFO | Copy/paste: /usr/local/bin/linux64-minidump_stackwalk /builds/worker/workspace/build/tests/gtest/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp /tmp/tmpVmJiqr
[task 2017-08-31T11:36:47.021275Z] 11:36:47     INFO -  mozcrash INFO | Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp
[task 2017-08-31T11:36:47.021708Z] 11:36:47     INFO -  mozcrash INFO | Saved app info as /builds/worker/workspace/build/blobber_upload_dir/3cbaba07-5872-23f8-0bb7-c6398552a86e.extra
[task 2017-08-31T11:36:47.022171Z] 11:36:47  WARNING -  PROCESS-CRASH | gtest | application crashed [@ Jemalloc_PtrInfo_Test::TestBody]
[task 2017-08-31T11:36:47.022800Z] 11:36:47     INFO -  Crash dump filename: /builds/worker/workspace/build/tests/gtest/3cbaba07-5872-23f8-0bb7-c6398552a86e.dmp
[task 2017-08-31T11:36:47.022917Z] 11:36:47     INFO -  Operating system: Linux
[task 2017-08-31T11:36:47.023444Z] 11:36:47     INFO -                    0.0.0 Linux 3.13.0-112-generic #159-Ubuntu SMP Fri Mar 3 15:26:07 UTC 2017 x86_64
[task 2017-08-31T11:36:47.023881Z] 11:36:47     INFO -  CPU: x86
[task 2017-08-31T11:36:47.024329Z] 11:36:47     INFO -       GenuineIntel family 6 model 62 stepping 4
[task 2017-08-31T11:36:47.024786Z] 11:36:47     INFO -       4 CPUs
[task 2017-08-31T11:36:47.025202Z] 11:36:47     INFO -  GPU: UNKNOWN
[task 2017-08-31T11:36:47.025590Z] 11:36:47     INFO -  Crash reason:  SIGFPE
[task 2017-08-31T11:36:47.026033Z] 11:36:47     INFO -  Crash address: 0xf30a3c91
[task 2017-08-31T11:36:47.026482Z] 11:36:47     INFO -  Process uptime: not available
[task 2017-08-31T11:36:47.026942Z] 11:36:47     INFO -  Thread 0 (crashed)
[task 2017-08-31T11:36:47.027290Z] 11:36:47     INFO -   0  libxul.so!Jemalloc_PtrInfo_Test::TestBody [TestJemalloc.cpp:5b30f38210e1 : 143 + 0x7]
[task 2017-08-31T11:36:47.027770Z] 11:36:47     INFO -      eip = 0xf30a3c91   esp = 0xffa39c20   ebp = 0xffa39cf8   ebx = 0xf5433000
[task 2017-08-31T11:36:47.028262Z] 11:36:47     INFO -      esi = 0xb8f0b800   edi = 0x00000101   eax = 0x000541fc   ecx = 0x00000101
[task 2017-08-31T11:36:47.028708Z] 11:36:47     INFO -      edx = 0x00000000   efl = 0x00010246
[task 2017-08-31T11:36:47.029178Z] 11:36:47     INFO -      Found by: given as instruction pointer in context
[task 2017-08-31T11:36:47.029629Z] 11:36:47     INFO -   1  libxul.so!testing::Test::Run [gtest.cc:5b30f38210e1 : 2475 + 0x21]
[task 2017-08-31T11:36:47.030130Z] 11:36:47     INFO -      eip = 0xf2d54cd9   esp = 0xffa39d00   ebp = 0xffa39d38   ebx = 0xf5433000
[task 2017-08-31T11:36:47.030578Z] 11:36:47     INFO -      esi = 0x9a272180   edi = 0xf7159150
[task 2017-08-31T11:36:47.030961Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.031409Z] 11:36:47     INFO -   2  libxul.so!testing::TestInfo::Run [gtest.cc:5b30f38210e1 : 2466 + 0x1a]
[task 2017-08-31T11:36:47.032020Z] 11:36:47     INFO -      eip = 0xf2d5d2a5   esp = 0xffa39d40   ebp = 0xffa39d98   ebx = 0xf5433000
[task 2017-08-31T11:36:47.032249Z] 11:36:47     INFO -      esi = 0xeea6e4e0   edi = 0xf7159150
[task 2017-08-31T11:36:47.032653Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.032967Z] 11:36:47     INFO -   3  libxul.so!testing::TestCase::Run [gtest.cc:5b30f38210e1 : 2631 + 0x12]
[task 2017-08-31T11:36:47.033406Z] 11:36:47     INFO -      eip = 0xf2d5d44d   esp = 0xffa39da0   ebp = 0xffa39e08   ebx = 0x00000001
[task 2017-08-31T11:36:47.033708Z] 11:36:47     INFO -      esi = 0xeea69b00   edi = 0xf7127690
[task 2017-08-31T11:36:47.034242Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.035034Z] 11:36:47     INFO -   4  libxul.so!testing::internal::UnitTestImpl::RunAllTests [gtest.cc:5b30f38210e1 : 4687 + 0x18]
[task 2017-08-31T11:36:47.035121Z] 11:36:47     INFO -      eip = 0xf2d5d92d   esp = 0xffa39e10   ebp = 0xffa39e98   ebx = 0x000000fc
[task 2017-08-31T11:36:47.035186Z] 11:36:47     INFO -      esi = 0xf7159150   edi = 0xf716c4b8
[task 2017-08-31T11:36:47.035638Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.036094Z] 11:36:47     INFO -   5  libxul.so!testing::UnitTest::Run [gtest.cc:5b30f38210e1 : 2458 + 0x8]
[task 2017-08-31T11:36:47.036620Z] 11:36:47     INFO -      eip = 0xf2d5dc74   esp = 0xffa39ea0   ebp = 0xffa39ec8   ebx = 0xf5433000
[task 2017-08-31T11:36:47.036948Z] 11:36:47     INFO -      esi = 0xf7159150   edi = 0xef3878c0
[task 2017-08-31T11:36:47.037355Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.037787Z] 11:36:47     INFO -   6  libxul.so!mozilla::RunGTestFunc [gtest.h:5b30f38210e1 : 2233 + 0xd]
[task 2017-08-31T11:36:47.038178Z] 11:36:47     INFO -      eip = 0xf2d603f5   esp = 0xffa39ed0   ebp = 0xffa39f38   ebx = 0xf5433000
[task 2017-08-31T11:36:47.038548Z] 11:36:47     INFO -      esi = 0xede7afd4   edi = 0xef3878c0
[task 2017-08-31T11:36:47.038976Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.039516Z] 11:36:47     INFO -   7  libxul.so!XREMain::XRE_mainStartup [nsAppRunner.cpp:5b30f38210e1 : 3877 + 0x9]
[task 2017-08-31T11:36:47.039798Z] 11:36:47     INFO -      eip = 0xf280f634   esp = 0xffa39f40   ebp = 0xffa3a118   ebx = 0xf5433000
[task 2017-08-31T11:36:47.040180Z] 11:36:47     INFO -      esi = 0xffa3a080   edi = 0xffa3a06c
[task 2017-08-31T11:36:47.040646Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.041110Z] 11:36:47     INFO -   8  libxul.so!XREMain::XRE_main [nsAppRunner.cpp:5b30f38210e1 : 4794 + 0xf]
[task 2017-08-31T11:36:47.041532Z] 11:36:47     INFO -      eip = 0xf28139ff   esp = 0xffa3a120   ebp = 0xffa3a178   ebx = 0xf5433000
[task 2017-08-31T11:36:47.042005Z] 11:36:47     INFO -      esi = 0x00000000   edi = 0xffa3a154
[task 2017-08-31T11:36:47.042478Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.042857Z] 11:36:47     INFO -   9  libxul.so!XRE_main [nsAppRunner.cpp:5b30f38210e1 : 4904 + 0x5]
[task 2017-08-31T11:36:47.043352Z] 11:36:47     INFO -      eip = 0xf2813ea7   esp = 0xffa3a180   ebp = 0xffa3a2d8   ebx = 0x08075000
[task 2017-08-31T11:36:47.043735Z] 11:36:47     INFO -      esi = 0xffa3a1a4   edi = 0xffa3a1e8
[task 2017-08-31T11:36:47.044097Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.044541Z] 11:36:47     INFO -  10  firefox!do_main [nsBrowserApp.cpp:5b30f38210e1 : 236 + 0x1a]
[task 2017-08-31T11:36:47.044994Z] 11:36:47     INFO -      eip = 0x0804d83a   esp = 0xffa3a2e0   ebp = 0xffa3b328   ebx = 0x08075000
[task 2017-08-31T11:36:47.045326Z] 11:36:47     INFO -      esi = 0xffa3b424   edi = 0x00000002
[task 2017-08-31T11:36:47.045780Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.046222Z] 11:36:47     INFO -  11  firefox!main [nsBrowserApp.cpp:5b30f38210e1 : 309 + 0xc]
[task 2017-08-31T11:36:47.046681Z] 11:36:47     INFO -      eip = 0x0804cf81   esp = 0xffa3b330   ebp = 0xffa3b378   ebx = 0x08075000
[task 2017-08-31T11:36:47.047147Z] 11:36:47     INFO -      esi = 0xffa3b424   edi = 0x00000002
[task 2017-08-31T11:36:47.047583Z] 11:36:47     INFO -      Found by: call frame info
[task 2017-08-31T11:36:47.048092Z] 11:36:47     INFO -  12  libc-2.23.so + 0x18637
[task 2017-08-31T11:36:47.048537Z] 11:36:47     INFO -      eip = 0xf734d637   esp = 0xffa3b380   ebp = 0x00000000
[task 2017-08-31T11:36:47.049065Z] 11:36:47     INFO -      Found by: previous frame's frame pointer
[task 2017-08-31T11:36:47.049580Z] 11:36:47     INFO -  13  libc-2.23.so + 0x1b2000
[task 2017-08-31T11:36:47.050148Z] 11:36:47     INFO -      eip = 0xf74e7000   esp = 0xffa3b384   ebp = 0x00000000
[task 2017-08-31T11:36:47.050298Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.050752Z] 11:36:47     INFO -  14  libc-2.23.so + 0x1b2000
[task 2017-08-31T11:36:47.051308Z] 11:36:47     INFO -      eip = 0xf74e7000   esp = 0xffa3b388   ebp = 0x00000000
[task 2017-08-31T11:36:47.051512Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.052128Z] 11:36:47     INFO -  15  libc-2.23.so + 0x18637
[task 2017-08-31T11:36:47.052485Z] 11:36:47     INFO -      eip = 0xf734d637   esp = 0xffa3b390   ebp = 0x00000000
[task 2017-08-31T11:36:47.052786Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.053361Z] 11:36:47     INFO -  16  libc-2.23.so + 0x1b2000
[task 2017-08-31T11:36:47.056094Z] 11:36:47     INFO -      eip = 0xf74e7000   esp = 0xffa3b3ac   ebp = 0x00000000
[task 2017-08-31T11:36:47.056142Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.056180Z] 11:36:47     INFO -  17  libc-2.23.so + 0x1b2000
[task 2017-08-31T11:36:47.056748Z] 11:36:47     INFO -      eip = 0xf74e7000   esp = 0xffa3b3bc   ebp = 0x00000000
[task 2017-08-31T11:36:47.056804Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.056851Z] 11:36:47     INFO -  18  libc-2.23.so + 0x1b2000
[task 2017-08-31T11:36:47.057162Z] 11:36:47     INFO -      eip = 0xf74e7000   esp = 0xffa3b3c0   ebp = 0x00000000
[task 2017-08-31T11:36:47.057212Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.057734Z] 11:36:47     INFO -  19  firefox + 0x5264
[task 2017-08-31T11:36:47.058017Z] 11:36:47     INFO -      eip = 0x0804d264   esp = 0xffa3b3e0   ebp = 0x00000000
[task 2017-08-31T11:36:47.058306Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.058770Z] 11:36:47     INFO -  20  ld-2.23.so + 0x15030
[task 2017-08-31T11:36:47.059288Z] 11:36:47     INFO -      eip = 0xf773a030   esp = 0xffa3b3e8   ebp = 0x00000000
[task 2017-08-31T11:36:47.059528Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.060106Z] 11:36:47     INFO -  21  ld-2.23.so + 0xf8a0
[task 2017-08-31T11:36:47.060555Z] 11:36:47     INFO -      eip = 0xf77348a0   esp = 0xffa3b3ec   ebp = 0x00000000
[task 2017-08-31T11:36:47.061148Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.061326Z] 11:36:47     INFO -  22  firefox + 0x5264
[task 2017-08-31T11:36:47.061762Z] 11:36:47     INFO -      eip = 0x0804d264   esp = 0xffa3b3f8   ebp = 0x00000000
[task 2017-08-31T11:36:47.062276Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.062718Z] 11:36:47     INFO -  23  firefox!_start + 0x21
[task 2017-08-31T11:36:47.063146Z] 11:36:47     INFO -      eip = 0x0804d285   esp = 0xffa3b400   ebp = 0x00000000
[task 2017-08-31T11:36:47.063601Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.064111Z] 11:36:47     INFO -  24  firefox + 0x4ed0
[task 2017-08-31T11:36:47.064374Z] 11:36:47     INFO -      eip = 0x0804ced0   esp = 0xffa3b404   ebp = 0x00000000
[task 2017-08-31T11:36:47.064918Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.065278Z] 11:36:47     INFO -  25  firefox!__libc_csu_fini + 0x10
[task 2017-08-31T11:36:47.065856Z] 11:36:47     INFO -      eip = 0x0806c470   esp = 0xffa3b410   ebp = 0xffa3b424
[task 2017-08-31T11:36:47.066051Z] 11:36:47     INFO -      Found by: stack scanning
[task 2017-08-31T11:36:47.066428Z] 11:36:47     INFO -  26  0xffa3c693
[task 2017-08-31T11:36:47.066972Z] 11:36:47     INFO -      eip = 0xffa3c693   esp = 0xffa3b42c   ebp = 0xffa3c658
[task 2017-08-31T11:36:47.067089Z] 11:36:47     INFO -      Found by: previous frame's frame pointer
[task 2017-08-31T11:36:47.067477Z] 11:36:47     INFO -  27  0x2f73646c
[task 2017-08-31T11:36:47.067929Z] 11:36:47     INFO -      eip = 0x2f73646c   esp = 0xffa3c660   ebp = 0x6975622f
[task 2017-08-31T11:36:47.068156Z] 11:36:47     INFO -      Found by: previous frame's frame pointer
Flags: needinfo?(n.nethercote)
That implies a variance on isFreedSmall and isFreedPage in
  ASSERT_TRUE(isFreedSmall / isFreedPage > 10);

This shouldn't be happening, right?
> That implies a variance on isFreedSmall and isFreedPage in
>   ASSERT_TRUE(isFreedSmall / isFreedPage > 10);

The crash is a SIGFPE which suggests that isFreedPage is zero, which is strange. And it appears to be Linux32-only.

I'm inclined to just remove the assertion.
Flags: needinfo?(n.nethercote)
Attachment #8898115 - Attachment is obsolete: true
I'd rather know why it's 0 when it shouldn't.
I've run the test locally (by moving it off gtest), and the numbers for isFreedSmall and isFreedPage are, as expected, constant. Which makes me think this could be due to gtests running multiple tests in parallel, which means other threads can be allocating memory, making things that were freed by this test reallocated. One way to avoid this problem is to make this test register for a thread local arena (jemalloc_thread_local_arena(true)), and unregister at the end (jemalloc_thread_local_arena(false)).
Blocks: 1395776
Did a try run with some logging, with and without my suggestion.
- without, I got 8 gtest failures and 24 successes.
  - all the failures had isFreedSmall=344572 and isFreedPage=0
  - the successes had varying numbers such as:
    isFreedSmall=343548 isFreedPage=1024
    isFreedSmall=344364 isFreedPage=208
    isFreedSmall=343340 isFreedPage=1232
    isFreedSmall=343212 isFreedPage=1360
    etc.
- with, I got no failures on 25 runs.
  - all the runs had isFreedSmall=332144 isFreedPage=12428

It seems gtest doesn't itself run tests in parallel, but we likely have some tests that trigger things on threads of some sort.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f789b5b305bb3934a3ab9caa19734d7c328a8a3e
Bug 1389305 (attempt 2) - Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of(). r=glandium.
Pushed by nnethercote@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f789b5b305bb
(attempt 2) - Add jemalloc_ptr_info() and moz_malloc_enclosing_size_of(). r=glandium.
https://hg.mozilla.org/mozilla-central/rev/f789b5b305bb
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: