<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="moz-cite-prefix">On 09/22/2017 01:45 PM, Sylvestre Ledru
wrote:<br>
</div>
<blockquote
cite="mid:523758f8-caac-7601-06ea-898c5f0a9403@debian.org"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<p><br>
</p>
<br>
<div class="moz-cite-prefix">On 22/09/2017 20:27, Hal Finkel
wrote:<br>
</div>
<blockquote
cite="mid:739a4d69-383f-d51b-44ed-c78025d559e9@anl.gov"
type="cite">
<p><br>
</p>
<div class="moz-cite-prefix">On 09/22/2017 01:09 PM, Kaylor,
Andrew wrote:<br>
</div>
<blockquote
cite="mid:0983E6C011D2DC4188F8761B533492DE823BAEF0@ORSMSX115.amr.corp.intel.com"
type="cite">
<pre wrap="">The reason I introduced this patch to begin with is that there are circumstances under which the optimizer will eliminate loads from addresses that were generated based on the null pointer arithmetic (because clang previously emitted a null-based GEP and still will in the Firefox case because it's using subtraction). It would seem that the Firefox case won't ever dereference the pointer it is creating this way, so it should be safe from the optimization I was seeing.
On the other hand, what the warning says is true, right? I believe clang will produce an inbounds GEP in the Firefox case and the LLVM language reference says, "The only in bounds address for a null pointer in the default address-space is the null pointer itself." So it's entirely possible that some optimization will interpret the result of the GEP generated to represent '(((char*)0)-1)' as a poison value.
-Andy</pre>
</blockquote>
<br>
I agree. The warning seems good here. As I recall, doing pointer
arithmetic on the null pointer is UB (even if we never
dereference it).<br>
<br>
For convenience, it looks like this:<br>
<br>
<blockquote type="cite">
<pre class="comment-text " id="ct-0" style="font-size: small; white-space: pre-wrap; line-height: 1.2; font-family: "Droid Sans Mono", Menlo, Monaco, "Courier New", monospace; background: rgb(255, 255, 255); color: rgb(34, 34, 34); margin: 1px 0px 0px; overflow: auto; padding: 8px; border-top: 1px solid rgb(221, 221, 221); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;">pointer arithmetic on a null pointer has undefined behavior if the offset is nonzero [-Werror,-Wnull-pointer-arithmetic]
return net_FindCharInSet(str, NET_MAX_ADDRESS, set);
^~~~~~~~~~~~~~~
/data/jenkins/workspace/firefox-clang-last/obj-x86_64-pc-linux-gnu/dist/include/nsURLHelper.h:224:36: note: expanded from macro 'NET_MAX_ADDRESS'
#define NET_MAX_ADDRESS (((char*)0)-1)</pre>
</blockquote>
<br>
</blockquote>
To be clear, I wasn't arguing! <br>
I was just giving feedback about this new warning.<br>
</blockquote>
<br>
Sounds good :-)<br>
<br>
<blockquote
cite="mid:523758f8-caac-7601-06ea-898c5f0a9403@debian.org"
type="cite"> <br>
By the way, maybe we should add that to the release notes? <a
moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://clang.llvm.org/docs/ReleaseNotes.html">https://clang.llvm.org/docs/ReleaseNotes.html</a><br>
</blockquote>
<br>
I agree. That would be a good idea.<br>
<br>
-Hal<br>
<br>
<blockquote
cite="mid:523758f8-caac-7601-06ea-898c5f0a9403@debian.org"
type="cite"> <br>
Sylvestre<br>
<br>
</blockquote>
<br>
<pre class="moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</body>
</html>