[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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- 8cee1dde4e708b1d4a9f028f3ac6cca99495d729 should apply to Ins_CALL too?,
Hin-Tak Leung <=