Open Bug 1367146 (ubsan) Opened 7 years ago Updated 11 days ago

[meta] UBSAN errors

Categories

(Core :: Sanitizers, task, P3)

task

Tracking

()

UNCONFIRMED

People

(Reporter: mliska, Unassigned)

References

(Depends on 54 open bugs)

Details

(Keywords: meta, Whiteboard: [js:tech-debt])

Attachments

(3 files)

Attached file firefox.ubsan.log —
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.
Component: Untriaged → General
Product: Firefox → Core
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.
Component: General → JavaScript Engine
Flags: needinfo?(nihsanullah)
Summary: UBSAN errors → [meta] UBSAN errors
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]
Depends on: 1400081
Depends on: 1400148
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)
Depends on: 1401145
Depends on: 1401191
Attached file Current UBSAN log —
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)
Filed https://ssl.icu-project.org/trac/ticket/13362 for the UBSan errors in the imported ICU library.
Depends on: 1401200
Depends on: 1401209
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 <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.
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.
Flags: needinfo?(mliska)
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)
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.
Depends on: 1401515
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
Flags: needinfo?(n.nethercote)
(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)
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.
Alias: ubsan
Depends on: 1404151
Depends on: 1404169
Depends on: 1404174
Depends on: 1404189
Depends on: 1404219
Depends on: 1404220
Depends on: 1404224
Depends on: 1404226
Depends on: 1404547
Depends on: 1404570
Depends on: 1404572
Depends on: 1469410
Depends on: 1469910
No longer depends on: 1469410
Depends on: 1467920
Depends on: 1460764
Depends on: 1478523
Depends on: 1436778
Depends on: 1284975
Depends on: 1532849
Depends on: 1532858
Depends on: 1431882
Depends on: 1532861
Depends on: 1532867
Depends on: 1532868
Depends on: 1413750
Depends on: 1533127
Depends on: 1533612
Depends on: 1534156
Depends on: 1534709
Depends on: 1535980
Depends on: 1538768
Type: defect → task
Depends on: 1439446, 1438260
Depends on: 1468131
Depends on: 1568047
Depends on: 1573049
Depends on: 1575584
Depends on: 1577584
Depends on: 1577669
Depends on: 1580352
Depends on: 1580317
Depends on: 1438948
Depends on: 1580918
Depends on: 1581655
Depends on: 1581672
Depends on: 1581946
Depends on: 1581964
Depends on: 1581986
Depends on: 1583291
Depends on: 1583293
Depends on: 1583399
Depends on: 1583402
Depends on: 1447055
Depends on: 1583405
Depends on: 1583645
Depends on: 1583946
Depends on: 1583967
Depends on: 1583970
Depends on: 1583995
Depends on: 1584005
Depends on: 1584006
Depends on: 1584008
Depends on: 1584407
Depends on: 1584601
Depends on: 1584639
Depends on: 1584640
Depends on: 1584643
Depends on: 1584660
Depends on: 1585721
Depends on: 1585845
Depends on: 1586165
Depends on: 1586170
Depends on: 1587146
Depends on: 1587159
Depends on: 1587162
Depends on: 1587164
Depends on: 1587173
Depends on: 1587176
Depends on: 1588938
Depends on: 1588940
Depends on: 1589496
Depends on: 1589527
Depends on: 1593387
Depends on: 1594942
Depends on: 1595259
Depends on: 1595260
Depends on: 1602333
Depends on: 1603224
Depends on: 1603271
Depends on: 1603280
Depends on: 1603289
Depends on: 1603296
Depends on: 1603298
Depends on: 1603807
Depends on: 1603857
Depends on: 1603859
Depends on: 1604307
Depends on: 1604331
Depends on: 1604357
Depends on: 1605797
Depends on: 1419232
Depends on: 1606148
Depends on: 1613286
Depends on: 1614512
Depends on: 1614530
Depends on: 1617270
Component: General → Sanitizers
Depends on: 1620671
Depends on: 1622989
Depends on: 1622895
Depends on: 1633828
Depends on: 1636596
Depends on: 1427673
Depends on: 1640041
Depends on: 1640042
Depends on: 1640247
Depends on: 1640248
Depends on: 1640253
Depends on: 1649862
Depends on: 1666607
Depends on: 1668589
Depends on: 1675366
Depends on: 1679882
Depends on: 1684377
Depends on: 1689415
Depends on: CVE-2021-23983
See Also: → 1697512
Depends on: 1704176
Depends on: 1709441
Depends on: 1724460
Depends on: 1724463
Depends on: 1446871
Depends on: 1746690
Depends on: 1746913
Depends on: 1746934
Depends on: 1746936
Depends on: 1746939
Depends on: 1746957
Depends on: 1746989
Depends on: 1747277
Depends on: 1747330
Depends on: 1747458
Depends on: 1217609
Depends on: 1748880
Depends on: 1749225
Depends on: 1749226
Depends on: 1749227
Depends on: 1749228
Depends on: 1749229
Depends on: 1752377
Depends on: 1782124
Depends on: 1782141
Depends on: 1784352
Depends on: 1788368
Depends on: 1788965
Depends on: 1790526
Depends on: 1792086
Depends on: 1792092
Severity: normal → S3
Depends on: 1795142
Depends on: 1798782
Depends on: 1801248
Depends on: 1804998
Depends on: 1805327
Depends on: 1829910
Depends on: 1874251
Depends on: 1882148
Depends on: 1883793
You need to log in before you can comment on or make changes to this bug.