...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<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.
(For the version of HarfBuzz optionally built as part of the LibreOffice build,
this has been addressed with
<https://git.libreoffice.org/core/+/6e53e03f752c2f85283c4d47efaaf0683299783c%5E!/>
"external/harfbuzz: hb_graphite2_cluster_t::advance can apparently be
negative.")
As of graphite2 1.3.7, `gr_make_face` is deprecated in favor of
`gr_make_face_with_ops`. It's a one-liner to port over to using it.
This is potentially a compatibility break since I'm not sure when the
`with_ops` API was added, but the minimum version of graphite2 that's
supported by Harfbuzz doesn't seem to be documented anywhere anyway.
This makes it possible to include all .cc files into build, even if not
building CoreText, Uniscribe, etc.
This was mostly to help custom builders. But also means that we can
include all files in our own build system. Not sure if we should.
Definitely simplifies things, but slightly only.
No other shaper will need shape_plan_data, by definition. So, remove
abstraction layer and always create hb_ot_shape_plan_t as part of
hb_shape_plan_t.
Stephan Bergmann
Khaled Hosny
Behdad Esfahbod
Peter Williams
n8willis
Evgeniy Reizner
Ebrahim Byagowi
Nathan Willis
Martin Hosken
David Corbett
mhosken
Bruce Mitchener
Khaled Hosny