Bug 600108 - fontforge crashes when building serafettin
fontforge crashes when building serafettin
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: fontforge (Show other bugs)
14
All Linux
low Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 599785
  Show dependency treegraph
 
Reported: 2010-06-03 19:43 EDT by Orcan Ogetbil
Modified: 2010-08-22 19:25 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-08-22 19:25:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
The backtrace (1.72 KB, application/octet-stream)
2010-06-03 19:43 EDT, Orcan Ogetbil
no flags Details
Single character from font that demonstrates problem (10.83 KB, text/plain)
2010-06-30 13:18 EDT, Paul Flo Williams
no flags Details
Patch to reveal failing glyphs (915 bytes, patch)
2010-07-28 04:32 EDT, Paul Flo Williams
no flags Details | Diff

  None (edit)
Description Orcan Ogetbil 2010-06-03 19:43:11 EDT
Description of problem:
Crash when building serafettin cartoon fonts

Version-Release number of selected component (if applicable):
fontforge-20100501-1.fc13.x86_64

How reproducible:
always

Steps to Reproduce:
wget http://downloads.sourceforge.net/serafettin/serafettin-cartoon-fonts-0.5.1.tar.bz2
tar jxvf serafettin-cartoon-fonts-0.5.1.tar.bz2
cd serafettin-cartoon-fonts-0.5.1
fontforge sfd2bold.ff serafettin-cartoon.sfd GPLv2+ 0.5.1
  
Actual results:
segfault. backtrace attached

Expected results:
build the font, or at least give some error if there is something wrong with the font sources

Additional info:
Upstream informed:
   http://sourceforge.net/mailarchive/forum.php?forum_name=fontforge-devel
See the thread: [Fontforge-devel] crash when building serafettin
Comment 1 Orcan Ogetbil 2010-06-03 19:43:50 EDT
Created attachment 421069 [details]
The backtrace
Comment 2 Paul Flo Williams 2010-06-30 05:15:41 EDT
Just a question and a few data points.

Firstly, was this a scratch build? This bug has been reported against a version that doesn't appear in the updates for F13.

Nevertheless, from the backtrace we see that the crash occurred in HideStrokePointsCircle(), which is in splinestroke.c.

I can confirm that Serafettin builds OK with FontForge 20090923 (i.e. the version currently in F13). The boldening algorithm throws a lot of warnings, but no crashes.

splinestroke.c was at version 1.46 at that time.

FontForge 20100501 included splinestroke.c version 1.56.

I can confirm oget's finding of a crash in this version. (Built direct from the tarball, as I couldn't find this RPM.)

A new algorithm for stroking splines had been introduced between the two releases (at splinestroke.c version 1.48), and the CVS log shows quite a few oopses being fixed since that time.

splinestroke.c is now at version 1.62 in CVS.

Unless someone else get there first, I'll try the current CVS version of FontForge tonight, but I wouldn't be surprised if this crash has already been fixed in CVS.
Comment 3 Paul Flo Williams 2010-06-30 13:18:59 EDT
Created attachment 428049 [details]
Single character from font that demonstrates problem

Today's pull from FontForge CVS still crashes with this font. I've now managed to isolate a single character from Serafettin that will reliably crash FontForge, and I've attached the SFD here.

The SFD contains just the right-hand half of the character U+0B94.
1. Open FontForge with this font
2. Select U+0B94
3. Select Expand Stroke

It seems that any combination of stroke width, line join and line cap will crash FontForge, including the default set.

I've sent another email to the upstream mailing list with the same attachment.
Comment 4 Kevin Fenzi 2010-07-04 15:08:07 EDT
Excellent work Paul. ;)
Comment 5 Orcan Ogetbil 2010-07-25 16:10:06 EDT
*sigh* this is still not fixed upstream. I confirmed that U+0B94 leads to a crash. However, I removed U+0B94 and all the other glpyhs that look like the right half of U+0B94. Fontforge still crashes. So there are other offending glyphs too. 

Paul, how did you trace it down to this glyph? I will just "fix" the other bad glyphs.
Comment 6 Paul Flo Williams 2010-07-26 07:41:07 EDT
To track down the glyph, I just changed FVStrokeItScript() to print the glyph name being processed at the time of the crash.

Since then, I've been tracking the changes in CVS and running serafettin's scripts against the font each time.

I hadn't tried to identify any other crashing glyphs because I was hoping that a fix for that one, when it's found, would apply to all. I haven't investigated why the new spline-stroking algorithm is failing because the overlap code is in flux in CVS and I'm hoping George will fix it before I hunt it down!

I can produce a list of the other bad glyphs if you like, but it won't be until tomorrow evening. (Feel free to email me for progress.) Or, I could start trying to fix the crash.
Comment 7 Paul Flo Williams 2010-07-28 04:32:36 EDT
Created attachment 434948 [details]
Patch to reveal failing glyphs

The list of failing glyphs is: cent, uni0B94, uni0BB0, uni0BB3, uni0BBE, uni0BC0, uni0BCC and uni0BD7.

Not all of these crash during all transformations, and some transformations don't produce any failures.

I've built FontForge from the 20100501 sources and patched the routine that crashes so that it will return prematurely on finding the error condition.

Note that the patch is *simply to stop FontForge crashing*. I have no idea whether this is part of the proper fix.

Orcan, if you have time, it would be worth seeing whether this patch produces usable fonts as an end result. If the affected glyphs look OK, then I'll nudge upstream to see if this patch is reasonable. If this breaks the affected glyphs, then this crash is simply the result of some fault elsewhere, and you may have to remove glyphs until upstream produces a proper fix.
Comment 8 Bug Zapper 2010-07-30 08:01:53 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 14 development cycle.
Changing version to '14'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 9 Orcan Ogetbil 2010-08-01 23:15:42 EDT
Sorry, I was buried under the python-2.7 rebuilds for the last few days. I'll get back to this bug over this week.
Comment 10 Paul Flo Williams 2010-08-02 16:55:22 EDT
I've had another good look at Serafettin and I can see why FontForge has such a hard time with it. If we validate:

Validation SerafettinCartoon ...Failed
  Self Intersecting Glyph
  Wrong Direction
  Flipped Reference
  Missing Points at Extrema
  More references in a glyph than specified in 'maxp'
  References nested more deeply than specified in 'maxp'
  Non integral coordinates in a glyph
  Two glyphs have the same name.
  Two glyphs have the unicode.
  Overlapping hints in a glyph.

There are several interesting lines here, but the most important for this bug appears to be "self intersecting glyph". If you go into FontForge and validate, you'll find a lot of glyphs that don't appear to be self-intersecting, but if you look at the SFD source, it will be clear that there are a lot of points on top of one another. These don't have any effect on the drawn outline, but they mess with FontForge's ability to determine the direction of each part of the contour when stroking the lines in order to expand or contract them.

Long story short, run the base font through FontForge and clean up the glyphs. For both serafettin-cartoon.sfd and serafettin-cartoon-italic.sfd:
1. Edit -> Select -> Select All
2. Element -> Simplify -> Cleanup Glyph
3. Go to glyph "cent" (the second one with this name, U+0BBF). Remove dots that are disconnected from main glyph
4. Go to glyph uni0BC0. Do same as above (there are two of them here).

This should allow Serafettin to build. (Works for me with upstream CVS head, haven't tried Rawhide).

There are several glyphs with the same name in this font as well, and a few that are mapped to the same Unicode point. I don't understand how this came about, but they will need correcting too.

Kevin: FontForge may be fragile when dealing with this font, but in fairness it *did* produce a big bunch of relevant warnings! I will send the relevant line of my patch upstream for the sake of robustness.
Comment 11 Orcan Ogetbil 2010-08-02 18:25:57 EDT
Oh geez. Thanks for all your work.

Honestly, I don't want to deal with a similar problem on the next fontforge update. Maybe I should just drop those glyphs. I kept them in when I forked this font from another project, just to have more coverage, but I can't even support them since I don't even read that language. Maybe if I have a co-author in the future, who wants to support those glyphs, we can add them back.
Comment 12 Orcan Ogetbil 2010-08-08 22:58:16 EDT
Hey Paul, if you really want to fix those glyphs, be my guest. I have committed the sources to sourceforge svn:
   http://serafettin.svn.sourceforge.net/viewvc/serafettin/
Do you have a sourceforge account? If so, give me your username, so I can grant you svn write access.

If you think it is too much work, let me know, and I will remove those glyphs.
Comment 13 Orcan Ogetbil 2010-08-22 16:01:21 EDT
I released serafettin-cartoon-fonts-0.6 with Paul's clean-ups and built RPMs for F-12, F-13 and F-14. Now to build serafettin, I don't need this fontforge bug fixed anymore. 

Shall we keep this open or close it WONTFIX?
Comment 14 Paul Flo Williams 2010-08-22 16:17:09 EDT
There is a single line change that can stop FontForge from crashing that I have still not submitted upstream because I'm not sure whether it's the right fix. However, I will do that (I know, second time I've said that.)

With Serafettin fixed, we have no means of reproducing this in Fedora and I suggest WONTFIX, as some form of the patch will appear upstream later. (If it occurs again in Fedora before that time, then we can carry the patch locally.)
Comment 15 Kevin Fenzi 2010-08-22 19:25:26 EDT
I'd be fine with closing it now... if it re-occurs we can reopen or file a new one. 

Thanks guys!

Note You need to log in before you can comment on or make changes to this bug.