Open
Bug 1367146
(ubsan)
Opened 8 years ago
Updated 2 years ago
[meta] UBSAN errors
Categories
(Core :: Sanitizers, task, P3)
Core
Sanitizers
Tracking
()
UNCONFIRMED
People
(Reporter: mliska, Unassigned)
References
(Depends on 51 open bugs)
Details
(Keywords: meta, Whiteboard: [js:tech-debt])
Attachments
(3 files)
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.
Comment 1•7 years ago
|
||
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...)
Comment 2•7 years ago
|
||
Naveed - wasn't sure it should go here. If not, please provide some guidance on how to handle this.
Component: General → JavaScript Engine
Flags: needinfo?(nihsanullah)
Summary: UBSAN errors → [meta] UBSAN errors
Comment 3•7 years ago
|
||
The log appears to be for all of gecko, no just JavaScript.
Most of what I see are alignment issues that could be performance hazards. This may be worth a more detailed look to see if there are other hazards lurking.
Flags: needinfo?(nihsanullah)
Priority: -- → P3
Whiteboard: [js:tech-debt]
Comment 4•7 years ago
|
||
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.
Component: JavaScript Engine → General
Flags: needinfo?(mliska)
Reporter | ||
Comment 5•7 years ago
|
||
I'm sending updated UBSAN log for revision:
commit 519bb0922bcfe5f0779ef8ae2518f959aac09448 (origin/master, origin/HEAD)
Merge: 1e48981df6a1 bcef780a7796
Author: Wes Kocher <wkocher@mozilla.com>
Date: Mon Sep 18 16:21:01 2017 -0700
Merge inbound to central, a=merge
MozReview-Commit-ID: EK8iFR1hSRp
Flags: needinfo?(mliska)
Comment 6•7 years ago
|
||
Filed https://ssl.icu-project.org/trac/ticket/13362 for the UBSan errors in the imported ICU library.
Reporter | ||
Comment 7•7 years ago
|
||
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.
Comment 8•7 years ago
|
||
> commit 519bb0922bcfe5f0779ef8ae2518f959aac09448 (origin/master, origin/HEAD)
> Merge: 1e48981df6a1 bcef780a7796
> Author: Wes Kocher <wkocher@mozilla.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.
Comment 9•7 years ago
|
||
I went through the "Current UBSAN log", removed duplicates, added some comments, and sorted them in rough order of interest.
Comment 10•7 years ago
|
||
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.
Flags: needinfo?(mliska)
Reporter | ||
Comment 11•7 years ago
|
||
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.
Flags: needinfo?(mliska)
Comment 12•7 years ago
|
||
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.
Comment 13•7 years ago
|
||
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.
Comment 14•7 years ago
|
||
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.
Comment 15•7 years ago
|
||
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.
Comment 16•7 years ago
|
||
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
Flags: needinfo?(n.nethercote)
Comment 17•7 years ago
|
||
(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.
Flags: needinfo?(n.nethercote)
Reporter | ||
Comment 18•7 years ago
|
||
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.
Updated•7 years ago
|
Alias: ubsan
Updated•6 years ago
|
Type: defect → task
Updated•5 years ago
|
Updated•5 years ago
|
Component: General → Sanitizers
Updated•4 years ago
|
Depends on: CVE-2021-23983
Updated•4 years ago
|
Updated•3 years ago
|
Depends on: float-cast-overflow
Updated•2 years ago
|
Severity: normal → S3
Updated•2 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•