build fails when cmake is called with -DCMAKE_BUILD_TYPE=Release

[nbp]

While packaging rr, the problem is that in Release mode the -DNDEBUG is omitted, and the warning-as-error compilation flag is catching debug-only variable declarations.

A work-around is to give the additional argument -DCMAKE_C_FLAGS_RELEASE:STRING= to cmake.

(spew obtained with make VERBOSE=1) When building with the Release mode:

building CMakeFiles/accept.dir/src/test/accept.c.o
/nix/store/vv90ccz0p6kyfasyhh1caxdjbrqdzqc1-cmake-2.8.11.2/bin/cmake -E cmake_progress_report /tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/CMakeFiles 2
[  2%] Building C object CMakeFiles/accept.dir/src/test/accept.c.o
/nix/store/xnb0w1i2jdjv178j1y28q4bfn79b8xia-gcc-wrapper-4.8.2/bin/cc   -pthread -O0 -g3 -Wall -Werror -m32 -Wstrict-prototypes -O3 -DNDEBUG -I/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/include    -o CMakeFiles/accept.dir/src/test/accept.c.o   -c /tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c
/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c: In function ‘client’:
/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c:6:6: erreur: variable ‘clientfd’ set but not used [-Werror=unused-but-set-variable]
  int clientfd;
      ^
/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c: In function ‘server’:
/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c:34:6: erreur: unused variable ‘status’ [-Werror=unused-variable]
  int status;
      ^
/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c:33:12: erreur: unused variable ‘len’ [-Werror=unused-variable]
  socklen_t len = sizeof(peer_addr);
            ^
cc1: all warnings being treated as errors
make[2]: *** [CMakeFiles/accept.dir/src/test/accept.c.o] Erreur 1

When building without the Release mode:

building CMakeFiles/accept.dir/src/test/accept.c.o
/nix/store/vv90ccz0p6kyfasyhh1caxdjbrqdzqc1-cmake-2.8.11.2/bin/cmake -E cmake_progress_report /tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/CMakeFiles 2
[  2%] Building C object CMakeFiles/accept.dir/src/test/accept.c.o
/nix/store/xnb0w1i2jdjv178j1y28q4bfn79b8xia-gcc-wrapper-4.8.2/bin/cc   -pthread -O0 -g3 -Wall -Werror -m32 -Wstrict-prototypes -I/tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/include    -o CMakeFiles/accept.dir/src/test/accept.c.o   -c /tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0/src/test/accept.c
building bin/accept
Linking C executable bin/accept
/nix/store/vv90ccz0p6kyfasyhh1caxdjbrqdzqc1-cmake-2.8.11.2/bin/cmake -E cmake_link_script CMakeFiles/accept.dir/link.txt --verbose=1
/nix/store/xnb0w1i2jdjv178j1y28q4bfn79b8xia-gcc-wrapper-4.8.2/bin/cc   -pthread -O0 -g3 -Wall -Werror -m32 -Wstrict-prototypes    CMakeFiles/accept.dir/src/test/accept.c.o  -o bin/accept -rdynamic -lrt 
make[2] : on quitte le répertoire « /tmp/nix-build-rr-1.3.0.drv-0/rr-1.3.0 »

[rocallahan]

PRs accepted to fix those unused variables :-). With or without MFBT-style DebugOnly, probably debug_only per rr style

[joneschrisg]

Right, the codebase has been playing fast and loose with debug-only variables. The plan was to import something like debug_only when needed, as roc says.

But in general, the gains to be made by building rr in a "release" mode are mostly higher-level than enabling optimizations and disabling assertions. The ones I can think of off hand are

• not reading and comparing registers when not useful (i.e. for all events other than async signals)
• related, not storing registers to trace when not useful. Results in smaller traces.
• not ready and verifying data like syscallbuf headers

(None of those are guarded by simple assert() calls, though they could be made to be.)

But that said, the one part of the code where I'd really like to turn on optimizations is preload/preload.c. The syscall hooks could easily be hot in some workloads, but the code has lots of dumb register/stack traffic that a decent inlining optimizer could remove. That's just a guess though, would be nice to motivate with some profiler data :).

So in general I wouldn't recommend bothering with building rr in a release mode for now unless (i) a profiler shows it's worthwhile or (ii) you want to take on the higher-level changes mentioned above. (Another reason is to enable turning on super-expensive debug checks, but that's not directly visible to end users.)

[joneschrisg]

mostly higher-level than enabling optimizations and disabling assertions

The reason is that if any rr tracer code is hot, then we're trapping into rr too much and something else is probably wrong :).