342482 - extension manager fails to recognize any extensions

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Shane Caraveo]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4

On OS X x86, MOZILLA_1_8_BRANCH (both current and going back as far as 5/11/2006), the extension manager is failing to load any of the installed extensions listed in extensions.ini.  

I've tracked this down to nsLocalFile::SetPersistentDescriptor which fails when validating the size of the data (see the comment "be paranoid about  whichhaving too few data"). 

I've forced the extension manager to recreate extensions.ini (by deleting extensions.* in the profile).  I also verified that what nsLocalFile::GetPersistentDescriptor generates is in fact what is in extensions.ini.

Commenting out that size verification causes the extensions to get properly loaded.

This is in Komodo, and our extensions load fine on OSX PPC, Linux and Windows.


Reproducible: Always

Comment 1

[Shane Caraveo]

cc'ing Josh since this is x86 osx only

Comment 2

[Josh Aas]

I remember hearing some comments from the last WWDC about how AliasRecord or something closely related to it was a common x86 transition problem. Unfortunately I don't remember what was said about it, I'll have to dig it up.

Comment 3

[Josh Aas]

I was thinking of the fact that alias records are now opaque (they weren't before Intel). Not the problem here.

On all systems, x86 or PPC, aliases are big-endian. Maybe we need to be byte swapping our alias data before or after we decode it... I haven't looked into the code yet.

Comment 4

[Josh Aas]

Shane - do you have a testcase for reproducing this with Firefox? Otherwise, can you give me the values for "aliasSize" and "dataSize" at this line:

if (aliasSize > (dataSize * 3) / 4) { // be paranoid about having too few data

Comment 5

[Shane Caraveo]

I added a debug print:

  PRInt32 aliasSize = GetAliasSizeFromRecord(aliasHeader);
  double maxsize = (dataSize * 3) / 4;
  fprintf(stderr, "aliasSize %d should be <= to %.2f\n", aliasSize, maxsize);

and get output like:

nsLocalFile::SetPersistentDescriptor...
aliasSize 41986 should be <= to 678

If I change the define for GetAliasSizeFromRecord from:

#if !defined(MAC_OS_X_VERSION_10_4) || MAC_OS_X_VERSION_MIN_REQUIRED < MAC_OS_X_VERSION_10_4
#define GetAliasSizeFromRecord(aliasRecord) aliasRecord.aliasSize
#else
#define GetAliasSizeFromRecord(aliasRecord) GetAliasSizeFromPtr(&aliasRecord)
#endif

to:

#define GetAliasSizeFromRecord(aliasRecord) GetAliasSizeFromPtr(&aliasRecord)

it works correctly.

Comment 6

[Shane Caraveo]

perhaps it has something to do with this in our .mozconfig:

ac_add_options --enable-macos-target=10.3

Comment 7

[Mark Mentovai]

You shouldn't (mustn't!) set the DT below 10.4 on x86.

Comment 8

[Josh Aas]

Attachment: fix v1.0

The problem is that if you're building on Intel but targeting Mac OS X 10.3 then you use the .aliasSize field instead of the accessor function. Since .aliasSize should work on all PPC systems and the accessor is only really necessary on Intel systems, we can just make the #ifdef for ppc systems and ignore target OS altogether.

Or even better we could do that same OS target test but also require us to be building on PPC systems in order for using .aliasSize to be considered. That is what this patch does.

Comment 9

[Shane Caraveo]

ok, verified patch builds and works on PPC 10.3 and x86 10.4
thanks!

Comment 10

[Mark Mentovai]

Comment on attachment 228330 [details] [diff] [review]
fix v1.0

This check shouldn't be needed, and it might not be the only part of our code we'd need to apply such a fix to.  The right thing to do is warn or give an error at configure time if someone's targeting x86 with the DT set below 10.4.

Comment 11

[Josh Aas]

I agree that we should add a warning about your target OS on Intel, but Shane's setup seems to be far from completely horked even with the bad OS target. Why not take out one major problem with a bad config? Its not like when users run into this problem they realize that their config is wrong - they just get wonder if the bug is in our XPCOM io code, exactly what happened in this bug. They are just as likely to file a bug like this or simply remove the problematic code as they are to actually fix their config.

Comment 12

[Josh Aas]

A bad config warning is not enough because if the user misses it and runs into this problem or something like it, they will just get confused and we'll go through this process all over again. I think we should either take this patch and make a warning, or make it a hard error to configure on Intel with a bad target OS. I prefer the latter.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

We won't block the final release of Firefox 2 for this, but would take an approved and baked patch (nominate for approval1.8.1) in the beta2 timeframe in order to prevent frustration from people using our platform.

Comment 14

[Josh Aas]

-> mark for writing a hard error in our configure code

Comment 15

[Dave Townsend [:mossop]]

This seems like a core issue to me

Comment 16

[Todd Whiteman]

Expired.