Skip to content

[gl] avoid crash with system libftgl#22101

Open
ferdymercury wants to merge 1 commit intoroot-project:masterfrom
ferdymercury:patch-20
Open

[gl] avoid crash with system libftgl#22101
ferdymercury wants to merge 1 commit intoroot-project:masterfrom
ferdymercury:patch-20

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

@ferdymercury ferdymercury commented Apr 29, 2026

Fixes #22076

(Maybe) since ulrichard/ftgl@84869ec#diff-1462936246cba8b5e9ee1c79a1207cea8efb0658be58cd25e7d3acd817ac0ac0R374

so 2.1.3, there is a regression that does not correctly find symbols above code 191. So clamp the text to that max to avoid crashes, if we are using system-ftgl which is for most distributions 2.4 and for some older ones 2.1.3.

Do not clamp for builtin-ftgl since it was forked at 2.1.2, before the regression happened

More details in the linked issue.

fyi @osschar

All latex*.C scripts now run without crashing!

image

Fixes root-project#22076

(Maybe) since ulrichard/ftgl@84869ec#diff-1462936246cba8b5e9ee1c79a1207cea8efb0658be58cd25e7d3acd817ac0ac0R374

so 2.1.3, there is a regression that does not correctly find symbols above code 191. So clamp the text to that max to avoid crashes, if we are using system-ftgl which is for most distributions 2.4 and for some older ones 2.1.3.

Do not clamp for builtin-ftgl since it was forked at 2.1.2, before the regression happened

More details
@ferdymercury ferdymercury requested a review from couet as a code owner April 29, 2026 13:34
@ferdymercury ferdymercury requested a review from linev April 29, 2026 13:34
@github-actions
Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 10h 45m 59s ⏱️
 3 850 tests  3 849 ✅ 0 💤 1 ❌
76 948 runs  76 947 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit 49c6c9b.

@linev
Copy link
Copy Markdown
Member

linev commented Apr 30, 2026

PR goes in right direction, but there should be a way to use all symbols with external ftgl.
We knew that integral symbol fails. So we need to understand which glyph is used with built-in ftgl version.
And then try to find symbol which will give us same glyph with external ftgl.

And most probably such re-coding will be simple - like symb += 0x800.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

ferdymercury commented Apr 30, 2026

And most probably such re-coding will be simple - like symb += 0x800.

What do you think about ulrichard/ftgl@84869ec#diff-1462936246cba8b5e9ee1c79a1207cea8efb0658be58cd25e7d3acd817ac0ac0R374

Isn't the fact that it assumes UTF8 multibytes preventing us from doing such simple remapping that you propose?
The missing symbols (int epsilon etc) are exactly the ones they define as multibytes in their table. So I do not know how to fix that without modifying FTGL itself?

@linev
Copy link
Copy Markdown
Member

linev commented Apr 30, 2026

I am off until Monday, but then I will try to investigate this.
As I mentioned - I still did not understand difference between builtin and external ftgl.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

I still did not understand difference between builtin and external ftgl

Builtin (2.1.2) converts each char to one glyph in order.

External (2.1.3) converts each char to one glyph in order if char <192. If char between 192 and 192+16, then it reads also the next char at once and then does the conversion. If 192+32 it's three bytes it assumes.

That's why, when I just prevented nullptr access, I was seeing Int being converted to three random glyphs. So each char int, nabla or epsilon was getting two or three glyphs together, not just one. Probably accessing garbage memory.

@linev
Copy link
Copy Markdown
Member

linev commented Apr 30, 2026

External (2.1.3) converts each char to one glyph in order if char <192. If char between 192 and 192+16, then it reads also the next char at once and then does the conversion. If 192+32 it's three bytes it assumes.

But then solution is clear - one need to transform string so that final result lead to same set of glyph.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

ferdymercury commented Apr 30, 2026

one need to transform string so that final result lead to same set of glyph.

That proposal sounds good, but does not work. Let me explain with the simple example of the Nabla symbol, which is code 209 in symbols.ttf

With external ftgl, since 209 it's above code 191, it thinks it's the start of an UTF8 code, so it reads two more bytes (garbage), so we get three random glyphs drawn.

So you would propose to change the string so that it thinks it's a proper UTF8 code. I can do that using:

         TString utxt;
         const auto len = strlen(txt);
         for(auto i = 0UL; i < len; ++i) {
            if (static_cast<unsigned char>(txt[i]) == static_cast<unsigned char>(209)) { // Nabla is 209 in symbols.ttf and 0xE2 0x88 0x87 in UTF8
               utxt.Append(0xe2); //'\277');
               utxt.Append(0x88);
               utxt.Append(0x87);
            } else if .... else if ... else {
               utxt.Append(txt[i]);
            }
         }
         txt = utxt.Data();

Now, FTGL correctly parses the sequence of three bytes (chars). I debugged and it's assigning this three-byte sequence to an integer 8711. And this integer matches the number that HTML uses to encode Nabla, so great.

So far so good. Now the problem. Then it goes into this function:

unsigned int FTCharmap::FontIndex(const unsigned int characterCode)
{
    return FT_Get_Char_Index(ftFace, characterCode);
}

characterCode is 8711, but this is now a Freetype Library function, no longer FTGL. Since symbols.ttf only defines codes until 256, it does not find characterCode 8711, so he returns the Char Index 0. And then draws 0 (just a bounding box).

In other words, by looking whether a char is or not UTF8, FTGL is blinding itself to the region 192 to 256 of whatever ttf font we have. Remapping won't help.

Of course, one could say: let's extend symbols.ttf so that there is a duplicate glyph at characterCode 8711 and then it would find it. But that's overkill in my opinion.

To me it would make more sense to create FTGL 2.4.1, fix this cumbersome UTF8 identification, pass instead an argument saying whether input is UTF8 or not, and force users to use the builtin version. Then backport to ftgl fork and signal Debian. Otherwise the remapping is very ugly.
For users having older packages, we can fix the nullptr access as done here and print a warning if needed.
Remapping+Extending symbols.ttf looks to me much more complicated than adding an additional FTGL function parameter that avoids the UTF8 detection.

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

As expected, just changing this in external libftgl:

image

makes all issues go away:

image

So I vote for fixing this via a boolean switch argument in FTGL 2.4.1

and allowing buggy versions (>2.1.2 && <2.4.1) with static one-time warning, and no crash by clamping to 191.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GL pad painter crashes displaying symbols.ttf font when build with -Dbuiltin_ftgl=OFF

3 participants