starting with glibc-2.15, trying to use FD_SET according to the API results in build time warnings when using -Wsign-conversion and fortification $ cat test.c #include <sys/select.h> int main(int argc, char *argv[]) { int fd = argc; fd_set fds; FD_ZERO(&fds); FD_SET(fd, &fds); return argv[0][fd]; } $ gcc test.c -c -Wsign-conversion -O2 -Werror -D_FORTIFY_SOURCE=2 error: conversion to 'long unsigned int' from 'int' may change the sign of the result [-Werror=sign-conversion] this is because FD_SET assigns the int fd to an "unsigned int long" without a cast, and that triggers signed warnings
patch to fix is posted in the thread: http://sourceware.org/ml/libc-alpha/2012-06/msg00275.html
(In reply to comment #1) > patch to fix is posted in the thread: > http://sourceware.org/ml/libc-alpha/2012-06/msg00275.html I was there first :-) http://sourceware.org/ml/libc-alpha/2012-05/msg01879.html
indeed. i think that version though generates worse code than casting the signed value to an unsigned and leaving all the checks to doing a single compare.
(In reply to comment #3) > i think that version though generates worse code than casting the > signed value to an unsigned and leaving all the checks to doing a single > compare. In my x86_64 build, the code for __fdelt_chk is identical before/after the patch: (gdb) disas __fdelt_chk Dump of assembler code for function __fdelt_chk: 0x00000000000fb910 <+0>: cmp $0x3ff,%rdi 0x00000000000fb917 <+7>: ja 0xfb921 <__fdelt_chk+17> 0x00000000000fb919 <+9>: mov %rdi,%rax 0x00000000000fb91c <+12>: shr $0x6,%rax 0x00000000000fb920 <+16>: retq 0x00000000000fb921 <+17>: push %rax 0x00000000000fb922 <+18>: callq 0xf9a60 <__GI___chk_fail> End of assembler dump. The other part -- macro expansion -- is a compile-time check, so it doesn't matter.
I pushed Paul Pluzhnikov's fix into the trunk as git ceb9e56b3d1f8c1922e0526c2e841373843460e2 and am marking this as fixed.
Should 14210 be mentioned in NEWS as fixed in 2.16?
On 06/13/2012 10:04 AM, ppluzhnikov at google dot com wrote: > Should 14210 be mentioned in NEWS as fixed in 2.16? Right you are; thanks. I pushed that.