From 98f3151a7dded8838fafcb5f46e6c8358def96b8 Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet Date: Wed, 25 Sep 2019 11:31:28 +0200 Subject: [PATCH] [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 This is a re-submit after it got reverted in https://reviews.llvm.org/rGbd8791610948 since the breakage doesn't seem to come from this patch. --- clang/include/clang/AST/Decl.h | 4 ++ clang/include/clang/Basic/Attr.td | 7 +++ clang/include/clang/Basic/AttrDocs.td | 37 +++++++++++++ clang/include/clang/Basic/DiagnosticSemaKinds.td | 9 +++ clang/lib/CodeGen/CGCall.cpp | 24 ++++++-- clang/lib/Sema/SemaDecl.cpp | 23 ++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 53 ++++++++++++++++++ clang/test/CodeGen/no-builtin.cpp | 64 ++++++++++++++++++++++ ...pragma-attribute-supported-attributes-list.test | 1 + clang/test/Sema/no-builtin.cpp | 51 +++++++++++++++++ 10 files changed, 269 insertions(+), 4 deletions(-) create mode 100644 clang/test/CodeGen/no-builtin.cpp create mode 100644 clang/test/Sema/no-builtin.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index b3e7a57..16094c0 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2031,6 +2031,10 @@ public: /// /// 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 4557a61..d5018f4 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 6e79d0b..9d0d274 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 c93edb2..2313c60 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3604,6 +3604,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>; +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 b74f6f9..62e8fa0 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(TargetDecl)) { AddAttributesFromFunctionProtoType( getContext(), FuncAttrs, Fn->getType()->getAs()); - // 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(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()) { + 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 6202391..aba7049 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()) + 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 abbd597..99eb23c 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(D))); } +static void handleNoBuiltinAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + static constexpr const StringRef kWildcard = "*"; + + llvm::SmallVector 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()) + 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()) + D->dropAttr(); + 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()) { 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 0000000..3c5d681 --- /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 25802bd..729e9b7 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 0000000..40781ab --- /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}} -- 2.7.4