threaded version build error

[hyln9]

Threaded version of Chez Scheme can be built via ./configure --threads, but it seems that gcc/glibc is not happy with some libc call without return value being used, such as write. With -Werror, build may fail. I guess headers of glibc declare __attribute__((warn_unused_result)).

gcc version: gcc version 5.3.1 20160413 (Ubuntu 5.3.1-14ubuntu2)

glibc version: GNU C Library (Ubuntu GLIBC 2.23-0ubuntu3) stable release version 2.23, by Roland McGrath et al.

one of the build errors(commit https://github.com/cisco/ChezScheme/commit/7427dda85c1ae43f1e3c8f8749a578d50764fdfd):

prim5.c: In function ‘s_backdoor_thread_start’:
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S));
                                           ^
prim5.c:1327:3: note: in expansion of macro ‘display’
   display("backdoor thread started\n")

[adamsmd]

I get the same error.

My system information is as follows:

 $ uname -a
Linux spiritus 4.4.0-21-generic #37-Ubuntu SMP Mon Apr 18 18:33:37 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ gcc --version
gcc (Ubuntu 5.3.1-14ubuntu2) 5.3.1 20160413
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

The compilation process produces the following:

$ ./configure --threads
Submodule 'nanopass' (https://github.com/nanopass/nanopass-framework-scheme.git) registered for path 'nanopass'
Submodule 'zlib' (https://github.com/madler/zlib.git) registered for path 'zlib'
Cloning into 'nanopass'...
remote: Counting objects: 607, done.
remote: Total 607 (delta 0), reused 0 (delta 0), pack-reused 607
Receiving objects: 100% (607/607), 26.69 MiB | 4.74 MiB/s, done.
Resolving deltas: 100% (318/318), done.
Checking connectivity... done.
Submodule path 'nanopass': checked out '221eecb965d9dfacccd97d1cb73f2a31c4119d3a'
Cloning into 'zlib'...
remote: Counting objects: 4370, done.
remote: Total 4370 (delta 0), reused 0 (delta 0), pack-reused 4370
Receiving objects: 100% (4370/4370), 2.43 MiB | 1.28 MiB/s, done.
Resolving deltas: 100% (2979/2979), done.
Checking connectivity... done.
Submodule path 'zlib': checked out '50893291621658f355bc5b4d450a8d06a563053d'
$ make
(cd ta6le && make build)
(cd c ; make)
ln -s ../../c/statics.c statics.c
ln -s ../../c/system.h system.h
ln -s ../../c/types.h types.h
ln -s ../../c/version.h version.h
ln -s ../../c/externs.h externs.h
ln -s ../../c/globals.h globals.h
ln -s ../../c/segment.h segment.h
ln -s ../../c/thread.h thread.h
ln -s ../../c/sort.h sort.h
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib statics.c
ln -s ../../c/segment.c segment.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib segment.c
ln -s ../../c/alloc.c alloc.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib alloc.c
ln -s ../../c/symbol.c symbol.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib symbol.c
ln -s ../../c/intern.c intern.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib intern.c
ln -s ../../c/gcwrapper.c gcwrapper.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib gcwrapper.c
ln -s ../../c/gc-ocd.c gc-ocd.c
ln -s ../../c/gc.c gc.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib gc-ocd.c
ln -s ../../c/gc-oce.c gc-oce.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib gc-oce.c
ln -s ../../c/number.c number.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib number.c
ln -s ../../c/schsig.c schsig.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib schsig.c
ln -s ../../c/io.c io.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib io.c
ln -s ../../c/new-io.c new-io.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib new-io.c
ln -s ../../c/print.c print.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib print.c
ln -s ../../c/fasl.c fasl.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib fasl.c
ln -s ../../c/stats.c stats.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib stats.c
ln -s ../../c/foreign.c foreign.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib foreign.c
ln -s ../../c/prim.c prim.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib prim.c
ln -s ../../c/prim5.c prim5.c
gcc -m64 -msse2 -Wpointer-arith -Wall -Wextra -Werror -O2 -D_REENTRANT -pthread -c -DX86_64 -I../boot/ta6le -I../zlib prim5.c
In file included from system.h:23:0,
                 from prim5.c:17:
prim5.c: In function ‘s_backdoor_thread’:
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1342:3: note: in expansion of macro ‘display’
   display("creating thread\n");
   ^
prim5.c: In function ‘s_backdoor_thread_start’:
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1327:3: note: in expansion of macro ‘display’
   display("backdoor thread started\n")
   ^
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1329:3: note: in expansion of macro ‘display’
   display("thread activated\n")
   ^
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1332:3: note: in expansion of macro ‘display’
   display("thread deactivated\n")
   ^
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1334:3: note: in expansion of macro ‘display’
   display("thread reeactivated\n")
   ^
version.h:424:16: error: ignoring return value of ‘write’, declared with attribute warn_unused_result [-Werror=unused-result]
 # define WRITE write
                ^
prim5.c:1325:43: note: in expansion of macro ‘WRITE’
 #define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
                                           ^
prim5.c:1337:3: note: in expansion of macro ‘display’
   display("thread destroyed\n")
   ^
cc1: all warnings being treated as errors
Makefile:29: recipe for target 'prim5.o' failed
make[2]: *** [prim5.o] Error 1
Makefile:19: recipe for target 'build' failed
make[1]: *** [build] Error 2
Makefile:19: recipe for target 'build' failed
make: *** [build] Error 2

[dybvig]

Does inserting (void) before the call to write in the definition of display fix the issue?

[hyln9]

No. This kind of trick is not working anymore with modern gcc.

[hyln9]

But we can do this: int r attribute((unused)) = WRITE(... (not portable)

[yinwang0]

I just built with --threads on OSX without this problem. But OSX's gcc is just clang in disguise. So much trouble to get all the C compilers work properly ;)

[pscollins]

It seems this will require looking at each individual use of WRITE, since there are these usages:

new-io.c: In function ‘S_bytevector_write’:
version.h:424:16: error: expected expression before ‘__attribute__’
 # define WRITE __attribute__((unused)) int __tmp_assgn = write
                ^
new-io.c:68:8: note: in definition of macro ‘FD_EINTR_GUARD’
   do { body;                                            \
        ^
new-io.c:541:32: note: in expansion of macro ‘WRITE’
                      flag, i = WRITE(fd, &BVIT(bv,s), (IO_SIZE_T)cx));
                                ^
new-io.c:519:7: error: unused variable ‘fd’ [-Werror=unused-variable]
   INT fd = gzflag ? 0 : GET_FD(file);
       ^
new-io.c: In function ‘S_put_byte’:
version.h:424:16: error: expected expression before ‘__attribute__’
 # define WRITE __attribute__((unused)) int __tmp_assgn = write
                ^
new-io.c:68:8: note: in definition of macro ‘FD_EINTR_GUARD’
   do { body;                                            \
        ^
new-io.c:596:30: note: in expansion of macro ‘WRITE’
                    flag, i = WRITE(fd, buf, 1));
                              ^
new-io.c:581:7: error: unused variable ‘fd’ [-Werror=unused-variable]
   INT fd = gzflag ? 0 : GET_FD(file);

This is with a temporary hack fix of:

# define WRITE __attribute__((unused)) int __tmp_assgn = write

[hyln9]

@pscollins This is not true. We only need to change the front of WRITE in prim5.c.

[ChaosEternal]

@hyln9 seems we can check the return value using an assert then make it a proper patch

[hyln9]

Gotcha, we can surround WRITE with an empty if clause to suppress this warning.

#define display(s) { const char *S = (s); if(WRITE(1, S, (unsigned int)strlen(S))){}; }

[pscollins]

Does that work? Does it not cause an error in new-io.c? WRITE needs to return a value to work there.

[hyln9]

@pscollins Yeah, tested. Return value of WRITE in new-io.c is indeed used.

[adamsmd]

@dybvig: Should the return value of WRITE be ignored in the display CPP macro in c/prim5.c? (That being the only use of WRITE causing this error.)

If it should be ignored, then, as pointed out by @hyln9, if (WRITE(...)) {} suppresses the warning/error. Apparently, (void) WRITE(...) used to suppress this warning/error but does not on modern GCC. Note that the body of the if must be {} and not ; because ; causes a warning about an empty if body.

The following patch does this:

diff --git a/c/prim5.c b/c/prim5.c
index 6793a18..13f3690 100644
--- a/c/prim5.c
+++ b/c/prim5.c
@@ -1322,7 +1322,7 @@ static void s_putenv(name, value) char *name, *value; {

 #ifdef PTHREADS
 /* backdoor thread is for testing thread creation by Sactivate_thread */
-#define display(s) { const char *S = (s); WRITE(1, S, (unsigned int)strlen(S)); }
+#define display(s) { const char *S = (s); if (WRITE(1, S, (unsigned int)strlen(S))) {} }
 static s_thread_rv_t s_backdoor_thread_start(p) void *p; {
   display("backdoor thread started\n")
   (void) Sactivate_thread();

Alternatively, if there is something meaningful to do when WRITE fails, we can do something like the following, where /* PUT ERROR HANDLING HERE */ is replaced with the appropriate code:

#define display(s) { const char *S = (s); size_t len = strlen(S); ssize_t r = WRITE(1, S, len); if(r != len) { /* PUT ERROR HANDLING HERE */ } }

[dybvig]

I made the suggested fix in my most recent commit. I didn't opt for one that actually does error handling since this is test code whose behavior is directly checked at a higher level already.

[adamsmd]

That commit fixed the problem for me. As far as I am concerned, this issue can be marked as closed.