From ad531fff81a2a266ffed1d7da3333778cb59c983 Mon Sep 17 00:00:00 2001 From: Vlad Tsyrklevich Date: Mon, 28 Oct 2019 15:21:38 -0700 Subject: [PATCH] Revert "[clang] Add no_builtin attribute" This reverts commit bd87916109483d33455cbf20da2309197b983cdd. It was causing ASan/MSan failures on the sanitizer buildbots. --- clang/include/clang/AST/Decl.h | 4 -- clang/include/clang/Basic/Attr.td | 7 -- clang/include/clang/Basic/AttrDocs.td | 37 ----------- .../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 ------------------- ...a-attribute-supported-attributes-list.test | 1 - clang/test/Sema/no-builtin.cpp | 51 --------------- 10 files changed, 4 insertions(+), 269 deletions(-) delete mode 100644 clang/test/CodeGen/no-builtin.cpp delete 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 16094c0988fa..b3e7a570fd6d 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2031,10 +2031,6 @@ 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 d5018f444e1c..4557a614d361 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -3427,10 +3427,3 @@ 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 9d0d27407573..6e79d0bb3631 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -4413,40 +4413,3 @@ 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 2313c60f006f..c93edb2f91c2 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3604,15 +3604,6 @@ 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 62e8fa037013..b74f6f942426 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -1853,27 +1853,11 @@ 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); - 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); - } - } - } + if (Fn->isNoReturn() && !(AttrOnCallSite && MD && MD->isVirtual())) + FuncAttrs.addAttribute(llvm::Attribute::NoReturn); } // 'const', 'pure' and 'noalias' attributed functions are also nounwind. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index aba7049b0a51..6202391ee0b8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -9529,29 +9529,6 @@ 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 99eb23c3fe61..abbd597a26d0 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -1069,56 +1069,6 @@ 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; @@ -6658,9 +6608,6 @@ 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 deleted file mode 100644 index 3c5d681282da..000000000000 --- a/clang/test/CodeGen/no-builtin.cpp +++ /dev/null @@ -1,64 +0,0 @@ -// 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 729e9b7a6f77..25802bd73c51 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/test/Misc/pragma-attribute-supported-attributes-list.test @@ -75,7 +75,6 @@ // 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 deleted file mode 100644 index 40781abd3037..000000000000 --- a/clang/test/Sema/no-builtin.cpp +++ /dev/null @@ -1,51 +0,0 @@ -// 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}}