Closed
Bug 1501808
Opened 6 years ago
Closed 6 years ago
CacheFileMetadata::GetElement+0x51
Categories
(Firefox Build System :: General: Unsupported Platforms, enhancement, P5)
Firefox Build System
General: Unsupported Platforms
Tracking
(firefox65 fixed)
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: tjr, Assigned: tjr)
References
Details
Attachments
(2 files)
4.00 KB,
text/plain
|
Details | |
1.31 KB,
patch
|
away
:
review+
|
Details | Diff | Splinter Review |
We crash right here: https://searchfox.org/mozilla-central/rev/72b1e834f384a2ffec6eb4ce405fbd4b5e881109/netwerk/cache2/CacheFileMetadata.cpp#396 because data/mBuf is NULL. Using this build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=83a430539f8bed766f1bcd00a13b629fb89f72d1&selectedJob=207371345 (x64 opt) opening this page: https://browserbench.org/ARES-6 I don't know if this is related to Bug 1501799 - they both seem to involve file access stuff. I'm not sure how well supported --disable-sandbox is on Windows; it might even be an error with that. I'm going to try a build with sandbox enabled and see what happens....
Assignee | ||
Comment 1•6 years ago
|
||
Same (for both bugs) with --enable-sandbox; so seems unrelated.
Assignee | ||
Comment 2•6 years ago
|
||
Backtracked this crash a bunch. I'm getting a string that contains 1;%64d,blahblahblah. That shouldn't be %64d, that should be a number. This line right here: https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/xpcom/string/nsTSubstring.h#727 I placed breakpoints on all of these and inspected the format strings. They're %I64d and %I64u...
Comment 3•6 years ago
|
||
Apparently Microsoft allows %I64d in printf format strings: https://msdn.microsoft.com/en-us/library/tcxf1dw6.aspx But nsTSubstring::AppendPrintf uses this code, which doesn't look like it supports that format specifier: https://searchfox.org/mozilla-central/rev/d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/mozglue/misc/Printf.cpp#549
Assignee | ||
Comment 4•6 years ago
|
||
(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #3) > Apparently Microsoft allows %I64d in printf format strings: > https://msdn.microsoft.com/en-us/library/tcxf1dw6.aspx > > But nsTSubstring::AppendPrintf uses this code, which doesn't look like it > supports that format specifier: > https://searchfox.org/mozilla-central/rev/ > d5fc6f3d4746279a7c6fd4f7a98b1bc5d05d6b01/mozglue/misc/Printf.cpp#549 clang-cl uses different format specifiers than mingw: https://ritter.vg/misc/transient/format-specifiers-diff.html
Assignee | ||
Comment 5•6 years ago
|
||
I wrote a MinGW patch that changes the format specifiers, and that fixes it! https://treeherder.mozilla.org/#/jobs?repo=try&revision=5db7504d12cf2e5181f2fcd46e09df37e0e80159 Even with the sandbox, I've run all four tests on browserbench without any crash. Once we find out if MinGW wants to upstream my patch or something equivalent; we can take action here.
Comment 6•6 years ago
|
||
I just pushed the fix to upstream mingw-w64.
Assignee | ||
Comment 7•6 years ago
|
||
Attachment #9022237 -
Flags: review?(core-build-config-reviews)
Comment on attachment 9022237 [details] [diff] [review] Bug 1501808 - Bump MinGW Version to capture several MinGW fixes 70860d94 (HEAD -> master, origin/master, origin/HEAD) inttypes.h: Take into account __USE_MINGW_ANSI_STDIO and msvcrt version instead of depending on _mingw_print_p*.h headers. b32ed6fb winternl.h: Added PUBLIC_OBJECT_TYPE_INFORMATION declaration. ed8173d5 Updated imported headers to current Wine version. e3542077 widl: Updated to current Wine version. a91a7e40 _mingw_mac.h: Enable WIDL_EXPLICIT_AGGREGATE_RETURNS workaround for ABI incompatibility on affected platforms 8bba2a9f winpthreads: make winpthreads compatible with libcxx __attribute__((__require_constant_initialization__)) c69c7a70 headers: Update the threadlocinfo struct for ucrt
Attachment #9022237 -
Flags: review?(core-build-config-reviews) → review+
Assignee | ||
Updated•6 years ago
|
Keywords: checkin-needed
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e8c25ac5d5bb Bump MinGW Version to capture several MinGW fixes. r=dmajor
Keywords: checkin-needed
Comment 10•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e8c25ac5d5bb
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox65:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
Updated•5 years ago
|
Priority: -- → P5
You need to log in
before you can comment on or make changes to this bug.
Description
•