Closed Bug 1795186 Opened 2 years ago Closed 1 year ago

Perma Bp-clang-trunk [tier 2] /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:847:1: error: mutex 'forward(aArgs)' is still held at the end of function [-Werror,-Wthread-safety-analysis]

Categories

(Core :: XPCOM, defect)

Product:
Core
Component:
XPCOM
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
120 Branch
Tracking Flags:
Tracking Status
firefox120 --- fixed

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Bug 1795186 - Specialize Maybe<*AutoLock> to silence thread-safety analysis.
48 bytes, text/x-phabricator-request
Details | Review
Bug 1795186 - Remove patches that reverted thread-safety analysis changes in clang.
48 bytes, text/x-phabricator-request
Details | Review

The error further below, for now, doesn't happen with clang-trunk because the relevant upstream change was backed out in bug 1794074.

The core problem is that the exceptions that were added for Maybe<*AutoLock>::emplace don't work anymore. One way around would be to add the exceptions onto Maybe::emplace itself (as opposed to the caller), but it also seems to me we should not use Maybe in the first place. After all, we have ReleasableBaseAutoLock, although we don't have typedefs for MutexSingleWriter or OffTheBooksMutex.

+++ This bug was initially created as a clone of Bug #1794074 +++

Filed by: nfay [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer?job_id=392536175&repo=mozilla-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/KJ3UtjcCRiGdKbpeGYR2fA/runs/0/artifacts/public/logs/live_backing.log

[task 2022-10-07T03:27:05.203Z] 03:27:05     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/base'
[task 2022-10-07T03:27:05.210Z] 03:27:05     INFO -  /builds/worker/fetches/clang/bin/clang++ --sysroot /builds/worker/fetches/sysroot-x86_64-linux-gnu -o Unified_cpp_dom_base0.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fstack-clash-protection -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/workspace/obj-build/dom/base -I/builds/worker/checkouts/gecko/dom/battery -I/builds/worker/checkouts/gecko/dom/events -I/builds/worker/checkouts/gecko/dom/media -I/builds/worker/checkouts/gecko/dom/network -I/builds/worker/checkouts/gecko/caps -I/builds/worker/checkouts/gecko/docshell/base -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/file -I/builds/worker/checkouts/gecko/dom/geolocation -I/builds/worker/checkouts/gecko/dom/html -I/builds/worker/checkouts/gecko/dom/ipc -I/builds/worker/checkouts/gecko/dom/storage -I/builds/worker/checkouts/gecko/dom/svg -I/builds/worker/checkouts/gecko/dom/u2f -I/builds/worker/checkouts/gecko/dom/xml -I/builds/worker/checkouts/gecko/dom/xslt/xpath -I/builds/worker/checkouts/gecko/dom/xul -I/builds/worker/checkouts/gecko/gfx/2d -I/builds/worker/checkouts/gecko/image -I/builds/worker/checkouts/gecko/js/xpconnect/loader -I/builds/worker/checkouts/gecko/js/xpconnect/src -I/builds/worker/checkouts/gecko/js/xpconnect/wrappers -I/builds/worker/checkouts/gecko/layout/base -I/builds/worker/checkouts/gecko/layout/forms -I/builds/worker/checkouts/gecko/layout/generic -I/builds/worker/checkouts/gecko/layout/style -I/builds/worker/checkouts/gecko/layout/xul -I/builds/worker/checkouts/gecko/netwerk/base -I/builds/worker/checkouts/gecko/netwerk/url-classifier -I/builds/worker/checkouts/gecko/parser/htmlparser -I/builds/worker/checkouts/gecko/security/manager/ssl -I/builds/worker/checkouts/gecko/widget -I/builds/worker/checkouts/gecko/xpcom/ds -I/builds/worker/checkouts/gecko/netwerk/sctp/datachannel -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -Qunused-arguments -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wdeprecated-this-capture -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wenum-compare-conditional -Wno-ambiguous-reversed-operator -Wno-error=deprecated -Wno-error=deprecated-anon-enum-enum-conversion -Wno-error=deprecated-enum-enum-conversion -Wno-error=deprecated-enum-float-conversion -Wno-error=deprecated-pragma -Wno-error=deprecated-this-capture -Wno-error=deprecated-volatile -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wformat -Wformat-security -Wno-psabi -Wthread-safety -Wno-unknown-warning-option -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fPIC -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -gdwarf-4 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -fno-omit-frame-pointer -funwind-tables -Werror -pthread -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/gtk-3.0/unix-print -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/gtk-3.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/at-spi2-atk/2.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/at-spi-2.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/dbus-1.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/x86_64-linux-gnu/dbus-1.0/include -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/gtk-3.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/cairo -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/pango-1.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/harfbuzz -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/pango-1.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/atk-1.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/cairo -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/pixman-1 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/freetype2 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/libpng12 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/gdk-pixbuf-2.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/libpng12 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/gio-unix-2.0/ -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/include/glib-2.0 -I/builds/worker/fetches/sysroot-x86_64-linux-gnu/usr/lib/x86_64-linux-gnu/glib-2.0/include -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/Unified_cpp_dom_base0.o.pp   Unified_cpp_dom_base0.cpp
[task 2022-10-07T03:27:05.210Z] 03:27:05     INFO -  In file included from Unified_cpp_dom_base0.cpp:2:
[task 2022-10-07T03:27:05.211Z] 03:27:05     INFO -  In file included from /builds/worker/checkouts/gecko/dom/base/AbstractRange.cpp:7:
[task 2022-10-07T03:27:05.211Z] 03:27:05     INFO -  In file included from /builds/worker/workspace/obj-build/dist/include/mozilla/dom/AbstractRange.h:12:
[task 2022-10-07T03:27:05.211Z] 03:27:05     INFO -  In file included from /builds/worker/workspace/obj-build/dist/include/js/RootingAPI.h:14:
[task 2022-10-07T03:27:05.212Z] 03:27:05    ERROR -  /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:847:1: error: mutex 'forward(aArgs)' is still held at the end of function [-Werror,-Wthread-safety-analysis]
[task 2022-10-07T03:27:05.212Z] 03:27:05     INFO -  }
[task 2022-10-07T03:27:05.212Z] 03:27:05     INFO -  ^
[task 2022-10-07T03:27:05.213Z] 03:27:05     INFO -  /builds/worker/checkouts/gecko/dom/base/BodyStream.cpp:438:8: note: in instantiation of function template specialization 'mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::MutexSingleWriter &>>::emplace<mozilla::MutexSingleWriter &>' requested here
[task 2022-10-07T03:27:05.213Z] 03:27:05     INFO -    lock.emplace(mMutex);
[task 2022-10-07T03:27:05.213Z] 03:27:05     INFO -         ^
[task 2022-10-07T03:27:05.214Z] 03:27:05     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Maybe.h:845:39: note: mutex acquired here
[task 2022-10-07T03:27:05.214Z] 03:27:05     INFO -    ::new (KnownNotNull, &mStorage.val) T(std::forward<Args>(aArgs)...);
[task 2022-10-07T03:27:05.214Z] 03:27:05     INFO -                                        ^
[task 2022-10-07T03:27:05.214Z] 03:27:05     INFO -  1 error generated.
[task 2022-10-07T03:27:05.215Z] 03:27:05    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:668: Unified_cpp_dom_base0.o] Error 1
[task 2022-10-07T03:27:05.215Z] 03:27:05     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/base'
[task 2022-10-07T03:27:05.215Z] 03:27:05     INFO -  gmake[4]: *** Waiting for unfinished jobs....
[task 2022-10-07T03:27:05.215Z] 03:27:05     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/dom/indexedDB/test/gtest'
[task 2022-10-07T03:27:05.216Z] 03:27:05     INFO -  dom/indexedDB/test/gtest/TestSimpleFileInfo.o
[task 2022-10-07T03:27:05.216Z] 03:27:05     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/dom/indexedDB/test/gtest'

We should fix this at some point by changing callers which use Maybe<*AutoLock> to instead use ReleasableAutoLock and friends, so that we don't need to back out the upstream change anymore.

Severity: -- → S3
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Blocks: 1855597
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/75bd8d868132 Specialize Maybe<*AutoLock> to silence thread-safety analysis. r=nika https://hg.mozilla.org/integration/autoland/rev/c123d1940504 Remove patches that reverted thread-safety analysis changes in clang. r=firefox-build-system-reviewers,sergesanspaille

Backed out for causing bustages on ActorsParent.cpp

[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  In file included from Unified_cpp_dom_localstorage0.cpp:11:
[task 2023-09-30T21:46:30.891Z] 21:46:30    ERROR -  /builds/worker/checkouts/gecko/dom/localstorage/ActorsParent.cpp:8991:34: error: no member named 'isNothing' in 'mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex &>>'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -   8991 |   MOZ_ASSERT(mShadowDatabaseLock.isNothing());
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |              ~~~~~~~~~~~~~~~~~~~ ^
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:444:24: note: expanded from macro 'MOZ_ASSERT'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    444 |         ("MOZ_ASSERT", __VA_ARGS__))
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                        ^~~~~~~~~~~
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:434:33: note: expanded from macro 'MOZ_ASSERT_GLUE'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    434 | #define MOZ_ASSERT_GLUE(a, b) a b
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                 ^
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:413:40: note: expanded from macro 'MOZ_ASSERT_HELPER1'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    413 |     MOZ_VALIDATE_ASSERT_CONDITION_TYPE(expr);                  \
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                        ^~~~
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:395:58: note: expanded from macro 'MOZ_VALIDATE_ASSERT_CONDITION_TYPE'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    395 |         mozilla::detail::AssertionConditionType<decltype(x)>::isValid, \
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                                          ^
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  In file included from Unified_cpp_dom_localstorage0.cpp:11:
[task 2023-09-30T21:46:30.891Z] 21:46:30    ERROR -  /builds/worker/checkouts/gecko/dom/localstorage/ActorsParent.cpp:8991:34: error: no member named 'isNothing' in 'mozilla::Maybe<mozilla::detail::BaseAutoLock<mozilla::Mutex &>>'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -   8991 |   MOZ_ASSERT(mShadowDatabaseLock.isNothing());
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |              ~~~~~~~~~~~~~~~~~~~ ^
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:444:24: note: expanded from macro 'MOZ_ASSERT'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    444 |         ("MOZ_ASSERT", __VA_ARGS__))
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                        ^~~~~~~~~~~
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:434:33: note: expanded from macro 'MOZ_ASSERT_GLUE'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    434 | #define MOZ_ASSERT_GLUE(a, b) a b
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                 ^
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Assertions.h:414:51: note: expanded from macro 'MOZ_ASSERT_HELPER1'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -    414 |     if (MOZ_UNLIKELY(!MOZ_CHECK_ASSERT_ASSIGNMENT(expr))) {    \
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                                   ^~~~
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/StaticAnalysisFunctions.h:62:73: note: expanded from macro 'MOZ_CHECK_ASSERT_ASSIGNMENT'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -     62 | #  define MOZ_CHECK_ASSERT_ASSIGNMENT(expr) MOZ_AssertAssignmentTest(!!(expr))
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                                                         ^~~~
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -  /builds/worker/workspace/obj-build/dist/include/mozilla/Likely.h:17:48: note: expanded from macro 'MOZ_UNLIKELY'
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -     17 | #  define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
[task 2023-09-30T21:46:30.891Z] 21:46:30     INFO -        |                                                ^
Flags: needinfo?(mh+mozilla)
Pushed by mh@glandium.org: https://hg.mozilla.org/integration/autoland/rev/d9efb4340fe8 Specialize Maybe<*AutoLock> to silence thread-safety analysis. r=nika https://hg.mozilla.org/integration/autoland/rev/1946adf7dff6 Remove patches that reverted thread-safety analysis changes in clang. r=firefox-build-system-reviewers,sergesanspaille

https://hg.mozilla.org/mozilla-central/rev/d9efb4340fe8
https://hg.mozilla.org/mozilla-central/rev/1946adf7dff6

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox120: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 120 Branch
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: