Crash in crashpad_handler on macOS at base::PersistentMemoryAllocator::PersistentMemoryAllocator() |
|||
Issue descriptionA large portion of crashpad_handler crashes on Mac are: https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%27crashpad-handler%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27base%3A%3APersistentMemoryAllocator%3A%3APersistentMemoryAllocator%27%20AND%20product.name%3D%27Chrome_Mac%27 Sample: go/crash/26e5300188000000 base::PersistentMemoryAllocator::PersistentMemoryAllocator base::FilePersistentMemoryAllocator::FilePersistentMemoryAllocator base::GlobalHistogramAllocator::CreateWithFile base::GlobalHistogramAllocator::CreateWithActiveFileInDir crashpad::HandlerMain The crash is EXC_BAD_ACCESS/KERN_MEMORY_ERROR at a page-aligned address at this line: https://chromium.googlesource.com/chromium/src/+/4e72a1554a6b4efb3ea711ead4876f0f3e276e45/base/metrics/persistent_memory_allocator.cc#350 350 if (shared_meta()->cookie != kGlobalCookie) { This is the first access into PersistentMemoryAllocator::mem_base_, and cookie is at offset 0. EXC_BAD_ACCESS/KERN_MEMORY_ERROR sounds like it could be a file that isn’t large enough to back the memory-mapped region (the Mach equivalent of SIGBUS). I’m wondering if this is a problem: https://chromium.googlesource.com/chromium/src/+/85f982b0e451c62c0127c83e846988b903b56e73/base/metrics/persistent_histogram_allocator.cc#762 767 bool exists = PathExists(file_path); […] 773 if (exists) { 774 mmfile->Initialize(std::move(file), MemoryMappedFile::READ_WRITE); 775 } else { 776 mmfile->Initialize(std::move(file), {0, static_cast<int64_t>(size)}, 777 MemoryMappedFile::READ_WRITE_EXTEND); 778 } If the file already exists, “size” is never used to ensure that the file is large enough. “size” is only used to set the size of the file if it didn’t exist to begin with. If the file exists with length 0, this would result in a zero-length file being used to back a nonzero-length mapped memory region. 785 Set(WrapUnique( 786 new GlobalHistogramAllocator(MakeUnique<FilePersistentMemoryAllocator>( 787 std::move(mmfile), size, id, name, false)))); I see a much smaller bucket (relative to total crashpad_handler crashes) for base::PersistentMemoryAllocator::PersistentMemoryAllocator on Windows. https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%27crashpad-handler%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27base%3A%3APersistentMemoryAllocator%3A%3APersistentMemoryAllocator%27%20AND%20product.name%3D%27Chrome They aren’t crashing on the same cookie check line as we’re seeing on macOS, but there are EXCEPTION_ACCESS_VIOLATION_{READ,WRITE} exceptions while working with the file-backed memory region, and it’s possible that they share a cause.
,
Aug 1 2017
I don’t think that would be a failure, you’d be able to interact with the portion that’s there.
,
Aug 1 2017
That would be my expectation, too. But since it's crashing when trying to access the header cookie which is at offset 0... Then the file must not be map-able. I've got a CL out that'll load the size from the file and use that but it will only correct access to actual data in the case of a corrupted memory segment. Perhaps it's the second case where the file is being created. There is a new Mac filesystem that properly supports sparse files. Could it be a bug in that? There is no "realization" of the file on Mac to force allocation of all the blocks. We could try enabling that... but if that was the cause then I'd expect it to affect the browser, too. Is this limited to Crashpad?
,
Aug 1 2017
The crash reports that I’ve seen haven’t been from the 10.13 betas, and I wouldn’t expect them to be using APFS with proper sparse file support. I’ve only looked at Crashpad, but I didn’t think that Chrome used the file-backed mmap approach for metrics for other process types.
,
Aug 2 2017
The browser process uses a 8MB memory-mapped file for all its metrics storage. It uses the same "active/old" file management. Subprocesses use shared memory but that isn't sufficient for persistence in the browser.
,
Aug 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fbdce5f50459c167c3e43641b4e1cde87d2ba512 commit fbdce5f50459c167c3e43641b4e1cde87d2ba512 Author: Brian White <bcwhite@chromium.org> Date: Wed Aug 02 21:18:01 2017 Capture the size of an existing file for allocator size. There are checks inside the PMA that limit any passed size to whatever was defined when the memory segment was created. But that could be wrong if the segment is corrupted. Bug: 749223 Change-Id: Ib0f5416e4c43b597b15e2cb05b3787677e5c0ea1 Reviewed-on: https://chromium-review.googlesource.com/596487 Commit-Queue: Brian White <bcwhite@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Cr-Commit-Position: refs/heads/master@{#491502} [modify] https://crrev.com/fbdce5f50459c167c3e43641b4e1cde87d2ba512/base/metrics/persistent_histogram_allocator.cc
,
Aug 9 2017
,
Aug 9 2017
I don't see any of these crashes in M61 or M62. Granted, they're infrequent so that might not mean anything given the reduced user base of beta & dev on Mac.
,
Aug 9 2017
Unfortunately I don’t think we can draw any conclusions until a version’s gone to stable.
,
Nov 3 2017
The last version exhibiting this crash was 60.0.3089.0 so I believe it's fixed. |
|||
►
Sign in to add a comment |
|||
Comment 1 by bcwh...@chromium.org
, Aug 1 2017