1354207 - signaling disables valgrind

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, defect)

Description

[Eric Rahm [:erahm]]

It looks like even under valgrind builds signaling is specifying '-DNVALGRIND'. This is causing build failures due to -Werror-unused in code that uses the valgrind macros.

Example command line:

> [task 2017-04-05T19:31:22.998732Z] 19:31:22     INFO -  /usr/bin/ccache /home/worker/workspace/build/src/gcc/bin/gcc -std=gnu99 -o Unified_c_webrtc_signaling0.o -c -I/home/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /home/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG -DTRIMMED=1 -D_FILE_OFFSET_BITS=64 -DCHROMIUM_BUILD -DUSE_LIBJPEG_TURBO=1 -DUSE_NSS=1 -DENABLE_ONE_CLICK_SIGNIN -DGTK_DISABLE_SINGLE_INCLUDES=1 -D_ISOC99_SOURCE=1 -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DLOG4CXX_STATIC -D_NO_LOG4CXX -DUSE_SSLEAY -D_CPR_USE_EXTERNAL_LOGGER -DWEBRTC_RELATIVE_PATH -DHAVE_WEBRTC_VIDEO -DHAVE_WEBRTC_VOICE -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_UINT8_T=1 -DHAVE_UINT16_T=1 -DHAVE_UINT32_T=1 -DHAVE_UINT64_T=1 -DOS_LINUX -DSIP_OS_LINUX -DWEBRTC_POSIX -D_GNU_SOURCE -DLINUX -DGIPS_VER=3510 -DSECLIB_OPENSSL -D__STDC_FORMAT_MACROS -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/home/worker/workspace/build/src/media/webrtc/signaling -I/home/worker/workspace/build/src/obj-firefox/media/webrtc/signaling/signaling_ecc -I/home/worker/workspace/build/src/media/webrtc -I/home/worker/workspace/build/src/media/webrtc/signaling/src -I/home/worker/workspace/build/src/media/webrtc/signaling/src/common -I/home/worker/workspace/build/src/media/webrtc/signaling/src/common/browser_logging -I/home/worker/workspace/build/src/media/webrtc/signaling/src/common/time_profiling -I/home/worker/workspace/build/src/media/webrtc/signaling/src/media -I/home/worker/workspace/build/src/media/webrtc/signaling/src/media-conduit -I/home/worker/workspace/build/src/media/webrtc/signaling/src/mediapipeline -I/home/worker/workspace/build/src/media/webrtc/signaling/src/peerconnection -I/home/worker/workspace/build/src/media/webrtc/signaling/src/sdp/sipcc -I/home/worker/workspace/build/src/dom/base -I/home/worker/workspace/build/src/dom/media -I/home/worker/workspace/build/src/media/mtransport -I/home/worker/workspace/build/src/media/webrtc/trunk -I/home/worker/workspace/build/src/media/libyuv/libyuv/include -I/home/worker/workspace/build/src/media/mtransport/third_party/nrappkit/src/util/libekr -I/home/worker/workspace/build/src/netwerk/srtp/src/include -I/home/worker/workspace/build/src/netwerk/srtp/src/crypto/include -I/home/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/home/worker/workspace/build/src/ipc/chromium/src -I/home/worker/workspace/build/src/ipc/glue -I/home/worker/workspace/build/src/obj-firefox/dist/include    -fPIC  -include /home/worker/workspace/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -MD -MP -MF .deps/Unified_c_webrtc_signaling0.o.pp  -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno -pthread -pipe  -g -freorder-blocks -Os -fno-omit-frame-pointer  -Werror -I/home/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/home/worker/workspace/build/src/obj-firefox/dist/include/nss  /home/worker/workspace/build/src/obj-firefox/media/webrtc/signaling/signaling_ecc/Unified_c_webrtc_signaling0.c

Example error:

[task 2017-04-05T19:31:58.991728Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ArenaAllocator.h: In instantiation of 'mozilla::ArenaAllocator<ArenaSize, Alignment>::ArenaChunk* mozilla::ArenaAllocator<ArenaSize, Alignment>::AllocateChunk(size_t) [with long unsigned int ArenaSize = 4096ul; long unsigned int Alignment = 8ul; size_t = long unsigned int]':
[task 2017-04-05T19:31:58.991857Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:214:73:   required from 'void* mozilla::ArenaAllocator<ArenaSize, Alignment>::InternalAllocate(size_t) [with long unsigned int ArenaSize = 4096ul; long unsigned int Alignment = 8ul; size_t = long unsigned int]'
[task 2017-04-05T19:31:58.991995Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:72:47:   required from 'void* mozilla::ArenaAllocator<ArenaSize, Alignment>::Allocate(size_t, const mozilla::fallible_t&) [with long unsigned int ArenaSize = 4096ul; long unsigned int Alignment = 8ul; size_t = long unsigned int]'
[task 2017-04-05T19:31:58.992131Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:77:39:   required from 'void* mozilla::ArenaAllocator<ArenaSize, Alignment>::Allocate(size_t) [with long unsigned int ArenaSize = 4096ul; long unsigned int Alignment = 8ul; size_t = long unsigned int]'
[task 2017-04-05T19:31:58.992196Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/layers/BSPTree.h:71:32:   required from here
[task 2017-04-05T19:31:58.992276Z] 19:31:58     INFO -  /home/worker/workspace/build/src/obj-firefox/dist/include/mozilla/ArenaAllocator.h:185:5: error: statement has no effect [-Werror=unused-value]
[task 2017-04-05T19:31:58.992320Z] 19:31:58     INFO -       MOZ_MAKE_MEM_NOACCESS((void*)arena->header.offset,

Comment 1

[Nils Ohlmeier [:drno]]

I'm not a build expert, but we appear to get that NVALGRIND from the webrtc.org build system: http://searchfox.org/mozilla-central/search?q=NVALGRIND&case=false&regexp=false&path=webrtc

Comment 2

[Eric Rahm [:erahm]]

Seems like maybe we're not setting build_for_tool: "memcheck" [1]? I'm not really sure how our gyp integration works unfortunately.

[1] http://searchfox.org/mozilla-central/rev/b8cce081200129a1307e2a682f121135b5ca1d19/media/webrtc/trunk/build/common.gypi#1359

Comment 3

[Eric Rahm [:erahm]]

Also note this is blocking bug 1351426 from landing.

Comment 4

[Nils Ohlmeier [:drno]]

So removing all three occurrences of NVALGRIND from the build files shows that the define no longer gets used during the build.

Comment 5

[Nils Ohlmeier [:drno]]

I just did a local debug build and there is no NVALGRIND define to be seen:

15:37.98 /usr/bin/clang -std=gnu99 -o Unified_c_webrtc_signaling0.o -c -fvisibility=hidden -fvisibility-inlines-hidden -DDEBUG=1 -DTRACING=1 -DCHROMIUM_BUILD -DUSE_LIBJPEG_TURBO=1 -DENABLE_ONE_CLICK_SIGNIN -DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS -DENABLE_HIDPI=1 -DENABLE_GPU=1 -DENABLE_EGLIMAGE=1 -DUSE_SKIA=1 -DENABLE_TASK_MANAGER=1 -DENABLE_WEB_INTENTS=1 -DENABLE_EXTENSIONS=1 -DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PROTECTOR_SERVICE=1 -DENABLE_SESSION_SERVICE=1 -DENABLE_THEMES=1 -DENABLE_BACKGROUND=1 -DENABLE_AUTOMATION=1 -DENABLE_PRINTING=1 -DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DLOG4CXX_STATIC -D_NO_LOG4CXX -DUSE_SSLEAY -D_CPR_USE_EXTERNAL_LOGGER -DWEBRTC_RELATIVE_PATH -DHAVE_WEBRTC_VIDEO -DHAVE_WEBRTC_VOICE -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_UINT8_T=1 -DHAVE_UINT16_T=1 -DHAVE_UINT32_T=1 -DHAVE_UINT64_T=1 -DWEBRTC_POSIX -DOS_MACOSX -DSIP_OS_OSX -DOSX -D_FORTIFY_SOURCE=2 -D__STDC_FORMAT_MACROS -DDYNAMIC_ANNOTATIONS_ENABLED=1 -DWTF_USE_DYNAMIC_ANNOTATIONS=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling -I/Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/media/webrtc/signaling/signaling_ecc -I/Users/nohlmeier/src/mozilla-central/media/webrtc -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/common -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/common/browser_logging -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/common/time_profiling -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/media -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/media-conduit -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/mediapipeline -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/peerconnection -I/Users/nohlmeier/src/mozilla-central/media/webrtc/signaling/src/sdp/sipcc -I/Users/nohlmeier/src/mozilla-central/dom/base -I/Users/nohlmeier/src/mozilla-central/dom/media -I/Users/nohlmeier/src/mozilla-central/media/mtransport -I/Users/nohlmeier/src/mozilla-central/media/webrtc/trunk -I/Users/nohlmeier/src/mozilla-central/media/libyuv/libyuv/include -I/Users/nohlmeier/src/mozilla-central/media/mtransport/third_party/nrappkit/src/util/libekr -I/Users/nohlmeier/src/mozilla-central/netwerk/srtp/src/include -I/Users/nohlmeier/src/mozilla-central/netwerk/srtp/src/crypto/include -I/Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/ipc/ipdl/_ipdlheaders -I/Users/nohlmeier/src/mozilla-central/ipc/chromium/src -I/Users/nohlmeier/src/mozilla-central/ipc/glue -I/Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/dist/include    -fPIC  -include /Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/mozilla-config.h -DMOZILLA_CLIENT -MD -MP -MF .deps/Unified_c_webrtc_signaling0.o.pp -Qunused-arguments  -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wclass-varargs -Wloop-analysis -Wstring-conversion -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wno-gnu-zero-variadic-macro-arguments -fno-strict-aliasing -fno-math-errno -pthread -pipe  -g -fno-omit-frame-pointer   -I/Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/dist/include/nspr -I/Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/dist/include/nss -Wno-inconsistent-missing-override -Wno-macro-redefined  /Users/nohlmeier/src/mozilla-central/objdir-x86_64-apple/media/webrtc/signaling/signaling_ecc/Unified_c_webrtc_signaling0.c

Comment 6

[Eric Rahm [:erahm]]

Attachment: WIP: Set memcheck flag when building under valgrind

MozReview-Commit-ID: 859LIn8n80i

Comment 7

[Eric Rahm [:erahm]]

Nils, I think attachment 8855483 [details] [diff] [review] should fix this specific issue, but again I'm not well versed in gyp. Arguably we should fix it for asan as well. Can you try it out?

Comment 8

[Nils Ohlmeier [:drno]]

Optimized local build has the NVALGRIND. So it looks because we optimize our Valgrind builds it pulls in the NVALGRIND

Comment 9

[Nils Ohlmeier [:drno]]

Comment on attachment 8855483 [details] [diff] [review]
WIP: Set memcheck flag when building under valgrind

Review of attachment 8855483 [details] [diff] [review]:
-----------------------------------------------------------------

Nope, sorry does not work. I still see NVALGRIND in the build log with this patch applied.

Comment 10

[Nils Ohlmeier [:drno]]

Dan do you know/understand what we need to do to get rid of the NVALGRIND if MOZ_VALGRIND is set?

Comment 11

[Eric Rahm [:erahm]]

(In reply to Nils Ohlmeier [:drno] from comment #9)
> Comment on attachment 8855483 [details] [diff] [review]
> WIP: Set memcheck flag when building under valgrind
> 
> Review of attachment 8855483 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Nope, sorry does not work. I still see NVALGRIND in the build log with this
> patch applied.

Even for signaling? I finally got a build locally, but it's possible my Werror setup is different than the build machine.

Comment 12

[Eric Rahm [:erahm]]

Attachment: Set webrtc build flags when building under valgrind, etc

The webrtc gyp files have a 'build_for_tool' flag that controls among other
things what defines are provided at build time. This meant that during a
firefox valgrind build webrtc would still specify NVALGRIND, thus disabling
some valgrind macros. Similarly there are flags for asan and tsan that we
should probably have been specifying as well. This patch sets the
'build_for_tool' flag to the appropriate value when building under valgrind,
asan, and tsan.

Comment 13

[Eric Rahm [:erahm]]

Kicked off a build including patches from the blocked bug at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=455c1849e68b4f378b2fa0a4510f792c7ca2f55b

Comment 14

[Nils Ohlmeier [:drno]]

(In reply to Eric Rahm [:erahm] from comment #11)
> (In reply to Nils Ohlmeier [:drno] from comment #9)
> > Comment on attachment 8855483 [details] [diff] [review]
> > WIP: Set memcheck flag when building under valgrind
> > 
> > Review of attachment 8855483 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > Nope, sorry does not work. I still see NVALGRIND in the build log with this
> > patch applied.
> 
> Even for signaling? I finally got a build locally, but it's possible my
> Werror setup is different than the build machine.

Sorry for Unified_c_webrtc_signaling0.c there is no NVALGRIND any more, but NVALGRIND is still present in a lot of other compiler lines. So I guess depends what exactly you need here.

Comment 15

[Eric Rahm [:erahm]]

(In reply to Nils Ohlmeier [:drno] from comment #14)
> Sorry for Unified_c_webrtc_signaling0.c there is no NVALGRIND any more, but
> NVALGRIND is still present in a lot of other compiler lines. So I guess
> depends what exactly you need here.

I mostly just care about unbreaking signaling, we can chase down the rest some other time. The patch I put up should apply to the rest of webrtc, so hopefully that covers enough.

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8855572 [details] [diff] [review]
Set webrtc build flags when building under valgrind, etc

Review of attachment 8855572 [details] [diff] [review]:
-----------------------------------------------------------------

r+ but adding a build peer for review

Comment 17

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4d3ecca7252bef0a57c6607de8bdd58812b3e4c7
Bug 1354207 - Set webrtc build flags when building under valgrind, etc. r=jesup

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4d3ecca7252b