GNOME Bugzilla – Bug 796164
Fix atomic ops check in meson.build
Last modified: 2018-05-17 17:43:23 UTC
Commit 3e96523e6b73c4d0a11a023a7fbae01ac0d09335 did not entirely fix the check for atomic ops support, since the compiled test code doesn’t have a main() function, resulting in the following failure in the meson log: Running compile: Working directory: /tmp/tmp88bqfmrd Command line: cc -L/opt/gnome/install/lib /tmp/tmp88bqfmrd/testfile.c -pipe -D_FILE_OFFSET_BITS=64 -o /t mp/tmp88bqfmrd/output.exe -O0 -g3 -ggdb -Wall -fstrict-aliasing -fdiagnostics-color=auto -O0 -std=gnu89 Code: void func() { volatile int atomic = 2; __sync_bool_compare_and_swap (&atomic, 2, 3); } Compiler stdout: Compiler stderr: /usr/lib/gcc/x86_64-redhat-linux/7/../../../../lib64/crt1.o: In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status Checking if "atomic ops" links: NO --- This has resulted in test timeouts for days, due to an invalid combination of builtin and non-builtin atomics/locks being used. It probably also means that our build with G_ATOMIC_LOCK_FREE=false is broken, but I don’t have time to investigate that at the moment. Here’s an example of a failing test pipeline, with everything timing out due to deadlocking: https://gitlab.gnome.org/GNOME/glib/-/jobs/33179 Did commit 3e96523e6b73c4d0a11a023a7fbae01ac0d09335 actually get reviewed, or was it pushed without review? Patch coming.
Created attachment 372120 [details] [review] build: Fix test for G_ATOMIC_LOCK_FREE in meson.build Commit 3e96523e6b did not entirely fix the test, as the compiled test code did not have a main() function, so failed to link with: /usr/lib/gcc/x86_64-redhat-linux/7/../../../../lib64/crt1.o: In function `_start': (.text+0x20): undefined reference to `main' collect2: error: ld returned 1 exit status This caused an invalid mixtures of builtin and non-builtin atomics/locks to be used, which caused deadlocks in a number of tests. Fix the atomic ops test in meson.build, and the unit tests all start working again. Signed-off-by: Philip Withnall <withnall@endlessm.com>
Review of attachment 372120 [details] [review]: Looks okay, but I spotted an issue. ::: meson.build @@ +1464,2 @@ volatile int atomic = 2; __sync_bool_compare_and_swap (&atomic, 2, 3); You're missing a return, and GCC will warn about it — which may end up breaking things if we have -Werror in the environment.
Thanks. Pushed with a return statement, and with a follow-up commit to add returns to some other compiled tests. Attachment 372120 [details] pushed as 1a6fc60 - build: Fix test for G_ATOMIC_LOCK_FREE in meson.build
(In reply to Emmanuele Bassi (:ebassi) from comment #2) > You're missing a return, and GCC will warn about it — which may end up > breaking things if we have -Werror in the environment. CFLAGS from the environment are never added to compiler checks, otherwise we will constantly get spurious failures. Compiler checks don't even get args from add_project_arguments() and friends. They are completely standalone. (In reply to Philip Withnall from comment #3) > Thanks. Pushed with a return statement, and with a follow-up commit to add > returns to some other compiled tests. > This was unnecessary, but it doesn't hurt.
(In reply to Nirbheek Chauhan from comment #4) > (In reply to Emmanuele Bassi (:ebassi) from comment #2) > > You're missing a return, and GCC will warn about it — which may end up > > breaking things if we have -Werror in the environment. > > CFLAGS from the environment are never added to compiler checks, otherwise we > will constantly get spurious failures. > I was wrong about this, I added this back in 2016 to Meson because too many environments depend on this for basic compiler functionality. > Compiler checks don't even get args from add_project_arguments() and > friends. This is still correct.