freetype-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

8cee1dde4e708b1d4a9f028f3ac6cca99495d729 should apply to Ins_CALL too?


From: Hin-Tak Leung
Subject: 8cee1dde4e708b1d4a9f028f3ac6cca99495d729 should apply to Ins_CALL too?
Date: Thu, 27 Apr 2023 00:10:20 +0000 (UTC)

Hi Werner and etc,

There is an addition to the fontval backend which has been sitting in my disk 
for a while. I periodically cherry-pick interesting changes from current master 
when I do fontval releases, and there was this change: 


commit 8cee1dde4e708b1d4a9f028f3ac6cca99495d729
Author: Dominik Röttsches <drott@chromium.org>
Date:   Tue Dec 17 14:12:38 2019 +0200

    Fix more UBSan warnings on adding offset to nullptr (#57432).
    
    * src/truetype/ttinterp.c (Ins_LOOPCALL), src/psaux/psft.c
    (cf2_initLocalRegionBuffer): Use `FT_OFFSET'.



The interesting aspect about this change is that, because Ins_LOOPCALL() is so 
similar to Ins_CALL() , if you cherry-pick it across branches which diff by 
large line number offsets (like how FontVal diffs from upstream... differing by 
over 1000 lines in some files now! :-)...) , and then rebase, after a while, 
8cee1dde4e708b1d4a9f028f3ac6cca99495d729 could be "mis-applied" to Ins_CALL() 
instead of where it is intended, Ins_LOOPCALL() .

I think I found this out when doing 4-ways diffs - as in FontVal-x against 
freetype-x, FontVal-y against freetype-y, then diff'ing the two diffs.

Anyway, it looks like it is a generally good change - as in, it protects 
against possible crashes, with a small costs in speed if you have to argue 
about that ... - so I have been adding that change to Ins_CALL() also.

I don't feel too strongly about it either way, but I think the part of 
8cee1dde4e708b1d4a9f028f3ac6cca99495d729 to Ins_LOOPCALL() should also apply to 
Ins_CALL() too, if only for uniformity.

Hin-Tak


reply via email to

[Prev in Thread] Current Thread [Next in Thread]