Open Bug 1367146 (ubsan) Opened 4 years ago Updated 4 days ago
[meta] UBSAN errors
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36 Steps to reproduce: Build trunk with -fsanitize=undefined. It's a follow up of: https://bugzilla.mozilla.org/show_bug.cgi?id=1165904 and it will probably need to split it by components.
This is related to this SuSE bug: https://bugzilla.opensuse.org/show_bug.cgi?id=1040105 Some of these errors may be causing Firefox not to work when built with newer GCC versions, particularly with -O3 and autovectorization (alignment bugs...)
Naveed - wasn't sure it should go here. If not, please provide some guidance on how to handle this.
Summary: UBSAN errors → [meta] UBSAN errors
Priority: -- → P3
Core::General is a reasonable place for this because the complains are all over the codebase. UBSAN reports are a mixed bag. Some of them are useful, but it tends to produce lots of complains about things that don't seem important. E.g. see bug 996026 comment 1. I've fixed a couple of problems from the attachment in dependent bugs. A lot of the remainder are alignment issues, which seem harmless, but there are a couple of non-alignment problems that look more interesting. However, the log is now a few months old and many of the line numbers are out-of-date. Martin, if it's not hard for you, would you mind doing another run and posting updated results, along with the revision that you used? Thank you.
I'm sending updated UBSAN log for revision: commit 519bb0922bcfe5f0779ef8ae2518f959aac09448 (origin/master, origin/HEAD) Merge: 1e48981df6a1 bcef780a7796 Author: Wes Kocher <firstname.lastname@example.org> Date: Mon Sep 18 16:21:01 2017 -0700 Merge inbound to central, a=merge MozReview-Commit-ID: EK8iFR1hSRp
Filed https://ssl.icu-project.org/trac/ticket/13362 for the UBSan errors in the imported ICU library.
Thanks Andree for taking that seriously. Tomorrow I'll send also updated version for ASAN. Anyhow, I really believe that project like Fifefox should build such builds periodically and run test-suite with them. I can help with question related to that.
> commit 519bb0922bcfe5f0779ef8ae2518f959aac09448 (origin/master, origin/HEAD) > Merge: 1e48981df6a1 bcef780a7796 > Author: Wes Kocher <email@example.com> > Date: Mon Sep 18 16:21:01 2017 -0700 > > Merge inbound to central, a=merge > > MozReview-Commit-ID: EK8iFR1hSRp That looks like a revision from a git repo? On mozilla-inbound this is revision 30a386ff1192.
I went through the "Current UBSAN log", removed duplicates, added some comments, and sorted them in rough order of interest.
Martin, a few more questions: - How did you build Firefox? - How did you run Firefox, and what workload did you exercise it on? - The original log had lots of complaints about misaligned addresses. Do you know why they aren't present in the latest log? Thank you.
I build it with: $ cat .mozconfig # Build Firefox with: OPT="-O2" MYFLAGS="-flto=9" make -f client.mk build mk_add_options MOZ_MAKE_FLAGS="-j9" MYFLAGS="$OPT $MYFLAGS" mk_add_options OS_CFLAGS="$MYFLAGS" mk_add_options OS_CXXFLAGS="$MYFLAGS" mk_add_options OS_LDFLAGS="$MYFLAGS" ac_add_options --enable-optimize=$OPT ac_add_options --enable-application=browser ac_add_options --disable-tests ac_add_options --disable-debug ac_add_options --disable-debug-symbols # ac_add_options --enable-system-icu # valgrind purpose ac_add_options --disable-jemalloc ac_add_options --disable-valgrind # ac_add_options --disable-ogg # ac_add_options --with-system-libvpx # ac_add_options --disable-necko-wifi ac_add_options --disable-elf-hack export CFLAGS="$MYFLAGS" export CXXFLAGS="$MYFLAGS" export LDFLAGS="$MYFLAGS" $ OPT="-O2" MYFLAGS="-fsanitize=undefined -ldl -flifetime-dse=1 -fno-sanitize=alignment" nice make -f client.mk build ... I basically visit couple of pages and I run http://browserbench.org/JetStream/. It's missing because I disabled this kind of errors.
Thank you for the info. The updated log is actually pretty good -- there are several things I plan to investigate, and not too much in the way of annoying or silly stuff.
I just saw that there is an existing bug for UBSan issues in SpiderMonkey (bug 1284975), I've added an additional list of UBSan errors when running the SpiderMonkey test suites to that bug report.
About this one: > /home/marxin/Programming/gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/mozilla/ipc/ProtocolUtils.h:626:22: runtime error: load of value 32764, which is not a valid value for type 'Mode' It's a false positive, more or less. Specifically, it's true that EndPoint::mMode is set to an invalid value, but there's another field EndPoint::mValid which is always false when that happens, and mMode is never read while mValid is false. One option is to set mMode in the constructor even when mValid is false, which should satisfy UBSan. The downside of this is that it will prevent Valgrind from detecting any use of mMode before it's set properly.
About this one: > /home/marxin/Programming/gecko-dev/gfx/layers/apz/util/InputAPZContext.cpp:58:18: runtime error: load of value 2463969997, which is not a valid value for type 'nsEventStatus' Again, looks like a false positive. This code calls the constructor with an uninitialized argument: http://searchfox.org/mozilla-central/rev/2ef8bd8a46a02c68ddbb1d5f25fa254dd7be1fbd/dom/ipc/TabChild.cpp#1684-1685 but that argument appears to never be used. Valgrind doesn't complain about uninitialized values until they are used in a way that could affect program execution. Unfortunately UBSan complains about some such values simply when they are copied, which is a recipe for false positives.
Should we add a MOZ_UBSAN_SILENCE("type") that will suppress the items you've identified as false positives? I think I can handle that for the couple you've noted in #14 and #15 https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#disabling-instrumentation-with-attribute-no-sanitize-undefined
(In reply to Tom Ritter [:tjr] from comment #16) > Should we add a MOZ_UBSAN_SILENCE("type") that will suppress the items > you've identified as false positives? I think I can handle that for the > couple you've noted in #14 and #15 That sounds like a good idea -- it will placate UBSan in a way that doesn't reduce Valgrind's effectiveness.
3 years ago
Depends on: 1402316
Agree that MOZ_UBSAN_SILENCE is a good idea, however note that GCC only supports that on functions, not on types or fields of a type.
You need to log in before you can comment on or make changes to this bug.