1587567 - Fatal assert in startup cache code: "Assertion failure: mPtr <= mRangeEnd, at $OBJ/dist/include/mozilla/RangedPtr.h:54"

Status: RESOLVED DUPLICATE of bug 1587112

(Toolkit :: Startup and Profile System, defect)

Description

[Daniel Holbert [:dholbert]]

I'm getting a startup crash in StartupCache::LoadArchive(), in a ~fresh profile.

Specifically, I'm getting a fatal assertion failure in RangedPtr code, when evaluating the if check here:

  if (data + headerSize > end) {
    return Err(NS_ERROR_UNEXPECTED);
  }

Backtrace of the failure:

Assertion failure: mPtr <= mRangeEnd, at $OBJ/dist/include/mozilla/RangedPtr.h:54
#01: mozilla::RangedPtr<unsigned char>::checkSanity() ($OBJ/dist/include/mozilla/RangedPtr.h:54)
#02: RangedPtr ($OBJ/dist/include/mozilla/RangedPtr.h:79)
#03: mozilla::RangedPtr<unsigned char>::create(unsigned char*) const ($OBJ/dist/include/mozilla/RangedPtr.h:60)
#04: mozilla::RangedPtr<unsigned char>::operator+(unsigned long) const ($OBJ/dist/include/mozilla/RangedPtr.h:161)
#05: mozilla::scache::StartupCache::LoadArchive() ($SRC/startupcache/StartupCache.cpp:286)
#06: mozilla::scache::StartupCache::Init() ($SRC/startupcache/StartupCache.cpp:232)
#07: mozilla::scache::StartupCache::InitSingleton() ($SRC/startupcache/StartupCache.cpp:132)
#08: mozilla::scache::StartupCache::GetSingleton() ($SRC/startupcache/StartupCache.cpp:119)
#09: NS_InitXPCOM ($SRC/xpcom/build/XPCOMInit.cpp:464)
#10: ScopedXPCOMStartup::Initialize() ($SRC/toolkit/xre/nsAppRunner.cpp:1281)
#11: XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) ($SRC/toolkit/xre/nsAppRunner.cpp:4731)
#12: XRE_main(int, char**, mozilla::BootstrapConfig const&) ($SRC/toolkit/xre/nsAppRunner.cpp:4816)
#13: mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) ($SRC/toolkit/xre/Bootstrap.cpp:45)
#14: do_main(int, char**, char**) ($SRC/browser/app/nsBrowserApp.cpp:218)
#15: main ($SRC/browser/app/nsBrowserApp.cpp:300)
#16: __libc_start_main (/build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:342)
#17: _start ($OBJ/dist/bin/firefox)
#18: ??? (???:???)

I poked around in gdb, and here's some useful info:

11 0x00007fe5771a6606 in mozilla::scache::StartupCache::LoadArchive (
    this=0x7fe564cd1b50)
    at $SRC/startupcache/StartupCache.cpp:286
(gdb) list
281	  data += sizeof(MAGIC);
282	
283	  headerSize = LittleEndian::readUint32(data.get());
284	  data += sizeof(headerSize);
285	
286	  if (data + headerSize > end) {
287	    return Err(NS_ERROR_UNEXPECTED);
288	  }
289	
290	  Range<uint8_t> header(data, data + headerSize);
(gdb) p data.mRangeEnd - data.mRangeStart
$11 = 21
(gdb) p data.mRangeEnd - data.mPtr
$12 = 0
(gdb) p headerSize
$13 = 36281

So it looks like we're at the end of a 21-long range, and we're checking whether we can step forward 36281 without hitting the end (trivially "no, we cannot"), and the way RangedPtr operators works, simply asking that question with operator+ is sufficient to trigger a fatal assertion.

Comment 1

[Daniel Holbert [:dholbert]]

This is for a local debug build with a relatively-fresh profile. It started happening after I hit another unrelated crash, and then tried to start Firefox with ./mach run. I'm guessing the first unrelated crash may have left behind some slightly-unexpected startupcache data (not sure), but we should still be able to fail gracefully instead of instacrashing, I'd expect.

Comment 2

[Daniel Holbert [:dholbert]]

Also: after quitting gdb & trying to start Firefox again, I was able to start successfully. So whatever the problem was, it was transient & wasn't something persistently busted in the profile.

Comment 3

[Daniel Holbert [:dholbert]]

So regarding this problematic comparison (which can create an invalid RangedPtr and fatally assert):

  if (data + headerSize > end) {

https://searchfox.org/mozilla-central/rev/05a22d864814cb1e4352faa4004e1f975c7d2eb9/startupcache/StartupCache.cpp#286

I suspect the better/non-crashy way to perform this comparison would be to subtract end from data like so (just adjusting the comparison to move data to the other side):

  if (headerSize > end - data) {

The end - data operation is valid and produces a size_t:
https://searchfox.org/mozilla-central/rev/05a22d864814cb1e4352faa4004e1f975c7d2eb9/mfbt/RangedPtr.h#271-273
...and it's fine to compare that size_t against headerSize.

Comment 4

[Alex Thayer [:alexical] (she/her)]

This is helpful info - thanks! I am duping it over to 1587112 though as it's the same issue.