[Libreoffice-commits] core.git: external/harfbuzz

[Stephan Bergmann (via logerrit)]

 external/harfbuzz/UnpackedTarball_harfbuzz.mk |    4 ++++
 external/harfbuzz/negativeadvance.patch       |   11 +++++++++++
 2 files changed, 15 insertions(+)

New commits:
commit 6e53e03f752c2f85283c4d47efaaf0683299783c
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Mon Aug 9 08:11:40 2021 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Aug 9 11:39:05 2021 +0200

    external/harfbuzz: hb_graphite2_cluster_t::advance can apparently be negative
    
    ...as seen with `instdir/program/soffice --headless --convert-to pdf` of
    doc/abi6073-2.doc from the crash-testing corpus when run under UBSan,
    
    > hb-graphite2.cc:361:15: runtime error: -1024 is outside the range of representable values of type 'unsigned int'
    >  #0 in _hb_graphite2_shape at workdir/UnpackedTarball/harfbuzz/src/hb-graphite2.cc:361:15
    >  #1 in _hb_shape_plan_execute_internal(hb_shape_plan_t*, hb_font_t*, hb_buffer_t*, hb_feature_t const*, unsigned int) at workdir/UnpackedTarball/harfbuzz/src/./hb-shaper-list.hh:38:1
    >  #2 in hb_shape_plan_execute at workdir/UnpackedTarball/harfbuzz/src/hb-shape-plan.cc:453:14
    >  #3 in hb_shape_full at workdir/UnpackedTarball/harfbuzz/src/hb-shape.cc:139:19
    >  #4 in GenericSalLayout::LayoutText(ImplLayoutArgs&, SalLayoutGlyphsImpl const*) at vcl/source/gdi/CommonSalLayout.cxx:495:23
    >  #5 in OutputDevice::getFallbackLayout(LogicalFontInstance*, int, ImplLayoutArgs&, SalLayoutGlyphs const*) const at vcl/source/outdev/font.cxx:1232:21
    >  #6 in OutputDevice::ImplGlyphFallbackLayout(std::unique_ptr<SalLayout, std::default_delete<SalLayout> >, ImplLayoutArgs&, SalLayoutGlyphs const*) const at vcl/source/outdev/font.cxx:1300:48
    >  #7 in OutputDevice::ImplLayout(rtl::OUString const&, int, int, Point const&, long, long const*, SalLayoutFlags, vcl::TextLayoutCache const*, SalLayoutGlyphs const*) const at vcl/source/outdev/text.cxx:1332:22
    >  #8 in lcl_CreateLayout(SwTextGlyphsKey const&, __gnu_debug::_Safe_iterator<std::_Rb_tree_iterator<std::pair<SwTextGlyphsKey const, SwTextGlyphsData> >, std::__debug::map<SwTextGlyphsKey, SwTextGlyphsData, std::less<SwTextGlyphsKey>, std::allocator<std::pair<SwTextGlyphsKey const, SwTextGlyphsData> > >, std::bidirectional_iterator_tag>) at sw/source/core/txtnode/fntcache.cxx:233:33
    >  #9 in SwFntObj::GetCachedSalLayoutGlyphs(SwTextGlyphsKey const&) at sw/source/core/txtnode/fntcache.cxx:257:12
    >  #10 in SwFont::GetTextBreak(SwDrawTextInfo const&, long) at sw/source/core/txtnode/fntcache.cxx:2551:58
    >  #11 in SwTextSizeInfo::GetTextBreak(long, o3tl::strong_int<int, Tag_TextFrameIndex>, unsigned short, vcl::TextLayoutCache const*) const at sw/source/core/text/inftxt.cxx:450:20
    >  #12 in SwTextGuess::Guess(SwTextPortion const&, SwTextFormatInfo&, unsigned short) at sw/source/core/text/guess.cxx:205:26
    >  #13 in SwTextPortion::Format_(SwTextFormatInfo&) at sw/source/core/text/portxt.cxx:305:32
    >  #14 in SwTextPortion::Format(SwTextFormatInfo&) at sw/source/core/text/portxt.cxx:456:12
    >  #15 in SwLineLayout::Format(SwTextFormatInfo&) at sw/source/core/text/porlay.cxx:260:31
    
    (where in frame #4 GenericSalLayout::LayoutText, pHbBuffer->props.direction is
    HB_DIRECTION_RTL, in case that is relevant).
    
    It is unclear to me whether it is sufficient to only change
    hb_graphite2_cluster_t::advance from signed to unsigned int, as there are other
    unsigned int variables (like curradv in _hb_graphite2_shape) whose value depend
    on hb_graphite2_cluster_t::advance, and which thus might also become negative.
    But unlike the float -> unsigned int conversion that UBSan warned about here
    (where gr_slot_origin_X() and xscale are float), those are signed int ->
    unsigned int conversions that do not cause undefined behavior.  At least, with
    this change, the above --convert-to pdf and a full `make check screenshot`
    succeeded for me under without further UBSan warnings.
    
    Change-Id: Ifa6fa930da162b986d3f536f8b3613790b3f19c8
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/120192
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/external/harfbuzz/UnpackedTarball_harfbuzz.mk b/external/harfbuzz/UnpackedTarball_harfbuzz.mk
index a99f116d80ad..13400700606a 100644
--- a/external/harfbuzz/UnpackedTarball_harfbuzz.mk
+++ b/external/harfbuzz/UnpackedTarball_harfbuzz.mk
@@ -15,4 +15,8 @@ $(eval $(call gb_UnpackedTarball_update_autoconf_configs,harfbuzz))
 
 $(eval $(call gb_UnpackedTarball_set_patchlevel,harfbuzz,0))
 
+$(eval $(call gb_UnpackedTarball_add_patches,harfbuzz, \
+    external/harfbuzz/negativeadvance.patch \
+))
+
 # vim: set noet sw=4 ts=4:
diff --git a/external/harfbuzz/negativeadvance.patch b/external/harfbuzz/negativeadvance.patch
new file mode 100644
index 000000000000..ab19aa2631e0
--- /dev/null
+++ b/external/harfbuzz/negativeadvance.patch
@@ -0,0 +1,11 @@
+--- src/hb-graphite2.cc
++++ src/hb-graphite2.cc
+@@ -223,7 +223,7 @@
+   unsigned int base_glyph;
+   unsigned int num_glyphs;
+   unsigned int cluster;
+-  unsigned int advance;
++  int advance;
+ };
+ 
+ hb_bool_t