<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 3 January 2017 at 16:03, James Y Knight <span dir="ltr"><<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">BTW, I just ran this test program (built with "gcc -ggdb -fno-builtin -Wall test.c"), to check if the library functions actually do all succeed when given null values, or if some might segfault. It would appear that in the current library implementation, on x86-64, everything can be null when given a zero length, other than the 1st arg to strncat.</div></blockquote><div><br></div><div>My bad -- the nonnull on parameter 1 of strncat is correct, it's the nonnull on parameter 2 that is dubious.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div><font face="monospace, monospace">#define _GNU_SOURCE</font></div><div><font face="monospace, monospace">#include <string.h></font></div><div><font face="monospace, monospace">#include <locale.h></font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">char *a = 0, *b = 0;</font></div><div><font face="monospace, monospace">char valid[10];</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">int main(int argc, char ** argv) {</font></div><div><font face="monospace, monospace"> locale_t ll = newlocale(LC_CTYPE, "", NULL);</font></div><div><font face="monospace, monospace"> int result = 0;</font></div><div><font face="monospace, monospace"> size_t count = argc-1;</font></div><div><font face="monospace, monospace"> memcpy(a, b, count);</font></div><div><font face="monospace, monospace"> memmove(a, b, count);</font></div><div><font face="monospace, monospace"> memset(a, 4, count);</font></div><div><font face="monospace, monospace"> result += memcmp(a, b, count);</font></div><div><font face="monospace, monospace"> result += memchr(a, 4, count) != 0;</font></div><div><font face="monospace, monospace"> result += memrchr(a, 4, count) != 0;</font></div><div><font face="monospace, monospace"> strncpy(a, b, count);</font></div><div><font face="monospace, monospace"> strncat(valid, b, count); // crashes if 1st arg is not valid</font></div><div><font face="monospace, monospace"> result += strncmp(a, b, count);</font></div><div><font face="monospace, monospace"> strndup(a, count);</font></div><div><font face="monospace, monospace"> memccpy(a, b, 4, count);</font></div><div><font face="monospace, monospace"> mempcpy(a, b, count);</font></div><div><font face="monospace, monospace"> result += strnlen(a, count);</font></div><div><font face="monospace, monospace"> bcopy(a, b, count);</font></div><div><font face="monospace, monospace"> bzero(a, count);</font></div><div><font face="monospace, monospace"> result += bcmp(a, b, count);</font></div><div><font face="monospace, monospace"> memfrob(a, count);</font></div><div><font face="monospace, monospace"> result += strncasecmp(a, b, count);</font></div><div><font face="monospace, monospace"> result += strncasecmp_l(a, b, count, ll);</font></div><div><font face="monospace, monospace"> stpncpy(a, b, count);</font></div><div><font face="monospace, monospace"> return result;</font></div><div><font face="monospace, monospace">}</font></div></div><div><br></div></div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 3, 2017 at 6:55 PM, James Y Knight <span dir="ltr"><<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Tue, Jan 3, 2017 at 6:19 PM, Richard Smith via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 3 January 2017 at 14:46, Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="m_4694664618634639327m_-323114221102625406m_-5509522498175200707gmail-HOEnZb"><div class="m_4694664618634639327m_-323114221102625406m_-5509522498175200707gmail-h5">On 1/3/2017 2:06 PM, Richard Smith via cfe-dev wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Via <a href="https://reviews.llvm.org/D27855" rel="noreferrer" target="_blank">https://reviews.llvm.org/D2785<wbr>5</a>, LLVM is likely to gain the ability to delete null checks in callers based on __attribute__((nonnull)) on the callee. This interacts badly with glibc's choice to mark the pointer parameters of memcpy, memmove, etc. as __attribute__((nonnull)) -- it is relatively common for programs to pass a null pointer and a zero size to these functions, and in practice libc implementations accept such usage. Indeed, LLVM's lowering of @llvm.memcpy intrinsics relies on these calls working.<br>
<br>
Deleting a null pointer check on p after a memcpy(p, q, 0) call seems extremely user-hostile, and very unlikely to result in a valuable improvement to the program, so I propose that we stop lowering __attribute__((nonnull)) on these builtin library functions to the llvm nonnull attribute.<br>
<br>
(Chandler is working on a paper for the C++ committee proposing to give these functions defined behavior when given a null pointer and a zero size, but optimizing on the basis of these particular nonnull attributes seems like a bad idea regardless of the C or C++ committees' decisions.)<br>
<br>
Thoughts?<br>
</blockquote>
<br></div></div>
"memcpy, memmove, etc." seems to be hiding some details here; do you have a complete list of functions you want to special-case? There are a lot of functions which could potentially go on that list, and some of them are POSIX or GNU extensions.<br></blockquote><div><br></div></span><div>The following glibc functions in <string.h> take both a pointer and a size providing an upper limit on the number of bytes that may be accessed through that pointer, and mark the pointer as __attribute__((nonnull)):</div><div><br></div><div>ISO C:</div><div> memcpy, memmove, memset, memcmp, memchr, memrchr</div><div> strncpy [param 1 only], strncat [param 1 only], strncmp</div><div><br></div><div>Extensions:</div><div> strndup, memccpy, mempcpy, strnlen, bcopy, bzero, bcmp, memfrob,</div><div> strncasecmp, strncasecmp_l [not parameter 4], stpncpy<br></div><div><br></div><div>I would be interested in covering at least the ISO C functions. I don't have strong opinions about the extensions (any code built using those is likely also compiled with GCC, and so presumably has to fix this regardless).</div></div></div></div></blockquote><div><br></div></span><div>IMO, ignoring nonnull on all of these would be a fine idea -- but if we go down this path, please do open a bug against glibc asking for them to be removed, pointing out that,</div><div>a. it is harmful to mark them nonnull because <evidence>.</div><div>b. clang is going to start ignoring the attributes on those functions even if glibc doesn't remove them, because of a.</div><div>c. there will be a standards proposal to stop requiring nonnull, too.</div><div><br></div><div>It's worth at least an attempt to persuade them it's wrong to have those attributes.</div></div></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>