================
Comment at: lib/CodeGen/CGExpr.cpp:3379-3382
@@ -3378,2 +3378,6 @@
- return EmitCall(FnInfo, Callee, ReturnValue, Args, TargetDecl);
+ RValue Ret = EmitCall(FnInfo, Callee, ReturnValue, Args, TargetDecl);
+
+ if (Ret.isScalar() && TargetDecl) {
+ if (const auto *AA = TargetDecl->getAttr<AssumeAlignedAttr>()) {
+ llvm::Value *OffsetValue = nullptr;
----------------
rsmith wrote:
> Is this really the right place for this code, rather than in the called
> `EmitCall` function? Various places call the lower-level overload directly.
When I first looked, I thought that there were no other relevant callers, but
maybe EmitCallAndReturnForThunk and EmitForwardingCallToLambda would need this
handling too?
Because of the way that the lower-level EmitCall is structured, I'd need to
wrap it (it has too many early returns). What would I call it?
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1130
@@ -1129,2 +1129,3 @@
+bool Sema::isValidPointerAttrType(QualType T) {
T = T.getNonReferenceType();
----------------
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.
================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:203-205
@@ +202,5 @@
+ const AssumeAlignedAttr *AssumeAligned =
dyn_cast<AssumeAlignedAttr>(TmplAttr);
+ if (AssumeAligned && (AssumeAligned->getAlignment()->isValueDependent() ||
+ (AssumeAligned->getOffset() &&
+ AssumeAligned->getOffset()->isValueDependent()))) {
+ instantiateDependentAssumeAlignedAttr(*this, TemplateArgs,
AssumeAligned, New);
----------------
rsmith wrote:
> You shouldn't have these 'is dependent' checks here; the other code above and
> below is wrong to include them. Testcase:
>
> template<typename T> __attribute__((assume_aligned(sizeof(int(T()))))) T
> *f();
> void *p = f<void>();
>
> This should result in a hard error, because `int(void())` is invalid, but I
> think you'll accept it, because `sizeof(int(T()))` is not value-dependent, so
> you never actually perform the substitution.
Interestingly, the current code does generate an error:
invalid application of 'sizeof' to a function type
Removing the type-dependent checks does not seem to change any of the current
behavior.
http://reviews.llvm.org/D4601
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
