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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.


================
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
 
----------------
rjmccall wrote:
> lebedev.ri wrote:
> > rjmccall wrote:
> > > Is there a reason for this exception?
> > Are you asking about the LHS of the diff, or about adding an exception to that for this sanitizer?
> > I'm adding an exception here because i don't know what should be done here.
> > Does it make sense to emit an assumptions for volatile pointers, but do not sanitize these assumptions?
> > Are you asking about the LHS of the diff, or about adding an exception to that for this sanitizer?
> 
> I'm asking about adding a new exception for one portion of one sanitizer.
> 
> > I'm adding an exception here because i don't know what should be done here.
> 
> Okay, that's not a good enough reason.
> 
> The overall rule for annotation-based language/tool designs is that explicit/specific/close wins over implicit/general/distant.  So the question is: how does that rule apply here?
> 
> You can't end up with a pointer to `volatile` completely implicitly — at some point, a programmer was explicit about requesting `volatile` semantics, and that has somehow propagated to this particular access/assumption site.  So that's a pretty strong piece of information, and if we have a general rule for the sanitizers that `volatile` bypasses the check, it's generally a good idea to be consistent with that.
> 
> On the other hand, these assumption annotations are themselves always explicit, right?  If you have to be explicit about putting `align_value` on a specific pointer variable, and that pointer just happens to be `volatile`-qualified, we probably *shouldn't* bypass the check: that's about an explicit, specific, and close as a programmer can get, just short of literally writing it on every access to the variable.  The only counter-argument is that maybe the pointer is only `volatile`-qualified because of template instantiation or something.
> 
> So I think it makes sense to enforce it for at least some of these annotations and/or builtin calls, but we should be clear about *why* it makes sense.  However, it's possible that I may be misunderstanding part of the motivation behind the general exception for `volatile`, so you should reach out for input from the UBSan etc. people.
Tried with nullability attributes https://godbolt.org/z/rJUb9U
They do not bypass this "ignore volatile".
So i guess i will drop this exception.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54589