1346305 - dist/include/gtest/internal/gtest-port.h:1492:5: error: acquiring mutex 'mutex_' requires negative capability '!mutex_'

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jan Beich]

Bug 1330240 disabled -Wthread-safety for Gecko while third-party code uses ALLOW_COMPILER_WARNINGS. However, bug 1343557 added FreeBSD support which enabled POSIX threads while security/pkix passes -Weverything which enables -Wthread-safety, and given upstream didn't test such a configuration we get an error.

$ echo "ac_add_options --enable-warnings-as-errors" >>.mozconfig
$ ./mach build -j1 -v
[...]
/usr/bin/clang++ -std=gnu++11 -o pkixbuild_tests.o -c -Iobjdir/dist/stl_wrappers -Iobjdir/dist/system_wrappers -include config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Isecurity/pkix/test/gtest -Iobjdir/security/pkix/test/gtest -Isecurity/pkix/include -Isecurity/pkix/lib -Isecurity/pkix/test/lib -Iobjdir/dist/include  -Iobjdir/dist/include/nspr -Iobjdir/dist/include/nss      -fPIC  -DMOZILLA_CLIENT -include objdir/mozilla-config.h -MD -MP -MF .deps/pkixbuild_tests.o.pp -Qunused-arguments   -I/usr/local/include -Qunused-arguments -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++11-compat-pedantic -Wc++14-compat -Wc++14-compat-pedantic -Wc++1z-compat -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pipe  -O -fno-omit-frame-pointer  -Werror -Weverything -Wno-c++98-compat -Wno-c++98-compat-pedantic -Wno-missing-prototypes -Wno-missing-variable-declarations -Wno-padded -Wno-reserved-id-macro -Wno-shadow -Wno-weak-vtables -Wno-error=shadow -Wno-old-style-cast -Wno-exit-time-destructors -Wno-global-constructors -Wno-used-but-marked-unused  security/pkix/test/gtest/pkixbuild_tests.cpp
In file included from security/pkix/test/gtest/pkixbuild_tests.cpp:41:
In file included from security/pkix/test/gtest/pkixgtest.h:49:
In file included from objdir/dist/include/gtest/gtest.h:58:
In file included from objdir/dist/include/gtest/internal/gtest-internal.h:40:
objdir/dist/include/gtest/internal/gtest-port.h:1492:5: error:
      acquiring mutex 'mutex_' requires negative capability '!mutex_'
      [-Werror,-Wthread-safety-negative]
    pthread_mutex_lock(&mutex_);
    ^
objdir/dist/include/gtest/internal/gtest-port.h:1491:17: note:
      Thread warning in function 'Notify'
  void Notify() {
                ^
objdir/dist/include/gtest/internal/gtest-port.h:1501:7: error:
      acquiring mutex 'mutex_' requires negative capability '!mutex_'
      [-Werror,-Wthread-safety-negative]
      pthread_mutex_lock(&mutex_);
      ^
objdir/dist/include/gtest/internal/gtest-port.h:1499:30: note:
      Thread warning in function 'WaitForNotification'
  void WaitForNotification() {
                             ^
objdir/dist/include/gtest/internal/gtest-port.h:1940:32: error:
      acquiring mutex 'mutex_' requires negative capability '!mutex_'
      [-Werror,-Wthread-safety-negative]
    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_lock(&mutex_));
                               ^
objdir/dist/include/gtest/internal/gtest-port.h:1939:15: note:
      Thread warning in function 'Lock'
  void Lock() {
              ^
objdir/dist/include/gtest/internal/gtest-port.h:1943:3: error:
      mutex 'mutex_' is still held at the end of function [-Werror,-Wthread-safety-analysis]
  }
  ^
objdir/dist/include/gtest/internal/gtest-port.h:1940:32: note:
      mutex acquired here
    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_lock(&mutex_));
                               ^
objdir/dist/include/gtest/internal/gtest-port.h:1939:15: note:
      Thread warning in function 'Lock'
  void Lock() {
              ^
objdir/dist/include/gtest/internal/gtest-port.h:1952:32: error:
      releasing mutex 'mutex_' that was not held [-Werror,-Wthread-safety-analysis]
    GTEST_CHECK_POSIX_SUCCESS_(pthread_mutex_unlock(&mutex_));
                               ^
objdir/dist/include/gtest/internal/gtest-port.h:1946:17: note:
      Thread warning in function 'Unlock'
  void Unlock() {
                ^
5 errors generated.
gmake[4]: *** [config/rules.mk:1018: pkixbuild_tests.o] Error 1
gmake[4]: Leaving directory 'objdir/security/pkix/test/gtest'

Comment 1

[Jan Beich]

Attachment: Bug 1346305 - Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557.

Review commit: https://reviewboard.mozilla.org/r/119136/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119136/

Comment 2

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8846054 [details]
Bug 1346305 - Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557.

https://reviewboard.mozilla.org/r/119136/#review121090

::: security/pkix/warnings.mozbuild:12
(Diff revision 1)
>      '-Wno-missing-prototypes',
>      '-Wno-missing-variable-declarations',
>      '-Wno-padded',
>      '-Wno-reserved-id-macro', # NSPR and NSS use reserved IDs in their include guards.
>      '-Wno-shadow', # XXX: Clang's rules are too strict for constructors.
> +    '-Wno-thread-safety', # gtest lacks locking annotations

Does it work if you put this in security/pkix/test/gtest/moz.build instead?

Comment 3

[Jan Beich]

Comment on attachment 8846054 [details]
Bug 1346305 - Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119136/diff/1-2/

Comment 4

[Jan Beich]

Comment on attachment 8846054 [details]
Bug 1346305 - Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557.

https://reviewboard.mozilla.org/r/119136/#review121104

::: security/pkix/warnings.mozbuild:12
(Diff revision 1)
>      '-Wno-missing-prototypes',
>      '-Wno-missing-variable-declarations',
>      '-Wno-padded',
>      '-Wno-reserved-id-macro', # NSPR and NSS use reserved IDs in their include guards.
>      '-Wno-shadow', # XXX: Clang's rules are too strict for constructors.
> +    '-Wno-thread-safety', # gtest lacks locking annotations

It does and makes my comment redundant.

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Comment on attachment 8846054 [details]
Bug 1346305 - Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557.

https://reviewboard.mozilla.org/r/119136/#review121148

Great - thanks.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/91e09a18fc3e
Unbreak --enable-warnings-as-errors on FreeBSD after bug 1343557. r=keeler

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/91e09a18fc3e