[PATCH] Support the assume_aligned function attribute

Richard Smith richard at metafoo.co.uk
Sat Sep 6 21:01:21 PDT 2014


================
Comment at: lib/Sema/SemaDeclAttr.cpp:1130
@@ -1129,2 +1129,3 @@
+bool Sema::isValidPointerAttrType(QualType T) {
   T = T.getNonReferenceType();
 
----------------
hfinkel wrote:
> rsmith wrote:
> > hfinkel wrote:
> > > rsmith wrote:
> > > > Is this really appropriate for your attribute? (It makes almost no sense for `__attribute__((nonnull))` either, but I couldn't find anyone at Apple to tell me what the relevant radar says, so I have no idea why we have this "feature".)
> > > Nice :-) -- Honestly, I don't have a strong opinion regarding whether or not we strip references, but I like the idea of consistency between the two attributes. The set of returned pointers about which I can assert something about the alignment should be the same as that about which I can assert a nonnull property.
> > > 
> > > That having been said, it looks like the CodeGen for returns_nonnull that adds the nonnull IR attribute (and the associated UBSan code) does not handle references-to-pointers correctly. I'll fix that as a follow-up too.
> > It seems really weird for either of these to look through references to the referenced pointer. And it's ambiguous: does the attribute apply to the reference or to the pointer that the reference refers to? (For the nonnull attribute, GCC and Clang make different choices here, at least in the cases where you can persuade GCC not to reject the attribute!)
> Indeed, unlike nonnull, this does become ambiguous. Perhaps it is best to prohibit it? 
> 
> How do Clang and GCC make different choices for nonnull; is that a bug? Because a reference cannot be null, it would seem that it should apply to the pointer being bound.
Here's what I can piece together:

 * GCC does not intend to support `nonnull` on reference parameters. It rejects `nonnull(N)` where parameter `N` is a reference, *but* if you apply `nonnull` with no `N`, it applies to both pointer and reference parameters, and happens to have the semantics of checking for "null references".
 * Clang made up its own thing in response to some non-publicly-visible radar issue, where it treats the `nonnull` attribute on references-to-pointers as applying to the pointer within the reference. This makes little sense to me, but there it is. We naturally can't optimize on the basis of `nonnull` on a reference-to-pointer like we can on a real pointer parameter/return value (we have no IR representation for that).

It seems our options are to either not support `assume_aligned` on references (which would be a shame, because it's natural and useful there just as it is for pointers), or make `nonnull` and `assume_aligned` inconsistent on pointers-to-references, or remove our (dubious, IMO) extension of the GNU `nonnull` semantics. I'm not happy with the third option since I don't know what the motivation for the extension is nor how many people might be relying on it, but either of the first two seem fine to me. I don't think the connection between `nonnull` and `assume_aligned` is strong enough that the difference would be jarring for people.

http://reviews.llvm.org/D4601






More information about the cfe-commits mailing list