On 09/22/2017 01:09 PM, Kaylor, Andrew wrote:
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

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).

For convenience, it looks like this:

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)

 -Hal


-----Original Message-----
From: Sylvestre Ledru via Phabricator [mailto:revi...@reviews.llvm.org]
Sent: Friday, September 22, 2017 9:02 AM
To: Kaylor, Andrew <andrew.kay...@intel.com>; rjmcc...@gmail.com; 
rich...@metafoo.co.uk; efrie...@codeaurora.org
Cc: sylves...@debian.org; Ivchenko, Alexander <alexander.ivche...@intel.com>; 
hfin...@anl.gov; mcros...@codeaurora.org; david.majne...@gmail.com; 
cfe-commits@lists.llvm.org
Subject: [PATCH] D37042: Teach clang to tolerate the 'p = nullptr + n' idiom 
used by glibc

sylvestre.ledru added a comment.

For the record, Firefox was using this trick. This patch is breaking a ci build 
(clang trunk + warning as errors) More information here: 
https://bugzilla.mozilla.org/show_bug.cgi?id=1402362


Repository:
   rL LLVM

https://reviews.llvm.org/D37042




--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to