From 14b018124c52da6c7bc8ab93a2c571ca2e8c2f80 Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Mon, 9 Aug 2021 17:17:48 +0200 Subject: [PATCH] hb_graphite2_cluster_t::advance can apparently be negative ...as seen with HarfBuzz used by LibreOffice, with `instdir/program/soffice --headless --convert-to pdf` of doc/abi6073-2.doc from the LibreOffice 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 >, 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::__debug::map, std::allocator > >, 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, 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. (For the version of HarfBuzz optionally built as part of the LibreOffice build, this has been addressed with "external/harfbuzz: hb_graphite2_cluster_t::advance can apparently be negative.") --- src/hb-graphite2.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-graphite2.cc b/src/hb-graphite2.cc index 15d6ce80b..9d2252b84 100644 --- a/src/hb-graphite2.cc +++ b/src/hb-graphite2.cc @@ -223,7 +223,7 @@ struct hb_graphite2_cluster_t { unsigned int base_glyph; unsigned int num_glyphs; unsigned int cluster; - unsigned int advance; + int advance; }; hb_bool_t