[clang] Annotating C++'s `operator new` with more attributes
Summary:
Right now we annotate C++'s `operator new` with `noalias` attribute,
which very much is healthy for optimizations.
However as per [[ http://eel.is/c++draft/basic.stc.dynamic.allocation | `[basic.stc.dynamic.allocation]` ]],
there are more promises on global `operator new`, namely:
* non-`std::nothrow_t` `operator new` *never* returns `nullptr`
* If `std::align_val_t align` parameter is taken, the pointer will also be `align`-aligned
* ~~global `operator new`-returned pointer is `__STDCPP_DEFAULT_NEW_ALIGNMENT__`-aligned ~~ It's more caveated than that.
Supplying this information may not cause immediate landslide effects
on any specific benchmarks, but it for sure will be healthy for optimizer
in the sense that the IR will better reflect the guarantees provided in the source code.
The caveat is `-fno-assume-sane-operator-new`, which currently prevents emitting `noalias`
attribute, and is automatically passed by Sanitizers ([[ https://bugs.llvm.org/show_bug.cgi?id=16386 | PR16386 ]]) - should it also cover these attributes?
The problem is that the flag is back-end-specific, as seen in `test/Modules/explicit-build-flags.cpp`.
But while it is okay to add `noalias` metadata in backend, we really should be adding at least
the alignment metadata to the AST, since that allows us to perform sema checks on it.
Reviewers: erichkeane, rjmccall, jdoerfert, eugenis, rsmith
Reviewed By: rsmith
Subscribers: xbolva00, jrtc27, atanasyan, nlopes, cfe-commits
Tags: #llvm, #clang
Differential Revision: https://reviews.llvm.org/D73380
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 216137b..7625acd 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -2952,7 +2952,8 @@
return (proto->getParamType(1).getCanonicalType() == Context.VoidPtrTy);
}
-bool FunctionDecl::isReplaceableGlobalAllocationFunction(bool *IsAligned) const {
+bool FunctionDecl::isReplaceableGlobalAllocationFunction(
+ Optional<unsigned> *AlignmentParam, bool *IsNothrow) const {
if (getDeclName().getNameKind() != DeclarationName::CXXOperatorName)
return false;
if (getDeclName().getCXXOverloadedOperator() != OO_New &&
@@ -2999,9 +3000,9 @@
// In C++17, the next parameter can be a 'std::align_val_t' for aligned
// new/delete.
if (Ctx.getLangOpts().AlignedAllocation && !Ty.isNull() && Ty->isAlignValT()) {
- if (IsAligned)
- *IsAligned = true;
Consume();
+ if (AlignmentParam)
+ *AlignmentParam = Params;
}
// Finally, if this is not a sized delete, the final parameter can
@@ -3010,8 +3011,11 @@
Ty = Ty->getPointeeType();
if (Ty.getCVRQualifiers() != Qualifiers::Const)
return false;
- if (Ty->isNothrowT())
+ if (Ty->isNothrowT()) {
+ if (IsNothrow)
+ *IsNothrow = true;
Consume();
+ }
}
return Params == FPT->getNumParams();
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 98365db..42d5467 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1906,6 +1906,13 @@
if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) {
AddAttributesFromFunctionProtoType(
getContext(), FuncAttrs, Fn->getType()->getAs<FunctionProtoType>());
+ if (AttrOnCallSite && Fn->isReplaceableGlobalAllocationFunction()) {
+ // A sane operator new returns a non-aliasing pointer.
+ auto Kind = Fn->getDeclName().getCXXOverloadedOperator();
+ if (getCodeGenOpts().AssumeSaneOperatorNew &&
+ (Kind == OO_New || Kind == OO_Array_New))
+ RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+ }
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn);
const bool IsVirtualCall = MD && MD->isVirtual();
// Don't use [[noreturn]], _Noreturn or [[no_builtin]] for a call to a
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index d4ed486..652efd8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1875,15 +1875,6 @@
// default, only if it is invoked by a new-expression or delete-expression.
F->addAttribute(llvm::AttributeList::FunctionIndex,
llvm::Attribute::NoBuiltin);
-
- // A sane operator new returns a non-aliasing pointer.
- // FIXME: Also add NonNull attribute to the return value
- // for the non-nothrow forms?
- auto Kind = FD->getDeclName().getCXXOverloadedOperator();
- if (getCodeGenOpts().AssumeSaneOperatorNew &&
- (Kind == OO_New || Kind == OO_Array_New))
- F->addAttribute(llvm::AttributeList::ReturnIndex,
- llvm::Attribute::NoAlias);
}
if (isa<CXXConstructorDecl>(FD) || isa<CXXDestructorDecl>(FD))
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4c088aa..7fe3213 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14475,6 +14475,77 @@
return FD;
}
+/// If this function is a C++ replaceable global allocation function
+/// (C++2a [basic.stc.dynamic.allocation], C++2a [new.delete]),
+/// adds any function attributes that we know a priori based on the standard.
+///
+/// We need to check for duplicate attributes both here and where user-written
+/// attributes are applied to declarations.
+void Sema::AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(
+ FunctionDecl *FD) {
+ if (FD->isInvalidDecl())
+ return;
+
+ if (FD->getDeclName().getCXXOverloadedOperator() != OO_New &&
+ FD->getDeclName().getCXXOverloadedOperator() != OO_Array_New)
+ return;
+
+ Optional<unsigned> AlignmentParam;
+ bool IsNothrow = false;
+ if (!FD->isReplaceableGlobalAllocationFunction(&AlignmentParam, &IsNothrow))
+ return;
+
+ // C++2a [basic.stc.dynamic.allocation]p4:
+ // An allocation function that has a non-throwing exception specification
+ // indicates failure by returning a null pointer value. Any other allocation
+ // function never returns a null pointer value and indicates failure only by
+ // throwing an exception [...]
+ if (!IsNothrow && !FD->hasAttr<ReturnsNonNullAttr>())
+ FD->addAttr(ReturnsNonNullAttr::CreateImplicit(Context, FD->getLocation()));
+
+ // C++2a [basic.stc.dynamic.allocation]p2:
+ // An allocation function attempts to allocate the requested amount of
+ // storage. [...] If the request succeeds, the value returned by a
+ // replaceable allocation function is a [...] pointer value p0 different
+ // from any previously returned value p1 [...]
+ //
+ // However, this particular information is being added in codegen,
+ // because there is an opt-out switch for it (-fno-assume-sane-operator-new)
+
+ // C++2a [basic.stc.dynamic.allocation]p2:
+ // An allocation function attempts to allocate the requested amount of
+ // storage. If it is successful, it returns the address of the start of a
+ // block of storage whose length in bytes is at least as large as the
+ // requested size.
+ if (!FD->hasAttr<AllocSizeAttr>()) {
+ FD->addAttr(AllocSizeAttr::CreateImplicit(
+ Context, /*ElemSizeParam=*/ParamIdx(1, FD),
+ /*NumElemsParam=*/ParamIdx(), FD->getLocation()));
+ }
+
+ // C++2a [basic.stc.dynamic.allocation]p3:
+ // For an allocation function [...], the pointer returned on a successful
+ // call shall represent the address of storage that is aligned as follows:
+ // (3.1) If the allocation function takes an argument of type
+ // std::align_val_t, the storage will have the alignment
+ // specified by the value of this argument.
+ if (AlignmentParam.hasValue() && !FD->hasAttr<AllocAlignAttr>()) {
+ FD->addAttr(AllocAlignAttr::CreateImplicit(
+ Context, ParamIdx(AlignmentParam.getValue(), FD), FD->getLocation()));
+ }
+
+ // FIXME:
+ // C++2a [basic.stc.dynamic.allocation]p3:
+ // For an allocation function [...], the pointer returned on a successful
+ // call shall represent the address of storage that is aligned as follows:
+ // (3.2) Otherwise, if the allocation function is named operator new[],
+ // the storage is aligned for any object that does not have
+ // new-extended alignment ([basic.align]) and is no larger than the
+ // requested size.
+ // (3.3) Otherwise, the storage is aligned for any object that does not
+ // have new-extended alignment and is of the requested size.
+}
+
/// Adds any function attributes that we know a priori based on
/// the declaration of this function.
///
@@ -14575,6 +14646,8 @@
}
}
+ AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(FD);
+
// If C++ exceptions are enabled but we are told extern "C" functions cannot
// throw, add an implicit nothrow attribute to any extern "C" function we come
// across.
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 9e5c735..44fd699 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1788,8 +1788,9 @@
return false;
if (FD.isDefined())
return false;
- bool IsAligned = false;
- if (FD.isReplaceableGlobalAllocationFunction(&IsAligned) && IsAligned)
+ Optional<unsigned> AlignmentParam;
+ if (FD.isReplaceableGlobalAllocationFunction(&AlignmentParam) &&
+ AlignmentParam.hasValue())
return true;
return false;
}
@@ -2943,6 +2944,7 @@
Alloc->setParams(ParamDecls);
if (ExtraAttr)
Alloc->addAttr(ExtraAttr);
+ AddKnownFunctionAttributesForReplaceableGlobalAllocationFunction(Alloc);
Context.getTranslationUnitDecl()->addDecl(Alloc);
IdResolver.tryAddTopLevelDecl(Alloc, Name);
};