rsmith added inline comments.

================
Comment at: clang/docs/ReleaseNotes.rst:63
 
+* As per C++ and C Standards (C++: ``[expr.add]``; C17: 6.5.6p8), applying
+  non-zero offset to ``nullptr`` (or making non-``nullptr`` a ``nullptr``,
----------------
In C, even adding 0 to a null pointer is undefined. Did this change in C17?


================
Comment at: clang/docs/ReleaseNotes.rst:68
+  (`D66608 <https://reviews.llvm.org/D66608>`_ ``[InstCombine] icmp eq/ne (gep
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM middle-end uses those
+  guarantees for transformations. If the source contains such UB's, said code
----------------
LLVM -> the LLVM


================
Comment at: clang/docs/ReleaseNotes.rst:69
+  inbounds P, Idx..), null -> icmp eq/ne P, null``) LLVM middle-end uses those
+  guarantees for transformations. If the source contains such UB's, said code
+  may now be miscompiled. Undefined Behaviour Sanitizer
----------------
"undefined behavior", like (eg) "code", is generally treated as an uncountable 
noun, so this should be "If the source contains such undefined behavior, said 
code [...]" rather than "If the source contains such undefined behavior*s*, 
said code [...]".


================
Comment at: clang/docs/ReleaseNotes.rst:238
 
-- ...
+- * ``pointer-overflow`` check was extended added to catch the cases where
+    a non-zero offset being applied, either to a ``nullptr``, or the result
----------------
Reusing this group seems a little surprising, since the new checks don't seem 
to have anything to do with overflow. Is the general idea that this warning 
identifies places where pointer artihmetic leaves the complete object (where, 
for now, we only catch the case where it wraps around the address space or 
leaves / reaches a hypothetical size-0 object at the null address)?


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4659
+  // and only in the default address space
+  bool ShallPerformOverflowCheck =
+      !isa<llvm::Constant>(GEPVal) && PtrTy->getPointerAddressSpace() == 0;
----------------
Remove the `Shall`s here. They don't add anything.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67122/new/

https://reviews.llvm.org/D67122



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

Reply via email to