474758 - [@ FcFontSetSort] crashes on Solaris

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Ginn Chen]

FcFontSetSort (FcConfig *config, FcFontSet **sets, intnsets, FcPattern *pattern, FcBool trim, FcCharSet **csp, FcResult *result);

config is not used on Linux, but it is used on Solaris.

And it will crash if it is NULL.

I think it is a bug of font config lib on Solaris, but we need to workaround, otherwise Firefox could not be launched with current font config lib on Solaris.

Comment 1

[Ginn Chen]

Attachment: patch

Comment 2

[Karl Tomlinson (:karlt)]

Ginn, do you know what Solaris's FcFontSetSort uses the FcConfig for?
If you are able to point me at the source, that would be most helpful.

If you don't have access to source, is the crash in FcFontSetSort itself or in a function that it uses, and so are there symbol names that might give hints as to what the FcConfig is being used for?

(The use of the FcConfig makes me wonder whether Solaris's FcFontSetSort is doing the same thing as the Keith Packard's freedesktop version, and whether it is suitable for the purpose here.)

Also, does OpenSolaris use freedesktop's fontconfig?
And is SOLARIS defined as a preprocessor symbol on OpenSolaris?

Comment 3

[Ginn Chen]

FcFontSetSort() is calling FcHideGetList(), FcHideGetList() is inlined so the crash is in FcFontSetSort()

The source is at
http://cvs.opensolaris.org/source/xref/fox/fox-gate/XW_NV/open-src/lib/fontconfig/fontconfig-2.5.0.patch.hide

But it is a patch, so it's not clear.
The problem is in this part.
    354 @@ -746,6 +927,9 @@
    355      
    356      new = nodes;
    357      nodep = nodeps;
    358 +
    359 +    r = FcHideGetList ( config, p );
    360 +
    361      for (set = 0; set < nsets; set++)
    362      {
    363  	s = sets[set];

It didn't null check config, FcHideGetList didn't, either.

Solaris and OpenSolaris use the same fontconfig, I think it's based on freedesktop's fontconfig.

SOLARIS is defined by mozilla-central/configure.in

Comment 4

[Karl Tomlinson (:karlt)]

Thanks for that info.  Solaris's FcFontSetSort does seem to be based on freedesktop's fontconfig, and will do the right thing.

The patch comes from here:
https://bugs.freedesktop.org/show_bug.cgi?id=7528

Now that fontconfig match elements can take target="scan", much of the functionality can be achieved in that way (though with less elegantly).

It looks like, if the more elegant editing functionality is ever added to fontconfig, it will be done by allowing editing of the language through <match target="scan"/> rather than at sort time.

This makes me reasonably confident that other OS's are unlikely to apply this patch, so the "SOLARIS" specific change seems the right thing to do.

Comment 5

[Karl Tomlinson (:karlt)]

Comment on attachment 358154 [details] [diff] [review]
patch

+    // Get around a crash of FcFontSetSort when FcConfig is NULL

Can you include a reference to this bug please?  A comment such as "Solaris's FcFontSetSort needs an FcConfig (bug 474758)" would be sufficient.

Comment 6

[Alan Coopersmith]

(In reply to comment #3)
> Solaris and OpenSolaris use the same fontconfig, I think it's based on
> freedesktop's fontconfig.

Right - Solaris 10, Solaris Express/Nevada and OpenSolaris all include
fontconfig 2.5.0 from freedesktop.org, with a couple patches applied as
you've found.

Comment 7

[Ginn Chen]

Pushed:

http://hg.mozilla.org/mozilla-central/rev/365b1102c67b

Comment 8

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

Comment on attachment 358154 [details] [diff] [review]
patch

a191=beltzner

Comment 9

[Ginn Chen]

Pushed to 1.9.1:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f41051e3b0db