I've reverted this change as it was causing ASan/MSan failures in check-clang, e.g. take a look at the bottom 2 failures here: http://lab.llvm.org:8014/builders/sanitizer-x86_64-linux-bootstrap/builds/124/steps/check-clang%20asan/logs/stdio or here http://lab.llvm.org:8014/builders/sanitizer-x86_64-linux-fast/builds/126/steps/check-clang%20msan/logs/stdio
On Mon, Oct 28, 2019 at 9:30 AM Guillaume Chatelet via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > Author: Guillaume Chatelet > Date: 2019-10-28T17:30:11+01:00 > New Revision: bd87916109483d33455cbf20da2309197b983cdd > > URL: > https://github.com/llvm/llvm-project/commit/bd87916109483d33455cbf20da2309197b983cdd > DIFF: > https://github.com/llvm/llvm-project/commit/bd87916109483d33455cbf20da2309197b983cdd.diff > > LOG: [clang] Add no_builtin attribute > > Summary: > This is a follow up on https://reviews.llvm.org/D61634 > This patch is simpler and only adds the no_builtin attribute. > > Reviewers: tejohnson, courbet, theraven, t.p.northover, jdoerfert > > Subscribers: mgrang, cfe-commits > > Tags: #clang > > Differential Revision: https://reviews.llvm.org/D68028 > > Added: > clang/test/CodeGen/no-builtin.cpp > clang/test/Sema/no-builtin.cpp > > Modified: > clang/include/clang/AST/Decl.h > clang/include/clang/Basic/Attr.td > clang/include/clang/Basic/AttrDocs.td > clang/include/clang/Basic/DiagnosticSemaKinds.td > clang/lib/CodeGen/CGCall.cpp > clang/lib/Sema/SemaDecl.cpp > clang/lib/Sema/SemaDeclAttr.cpp > clang/test/Misc/pragma-attribute-supported-attributes-list.test > > Removed: > > > > > ################################################################################ > diff --git a/clang/include/clang/AST/Decl.h > b/clang/include/clang/AST/Decl.h > index b3e7a570fd6d..16094c0988fa 100644 > --- a/clang/include/clang/AST/Decl.h > +++ b/clang/include/clang/AST/Decl.h > @@ -2031,6 +2031,10 @@ class FunctionDecl : public DeclaratorDecl, > /// > /// This does not determine whether the function has been defined > (e.g., in a > /// previous definition); for that information, use isDefined. > + /// > + /// Note: the function declaration does not become a definition until > the > + /// parser reaches the definition, if called before, this function will > return > + /// `false`. > bool isThisDeclarationADefinition() const { > return isDeletedAsWritten() || isDefaulted() || Body || > hasSkippedBody() || > isLateTemplateParsed() || willHaveBody() || hasDefiningAttr(); > > diff --git a/clang/include/clang/Basic/Attr.td > b/clang/include/clang/Basic/Attr.td > index 4557a614d361..d5018f444e1c 100644 > --- a/clang/include/clang/Basic/Attr.td > +++ b/clang/include/clang/Basic/Attr.td > @@ -3427,3 +3427,10 @@ def ObjCExternallyRetained : InheritableAttr { > let Subjects = SubjectList<[NonParmVar, Function, Block, ObjCMethod]>; > let Documentation = [ObjCExternallyRetainedDocs]; > } > + > +def NoBuiltin : Attr { > + let Spellings = [Clang<"no_builtin">]; > + let Args = [VariadicStringArgument<"BuiltinNames">]; > + let Subjects = SubjectList<[Function]>; > + let Documentation = [NoBuiltinDocs]; > +} > > diff --git a/clang/include/clang/Basic/AttrDocs.td > b/clang/include/clang/Basic/AttrDocs.td > index 6e79d0bb3631..9d0d27407573 100644 > --- a/clang/include/clang/Basic/AttrDocs.td > +++ b/clang/include/clang/Basic/AttrDocs.td > @@ -4413,3 +4413,40 @@ and is not a general mechanism for declaring > arbitrary aliases for > clang builtin functions. > }]; > } > + > +def NoBuiltinDocs : Documentation { > + let Category = DocCatFunction; > + let Content = [{ > +.. Note:: This attribute is not yet fully implemented, it is validated > but has > +no effect on the generated code. > + > +The ``__attribute__((no_builtin))`` is similar to the ``-fno-builtin`` > flag > +except it is specific to the body of a function. The attribute may also be > +applied to a virtual function but has no effect on the behavior of > overriding > +functions in a derived class. > + > +It accepts one or more strings corresponding to the specific names of the > +builtins to disable (e.g. "memcpy", "memset"). > +If the attribute is used without parameters it will disable all buitins at > +once. > + > +.. code-block:: c++ > + > + // The compiler is not allowed to add any builtin to foo's body. > + void foo(char* data, size_t count) __attribute__((no_builtin)) { > + // The compiler is not allowed to convert the loop into > + // `__builtin_memset(data, 0xFE, count);`. > + for (size_t i = 0; i < count; ++i) > + data[i] = 0xFE; > + } > + > + // The compiler is not allowed to add the `memcpy` builtin to bar's > body. > + void bar(char* data, size_t count) > __attribute__((no_builtin("memcpy"))) { > + // The compiler is allowed to convert the loop into > + // `__builtin_memset(data, 0xFE, count);` but cannot generate any > + // `__builtin_memcpy` > + for (size_t i = 0; i < count; ++i) > + data[i] = 0xFE; > + } > + }]; > +} > > diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td > b/clang/include/clang/Basic/DiagnosticSemaKinds.td > index db877a46c300..20670a98fe03 100644 > --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td > +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td > @@ -3624,6 +3624,15 @@ def err_attribute_overloadable_no_prototype : Error< > def err_attribute_overloadable_multiple_unmarked_overloads : Error< > "at most one overload for a given name may lack the 'overloadable' " > "attribute">; > +def warn_attribute_no_builtin_invalid_builtin_name : Warning< > + "'%0' is not a valid builtin name for %1">, > + InGroup<DiagGroup<"invalid-no-builtin-names">>; > +def err_attribute_no_builtin_wildcard_or_builtin_name : Error< > + "empty %0 cannot be composed with named ones">; > +def err_attribute_no_builtin_on_non_definition : Error< > + "%0 attribute is permitted on definitions only">; > +def err_attribute_no_builtin_on_defaulted_deleted_function : Error< > + "%0 attribute has no effect on defaulted or deleted functions">; > def warn_ns_attribute_wrong_return_type : Warning< > "%0 attribute only applies to %select{functions|methods|properties}1 > that " > "return %select{an Objective-C object|a pointer|a non-retainable > pointer}2">, > > diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp > index b74f6f942426..62e8fa037013 100644 > --- a/clang/lib/CodeGen/CGCall.cpp > +++ b/clang/lib/CodeGen/CGCall.cpp > @@ -1853,11 +1853,27 @@ void CodeGenModule::ConstructAttributeList( > if (const FunctionDecl *Fn = dyn_cast<FunctionDecl>(TargetDecl)) { > AddAttributesFromFunctionProtoType( > getContext(), FuncAttrs, > Fn->getType()->getAs<FunctionProtoType>()); > - // Don't use [[noreturn]] or _Noreturn for a call to a virtual > function. > - // These attributes are not inherited by overloads. > const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn); > - if (Fn->isNoReturn() && !(AttrOnCallSite && MD && MD->isVirtual())) > - FuncAttrs.addAttribute(llvm::Attribute::NoReturn); > + const bool IsVirtualCall = MD && MD->isVirtual(); > + // Don't use [[noreturn]], _Noreturn or [[no_builtin]] for a call > to a > + // virtual function. These attributes are not inherited by > overloads. > + if (!(AttrOnCallSite && IsVirtualCall)) { > + if (Fn->isNoReturn()) > + FuncAttrs.addAttribute(llvm::Attribute::NoReturn); > + > + if (const auto *NBA = TargetDecl->getAttr<NoBuiltinAttr>()) { > + bool HasWildcard = llvm::is_contained(NBA->builtinNames(), "*"); > + if (HasWildcard) > + FuncAttrs.addAttribute("no-builtins"); > + else > + for (StringRef BuiltinName : NBA->builtinNames()) { > + SmallString<32> AttributeName; > + AttributeName += "no-builtin-"; > + AttributeName += BuiltinName; > + FuncAttrs.addAttribute(AttributeName); > + } > + } > + } > } > > // 'const', 'pure' and 'noalias' attributed functions are also > nounwind. > > diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp > index 6202391ee0b8..aba7049b0a51 100644 > --- a/clang/lib/Sema/SemaDecl.cpp > +++ b/clang/lib/Sema/SemaDecl.cpp > @@ -9529,6 +9529,29 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator > &D, DeclContext *DC, > } > } > > + // Diagnose no_builtin attribute on function declaration that are not a > + // definition. > + // FIXME: We should really be doing this in > + // SemaDeclAttr.cpp::handleNoBuiltinAttr, unfortunately we only have > access to > + // the FunctionDecl and at this point of the code > + // FunctionDecl::isThisDeclarationADefinition() which always returns > `false` > + // because Sema::ActOnStartOfFunctionDef has not been called yet. > + if (const auto *NBA = NewFD->getAttr<NoBuiltinAttr>()) > + switch (D.getFunctionDefinitionKind()) { > + case FDK_Defaulted: > + case FDK_Deleted: > + Diag(NBA->getLocation(), > + diag::err_attribute_no_builtin_on_defaulted_deleted_function) > + << NBA->getSpelling(); > + break; > + case FDK_Declaration: > + Diag(NBA->getLocation(), > diag::err_attribute_no_builtin_on_non_definition) > + << NBA->getSpelling(); > + break; > + case FDK_Definition: > + break; > + } > + > return NewFD; > } > > > diff --git a/clang/lib/Sema/SemaDeclAttr.cpp > b/clang/lib/Sema/SemaDeclAttr.cpp > index abbd597a26d0..99eb23c3fe61 100644 > --- a/clang/lib/Sema/SemaDeclAttr.cpp > +++ b/clang/lib/Sema/SemaDeclAttr.cpp > @@ -1069,6 +1069,56 @@ static void handleDiagnoseIfAttr(Sema &S, Decl *D, > const ParsedAttr &AL) { > S.Context, AL, Cond, Msg, DiagType, ArgDependent, > cast<NamedDecl>(D))); > } > > +static void handleNoBuiltinAttr(Sema &S, Decl *D, const ParsedAttr &AL) { > + static constexpr const StringRef kWildcard = "*"; > + > + llvm::SmallVector<StringRef, 16> Names; > + bool HasWildcard = false; > + > + const auto AddBuiltinName = [&Names, &HasWildcard](StringRef Name) { > + if (Name == kWildcard) > + HasWildcard = true; > + Names.push_back(Name); > + }; > + > + // Add previously defined attributes. > + if (const auto *NBA = D->getAttr<NoBuiltinAttr>()) > + for (StringRef BuiltinName : NBA->builtinNames()) > + AddBuiltinName(BuiltinName); > + > + // Add current attributes. > + if (AL.getNumArgs() == 0) > + AddBuiltinName(kWildcard); > + else > + for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) { > + StringRef BuiltinName; > + SourceLocation LiteralLoc; > + if (!S.checkStringLiteralArgumentAttr(AL, I, BuiltinName, > &LiteralLoc)) > + return; > + > + if (Builtin::Context::isBuiltinFunc(BuiltinName.data())) > + AddBuiltinName(BuiltinName); > + else > + S.Diag(LiteralLoc, > diag::warn_attribute_no_builtin_invalid_builtin_name) > + << BuiltinName << AL.getAttrName()->getName(); > + } > + > + // Repeating the same attribute is fine. > + llvm::sort(Names); > + Names.erase(std::unique(Names.begin(), Names.end()), Names.end()); > + > + // Empty no_builtin must be on its own. > + if (HasWildcard && Names.size() > 1) > + S.Diag(D->getLocation(), > + diag::err_attribute_no_builtin_wildcard_or_builtin_name) > + << AL.getAttrName()->getName(); > + > + if (D->hasAttr<NoBuiltinAttr>()) > + D->dropAttr<NoBuiltinAttr>(); > + D->addAttr(::new (S.Context) > + NoBuiltinAttr(S.Context, AL, Names.data(), > Names.size())); > +} > + > static void handlePassObjectSizeAttr(Sema &S, Decl *D, const ParsedAttr > &AL) { > if (D->hasAttr<PassObjectSizeAttr>()) { > S.Diag(D->getBeginLoc(), diag::err_attribute_only_once_per_parameter) > << AL; > @@ -6608,6 +6658,9 @@ static void ProcessDeclAttribute(Sema &S, Scope > *scope, Decl *D, > case ParsedAttr::AT_DiagnoseIf: > handleDiagnoseIfAttr(S, D, AL); > break; > + case ParsedAttr::AT_NoBuiltin: > + handleNoBuiltinAttr(S, D, AL); > + break; > case ParsedAttr::AT_ExtVectorType: > handleExtVectorTypeAttr(S, D, AL); > break; > > diff --git a/clang/test/CodeGen/no-builtin.cpp > b/clang/test/CodeGen/no-builtin.cpp > new file mode 100644 > index 000000000000..3c5d681282da > --- /dev/null > +++ b/clang/test/CodeGen/no-builtin.cpp > @@ -0,0 +1,64 @@ > +// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -emit-llvm -o - %s | > FileCheck %s > + > +// CHECK-LABEL: define void @foo_no_mempcy() #0 > +extern "C" void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {} > + > +// CHECK-LABEL: define void @foo_no_mempcy_twice() #0 > +extern "C" void foo_no_mempcy_twice() > __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memcpy"))) > {} > + > +// CHECK-LABEL: define void @foo_no_builtins() #1 > +extern "C" void foo_no_builtins() __attribute__((no_builtin)) {} > + > +// CHECK-LABEL: define void @foo_no_mempcy_memset() #2 > +extern "C" void foo_no_mempcy_memset() > __attribute__((no_builtin("memset", "memcpy"))) {} > + > +// CHECK-LABEL: define void @separate_attrs() #2 > +extern "C" void separate_attrs() __attribute__((no_builtin("memset"))) > __attribute__((no_builtin("memcpy"))) {} > + > +// CHECK-LABEL: define void @separate_attrs_ordering() #2 > +extern "C" void separate_attrs_ordering() > __attribute__((no_builtin("memcpy"))) __attribute__((no_builtin("memset"))) > {} > + > +struct A { > + virtual int foo() const __attribute__((no_builtin("memcpy"))) { return > 1; } > + virtual ~A(); > +}; > + > +struct B : public A { > + int foo() const override __attribute__((no_builtin("memmove"))) { > return 2; } > + virtual ~B(); > +}; > + > +// CHECK-LABEL: define void @call_a_foo(%struct.A* %a) #3 > +extern "C" void call_a_foo(A *a) { > + // CHECK: %call = call i32 %2(%struct.A* %0) > + a->foo(); // virtual call is not annotated > +} > + > +// CHECK-LABEL: define void @call_b_foo(%struct.B* %b) #3 > +extern "C" void call_b_foo(B *b) { > + // CHECK: %call = call i32 %2(%struct.B* %0) > + b->foo(); // virtual call is not annotated > +} > + > +// CHECK-LABEL: define void @call_foo_no_mempcy() #3 > +extern "C" void call_foo_no_mempcy() { > + // CHECK: call void @foo_no_mempcy() #6 > + foo_no_mempcy(); // call gets annotated with "no-builtin-memcpy" > +} > + > +A::~A() {} // Anchoring A so A::foo() gets generated > +B::~B() {} // Anchoring B so B::foo() gets generated > + > +// CHECK-LABEL: define linkonce_odr i32 @_ZNK1A3fooEv(%struct.A* %this) > unnamed_addr #0 comdat align 2 > +// CHECK-LABEL: define linkonce_odr i32 @_ZNK1B3fooEv(%struct.B* %this) > unnamed_addr #5 comdat align 2 > + > +// CHECK: attributes #0 = {{{.*}}"no-builtin-memcpy"{{.*}}} > +// CHECK-NOT: attributes #0 = {{{.*}}"no-builtin-memmove"{{.*}}} > +// CHECK-NOT: attributes #0 = {{{.*}}"no-builtin-memset"{{.*}}} > +// CHECK: attributes #1 = {{{.*}}"no-builtins"{{.*}}} > +// CHECK: attributes #2 = > {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}} > +// CHECK-NOT: attributes #2 = {{{.*}}"no-builtin-memmove"{{.*}}} > +// CHECK: attributes #5 = {{{.*}}"no-builtin-memmove"{{.*}}} > +// CHECK-NOT: attributes #5 = {{{.*}}"no-builtin-memcpy"{{.*}}} > +// CHECK-NOT: attributes #5 = {{{.*}}"no-builtin-memset"{{.*}}} > +// CHECK: attributes #6 = { "no-builtin-memcpy" } > > diff --git > a/clang/test/Misc/pragma-attribute-supported-attributes-list.test > b/clang/test/Misc/pragma-attribute-supported-attributes-list.test > index 25802bd73c51..729e9b7a6f77 100644 > --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test > +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test > @@ -75,6 +75,7 @@ > // CHECK-NEXT: NSConsumed (SubjectMatchRule_variable_is_parameter) > // CHECK-NEXT: NSConsumesSelf (SubjectMatchRule_objc_method) > // CHECK-NEXT: Naked (SubjectMatchRule_function) > +// CHECK-NEXT: NoBuiltin (SubjectMatchRule_function) > // CHECK-NEXT: NoCommon (SubjectMatchRule_variable) > // CHECK-NEXT: NoDebug (SubjectMatchRule_type_alias, > SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, > SubjectMatchRule_variable_not_is_parameter) > // CHECK-NEXT: NoDestroy (SubjectMatchRule_variable) > > diff --git a/clang/test/Sema/no-builtin.cpp > b/clang/test/Sema/no-builtin.cpp > new file mode 100644 > index 000000000000..40781abd3037 > --- /dev/null > +++ b/clang/test/Sema/no-builtin.cpp > @@ -0,0 +1,51 @@ > +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify %s > + > +/// Prevent use of all builtins. > +void valid_attribute_all_1() __attribute__((no_builtin)) {} > +void valid_attribute_all_2() __attribute__((no_builtin())) {} > + > +/// Prevent use of specific builtins. > +void valid_attribute_function() __attribute__((no_builtin("memcpy"))) {} > +void valid_attribute_functions() __attribute__((no_builtin("memcpy"))) > __attribute__((no_builtin("memcmp"))) {} > + > +/// Many times the same builtin is fine. > +void many_attribute_function_1() __attribute__((no_builtin)) > __attribute__((no_builtin)) {} > +void many_attribute_function_2() __attribute__((no_builtin("memcpy"))) > __attribute__((no_builtin("memcpy"))) {} > +void many_attribute_function_3() __attribute__((no_builtin("memcpy", > "memcpy"))) {} > +void many_attribute_function_4() __attribute__((no_builtin("memcpy", > "memcpy"))) __attribute__((no_builtin("memcpy"))) {} > + > +/// Invalid builtin name. > +void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {} > +// expected-warning@-1 {{'not_a_builtin' is not a valid builtin name for > no_builtin}} > + > +/// Can't use bare no_builtin with a named one. > +void wildcard_and_functionname() __attribute__((no_builtin)) > __attribute__((no_builtin("memcpy"))) {} > +// expected-error@-1 {{empty no_builtin cannot be composed with named > ones}} > + > +/// Can't attach attribute to a variable. > +int __attribute__((no_builtin)) variable; > +// expected-warning@-1 {{'no_builtin' attribute only applies to > functions}} > + > +/// Can't attach attribute to a declaration. > +void nobuiltin_on_declaration() __attribute__((no_builtin)); > +// expected-error@-1 {{no_builtin attribute is permitted on definitions > only}} > + > +struct S { > + /// Can't attach attribute to a defaulted function, > + S() > + __attribute__((no_builtin)) = default; > + // expected-error@-1 {{no_builtin attribute has no effect on defaulted > or deleted functions}} > + > + /// Can't attach attribute to a deleted function, > + S(const S &) > + __attribute__((no_builtin)) = delete; > + // expected-error@-1 {{no_builtin attribute has no effect on defaulted > or deleted functions}} > + > + void whatever() __attribute__((no_builtin("memcpy"))); > + // expected-error@-1 {{no_builtin attribute is permitted on > definitions only}} > +}; > + > +/// Can't attach attribute to an aliased function. > +void alised_function() {} > +void aliasing_function() __attribute__((no_builtin)) > __attribute__((alias("alised_function"))); > +// expected-error@-1 {{no_builtin attribute is permitted on definitions > only}} > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
