mupdf fails to build with ubsan (multiple errors)

[inferno-chromium]

• python infra/helper.py build_image mupdf
• python infra/helper.py build_fuzzers --sanitizer undefined mupdf

Output:
Error 1:
clang-6.0: error: invalid argument '-fsanitize=vptr' not allowed with '-fno-rtti'
Makethird:167: recipe for target '/work/thirdparty/harfbuzz/hb-fallback-shape.o' failed
make: *** [/work/thirdparty/harfbuzz/hb-fallback-shape.o] Error 1
make: *** Waiting for unfinished jobs....

Error 2:
scripts/../source/fitz/context.c:208:38: runtime error: index -1 out of bounds for type 'fz_error_stack_slot [256]'
Makefile:263: recipe for target 'generated/pdf-cmap-extra.c' failed
make: *** [generated/pdf-cmap-extra.c] Error 1

@titanous @alex @cx - i am disabling ubsan until you guys figure out fixes for these.

[titanous]

I believe this will require tweaking the build system to avoid these errors.

/cc @sebras @ccxvii

[inferno-chromium]

atleast mupdf build is green for now, it will build and test with asan and msan configs for now. keeping this bug open so that you can fix ubsan.

if rtti can't be enabled, you can disable vptr as https://github.com/google/oss-fuzz/blob/master/projects/ffmpeg/build.sh#L18. but the other bug of index out of bounds needs to be fixed.

[sebras]

Seems like we have disabled rtti because harfbuzz doesn't rely on it. And were not regularly running with ubsan so this is not something we've encountered before.

[alex]

Removing the disable rtti flag when ubsan is enabled seems reasonable, and hopefully not burdensome

[inferno-chromium]

Yes right, harfbuzz has vptr disabled as well - https://github.com/google/oss-fuzz/blob/master/projects/harfbuzz/build.sh#L19. So, it is fine to disable it for you as well for now. +cc @behdad - is it fixable in harfbuzz so that then we can enable vptr for both harfbuzz and then its dependent project mupdf.

[inferno-chromium]

Once that out-of-bounds error is fixed, please submit a pull request to reenable ubsan and disable vptr.

[sebras]

Is this behaviour new to clang-6.0? I tried to build using clang-4.0.1 and clang-5.0.1 but those builds didn't fail for me.

[inferno-chromium]

could be (since it can have new ubsan signatures), try using latest image using "python infra/helper.py build_image mupdf" and say yes to pull latest image.

[inferno-chromium]

@sebras @ccxvii - can you try with step from my last comment.

[inferno-chromium]

Closing, please feel free to fix your ubsan build and later submit pull request to enable ubsan.

[behdad]

@behdad - is it fixable in harfbuzz so that then we can enable vptr for both harfbuzz and then its dependent project mupdf.

No. We don't want HarfBuzz to link to libstdc++. If you want, I can add a configure option to do that and not pass -fno-rtti to compiler.

[behdad]

cc @ebraminio

[inferno-chromium]

@behdad - a new configure option should work well for both harfbuzz and mupdf builds in oss-fuzz. can you send a pull request for harfbuzz build to use it (and enable ubsan in project.yaml) and then mupdf folks can do it in similar way.

[behdad]

@behdad - a new configure option should work well for both harfbuzz and mupdf builds in oss-fuzz. can you send a pull request for harfbuzz build to use it (and enable ubsan in project.yaml) and then mupdf folks can do it in similar way.

Sure. @ebraminio, is this somehting you can do please? Perhaps --with-stdc++.

[ebraminio]

Sure. @ebraminio, is this somehting you can do please? Perhaps --with-stdc++.

Sure, harfbuzz/harfbuzz#770 feel free to give me a hand :)

[ebraminio]

Also we love to help our users as far as we can and just recently we contacted @robinwatts here about some other issue.

Now that all of you mupdf people are present here, I like to mention however your use of harfbuzz doesn't feel that correct, you have used harfbuzz for typesetting but rest of a complete text rendering stack apparently doesn't exist on mupdf such as bidi/line-breaking/fallback such. Perhaps I can avoid derailing this discussion to unrelated topic just by providing some links, 1 2 3 4.

[ebraminio]

With autotools, now you can use --with-stdcpp option.

[ccxvii]

@ebraminio, we only use Harfbuzz for text layout in EPUB documents. In our EPUB layout we have bidi, line breaking and font fallbacks (using our own code, no external third party libraries). PDF and XPS and other document formats have completely different requirements and invoking harfbuzz layout for that would not behave like their specifications need. Now, that said, we probably have issues and bugs that we're blind to given the nature of the problem, and would appreciate someone with an eye for these non-western scripts (especially RTL) to find and help point out bugs in the implementation. If you want to talk more about this, the best place to contact the mupdf developers is on the IRC channel #mupdf on freenode.

[ebraminio]

I didn't use it but was wondering how it is working without any third party libraries. Sounds cool! Thanks

[ebraminio]

It changed to --with-libstdc++ later harfbuzz/harfbuzz#782