[clang] bd87916 - [clang] Add no_builtin attribute

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


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 at 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 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}}
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191028/8a4d462c/attachment-0001.html>


More information about the cfe-commits mailing list