The default bug view has changed. See this FAQ.

Cache causes crash crossing architecture [@ nsDiskCacheRecord::HashNumber].[@ nsDiskCacheMap::FindRecord]

RESOLVED FIXED

Status

()

Core
Networking: Cache
--
critical
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Stuart Morgan, Assigned: Mark Mentovai)

Tracking

({fixed1.8.0.2, fixed1.8.1})

1.8 Branch
PowerPC
Mac OS X
fixed1.8.0.2, fixed1.8.1
Points:
---
Bug Flags:
blocking1.8.0.2 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5][camino-1.0.6])

Attachments

(4 attachments)

(Reporter)

Description

11 years ago
Try saying that three times fast.

Camino and Firefox will both crash on first lanch if the previous launch was under the opposite architecture (because someone copied their entire library from an old machine, got a universal version after having run under Rosetta, or just toggled Rosetta on and off for kicks).  The crash doesn't reproduce if you manually delete the cache before launching the other way.
(Assignee)

Comment 1

11 years ago
Created attachment 210651 [details]
Stack trace (ppc with x86 cache)

Oh yeah.  It's all about the cache.

Also TB14724380H.
(Assignee)

Updated

11 years ago
Component: Profile: BackEnd → Networking: Cache
(Assignee)

Updated

11 years ago
Attachment #210651 - Attachment description: Stack trace → Stack trace (ppc with x86 cache)
(Assignee)

Updated

11 years ago
Severity: normal → critical
Status: NEW → ASSIGNED
Summary: Cache causes crash crossing architecture → Cache causes crash crossing architecture [@ nsDiskCacheRecord::HashNumber].[@ nsDiskCacheMap::FindRecord]
(Assignee)

Comment 2

11 years ago
Created attachment 210674 [details] [diff] [review]
Byte-swap the entire cache header

This is crashing because the nsDiskCacheHeader::mBucketUsage array is not being byte-swapped when read from/written to disk.  See Swap and Unswap:

http://lxr.mozilla.org/mozilla/source/netwerk/cache/src/nsDiskCacheMap.h#332

nsDiskCacheMap::FindRecord was then getting bad data for the number of buckets on a table entry, so attempts to get the buckets would fail.  Instead of records[1], it would look for records[16777216].  Kaboom.

http://lxr.mozilla.org/mozilla/source/netwerk/cache/src/nsDiskCacheMap.cpp#446

The cache would be marked dirty on first launch before crashing and therefore discarded on a subsequent launch.

The disk cache is supposed to be endian-neutral and stored on disk in big-endian format.  (Re)introducing proper byte-swapping renders all existing caches unusable on little-endian systems (for the above reason), so the disk cache version number is bumped.  I selected 1.9 for the trunk, reserving 1.8 for the same change on the branch - the trunk and branch are currently at 1.7 and 1.6, respectively.
Attachment #210674 - Flags: review?(darin)
(Assignee)

Comment 3

11 years ago
Created attachment 210675 [details] [diff] [review]
Branch version
QA Contact: profile-manager-backend → networking.cache

Comment 4

11 years ago
Comment on attachment 210674 [details] [diff] [review]
Byte-swap the entire cache header

r=darin
Attachment #210674 - Flags: review?(darin) → review+
(Assignee)

Updated

11 years ago
Attachment #210674 - Flags: superreview?(sfraser_bugs)

Updated

11 years ago
Attachment #210674 - Flags: superreview?(sfraser_bugs) → superreview+

Comment 5

11 years ago
mEvictionRank was never swapped, and mBucketUsage was introduced by me.
So, at least for the last one, my apologies for this crasher.
(Assignee)

Comment 6

11 years ago
mEvictionRank never hurt because it's never used for pointer arithmetic, only in comparisons.  It was wrong, but apparently nobody ever noticed.

Comment 7

11 years ago
Comment on attachment 210675 [details] [diff] [review]
Branch version

a=darin for the 1.8 branch
Attachment #210675 - Flags: branch-1.8.1+
(Assignee)

Comment 8

11 years ago
Fixed on trunk, MOZILLA_1_8_BRANCH, and CAMINO_1_0_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [camino-1.0]
(Assignee)

Comment 9

11 years ago
Comment on attachment 210675 [details] [diff] [review]
Branch version

I'd like this for 1.8.0.2.
Attachment #210675 - Flags: approval1.8.0.2?

Comment 10

11 years ago
It'd be nice if we could avoid discarding the cache for all FF 1.5 users.  Maybe some migration code is the answer, or perhaps we should make this change be XP_MACOSX only on the 1.8.0 branch.  Neither is a great solution, but tossing out 50 MB of cache for users behind a modem isn't very nice either.
(Assignee)

Comment 11

11 years ago
Created attachment 210720 [details] [diff] [review]
1.8.0 branch version

Considering that the only caches out there on Macs right now are big-endian, we can handle this with #ifdef XP_MACOSX and not bumping the cache version on the 1.8.0 branch.  The only people who will get shafted this way are those who used an unofficial Firefox 1.5 x86 build.
Attachment #210720 - Flags: review?(darin)
(Assignee)

Updated

11 years ago
Attachment #210675 - Flags: approval1.8.0.2?
(Assignee)

Comment 12

11 years ago
We can spin a new unofficial x86 Firefox build equivalent to 1.5.0.1 with this 1.8.0 patch, so users running the unofficials will crash first using THAT, and not with an official release.

(http://wiki.mozilla.org/Mac:Intel)

Updated

11 years ago
Attachment #210720 - Flags: review?(darin) → review+
(Assignee)

Updated

11 years ago
Attachment #210720 - Flags: approval1.8.0.2?
Flags: blocking1.8.0.2+
Comment on attachment 210720 [details] [diff] [review]
1.8.0 branch version

approved for 180 branch, a=dveditz for drivers
Attachment #210720 - Flags: approval1.8.0.2? → approval1.8.0.2+
(Assignee)

Comment 14

11 years ago
1.8.0 branch version checked in on 1.8.0 branch.
Keywords: fixed1.8.0.2
(Assignee)

Updated

11 years ago
Whiteboard: [camino-1.0] → [camino-1.0][camino-1.0.1]
Whiteboard: [camino-1.0][camino-1.0.1] → [camino-1.0][camino-1.0.1][camino-1.0.2]
(Assignee)

Updated

11 years ago
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3]
(Assignee)

Updated

10 years ago
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4]
(Assignee)

Updated

10 years ago
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5]
(Assignee)

Updated

10 years ago
Whiteboard: [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5] → [camino-1.0][camino-1.0.1][camino-1.0.2][camino-1.0.3][camino-1.0.4][camino-1.0.5][camino-1.0.6]
You need to log in before you can comment on or make changes to this bug.