Created attachment 6674 [details] Simple patch resolving the valgrind warning glibc 2.16.0's commit c030f70c introduces some variables on the stack which don't get fully initialized, leading to valgrind complaints in the __libc_res_nsend() -> send_dg() -> sendmmsg() call chain, eg: Syscall param sendmsg(mmsg[0].msg_hdr) points to uninitialised byte(s) at 0x5AEAC6B: sendmmsg (sendmmsg.c:36) by 0x97B4643: __libc_res_nsend (res_send.c:1133) by 0x97B1C66: __libc_res_nquery (res_query.c:225) by 0x97B28D9: __libc_res_nsearch (res_query.c:582) by 0x95A5AC7: _nss_dns_gethostbyname4_r (dns-host.c:313) by 0x5AD076A: gaih_inet (getaddrinfo.c:842) by 0x5AD4653: getaddrinfo (getaddrinfo.c:2421) by 0x55BF384: Curl_getaddrinfo_ex (in /usr/lib64/libcurl.so.4.2.0) by 0x55C61C9: getaddrinfo_thread (in /usr/lib64/libcurl.so.4.2.0) by 0x55C4BF5: curl_thread_create_thunk (in /usr/lib64/libcurl.so.4.2.0) by 0x57E1EBE: start_thread (pthread_create.c:308) by 0x5AE932C: clone (clone.S:114) Address 0x9393ff0 is on thread 5's stack Uninitialised value was created by a stack allocation at 0x97B39E0: __libc_res_nsend (res_send.c:347) It looks like the iov[], reqs.msg_len and reqs.msg_hdr.msg_flags could be uninitialized there. A simple memset to zero resolves the warning and seems sensible enough, but there may be more sensible values to which these members could be explicitly set by somebody who knows the code.
It's a spurious warning by valgrind since relevant fields are initialized correctly. It does not make sense to add cruft just to make valgrind happy.
(In reply to comment #1) > It's a spurious warning by valgrind since relevant fields are initialized > correctly. It does not make sense to add cruft just to make valgrind happy. I can see that assessment. But it is also a spurious warning that given the code path many many many developers will have to see, worry about, spend time analyzing, and finally be able to mark off their list of potential sources for whatever oddity they were debugging once they've figured out what glibc is doing, and then they add a valgrind suppression to their codebase. That leads to a lot of extra work glibc causes other developers and leads to cruft in their codebases. Worst case the glibc code evolves somehow to actually use the uninitialized fields and rather than valgrind-using developers helping catch this, they've suppressed it already at their end. This is not positive for overall distro supportability and maintenance. If the memset is cruft, the fields unused in this code path are cruft. Would you find more acceptable the patch to differentiate the data structures used in the send and receive paths, along with all the associated code changes to the places that would need to use the new data structures?
(In reply to comment #2) > glibc causes other developers and leads to cruft in their codebases. Worst > case the glibc code evolves somehow to actually use the uninitialized fields > and rather than valgrind-using developers helping catch this, they've > suppressed it already at their end. This is not positive for overall distro > supportability and maintenance. In this particular case, something like this happening is unlikely because of the specific way in which the code works. It is interface code between the kernel and glibc, where the kernel initializes the uninitialized values. One could argue that initializing these values would mask a kernel bug. > If the memset is cruft, the fields unused in this code path are cruft. Would > you find more acceptable the patch to differentiate the data structures used in > the send and receive paths, along with all the associated code changes to the > places that would need to use the new data structures? I personally find any addition of code to this path unacceptable because it is unnecessary. If you are looking for consensus in the glibc community, then you can post your patch on the libc-alpha mailing list. Please follow the guidelines given in this link to post: http://sourceware.org/glibc/wiki/Contribution%20checklist
Tim, Valgrind has a way for dealing with false positives of this nature; specifically the suppressions need updating to handle this case. This really should be dealt with by the valgrind maintainers.
Actually, for the record it looks like this was considered a bug in valgrind, on grounds that it's mishandling the syscall, and fixed there: https://bugs.kde.org/show_bug.cgi?id=315441