[PATCH] D54589: [clang][UBSan] Sanitization for alignment assumptions.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: docs/UndefinedBehaviorSanitizer.rst:75
   -  ``-fsanitize=alignment``: Use of a misaligned pointer or creation
-     of a misaligned reference.
+     of a misaligned reference. Also sanitizes assume-aligned-like attributes.
   -  ``-fsanitize=bool``: Load of a ``bool`` value which is neither
----------------
`assume_aligned`-like


================
Comment at: docs/UndefinedBehaviorSanitizer.rst:198
+assume-aligned-like attributes), `object-size``, and ``vptr`` checks do not
+apply to pointers to types with the ``volatile`` qualifier
 
----------------
Is there a reason for this exception?


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1895
 
-    EmitAlignmentAssumption(PtrValue, Alignment, OffsetValue);
+    EmitAlignmentAssumption(PtrValue, Ptr, {/*The expr loc is sufficient.*/},
+                            Alignment, OffsetValue);
----------------
Is this `{}`-initializing a `SourceLocation`?  Please use `SourceLocation()` instead and put the comment before it.


================
Comment at: lib/CodeGen/CodeGenFunction.cpp:2467
+    llvm::Value *OffsetValue, llvm::Value *TheCheck,
+    llvm::Instruction *Assumption) {
+  assert(Assumption && isa<llvm::CallInst>(Assumption) &&
----------------
What's the deal with the two different source locations?


================
Comment at: lib/CodeGen/CodeGenFunction.h:2840
+    if (isa<CastExpr>(E))
+      E = cast<CastExpr>(E)->getSubExprAsWritten();
+    QualType Ty = E->getType();
----------------
`dyn_cast`, please.  And these two methods should really be defined out-of-line.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54589