[clang] ad531ff - Revert "[clang] Add no_builtin attribute"

Vlad Tsyrklevich via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 28 15:22:08 PDT 2019


Author: Vlad Tsyrklevich
Date: 2019-10-28T15:21:59-07:00
New Revision: ad531fff81a2a266ffed1d7da3333778cb59c983

URL: https://github.com/llvm/llvm-project/commit/ad531fff81a2a266ffed1d7da3333778cb59c983
DIFF: https://github.com/llvm/llvm-project/commit/ad531fff81a2a266ffed1d7da3333778cb59c983.diff

LOG: Revert "[clang] Add no_builtin attribute"

This reverts commit bd87916109483d33455cbf20da2309197b983cdd. It was
causing ASan/MSan failures on the sanitizer buildbots.

Added: 
    

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: 
    clang/test/CodeGen/no-builtin.cpp
    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 @@ 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 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<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 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<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);
-      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);
-            }
-        }
-      }
+      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<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 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<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;
@@ -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 at -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 at -1 {{empty no_builtin cannot be composed with named ones}}
-
-/// Can't attach attribute to a variable.
-int __attribute__((no_builtin)) variable;
-// expected-warning at -1 {{'no_builtin' attribute only applies to functions}}
-
-/// Can't attach attribute to a declaration.
-void nobuiltin_on_declaration() __attribute__((no_builtin));
-// expected-error at -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 at -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 at -1 {{no_builtin attribute has no effect on defaulted or deleted functions}}
-
-  void whatever() __attribute__((no_builtin("memcpy")));
-  // expected-error at -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 at -1 {{no_builtin attribute is permitted on definitions only}}


        


More information about the cfe-commits mailing list