[clang] Add no_builtin attribute

[gchatelet]

Summary

This is a follow up on https://reviews.llvm.org/D61634
This patch is simpler and only adds the no_builtin attribute.

[aaron.ballman]

Thank you for working on this!

It looks like you're missing all of the sema tests that check that the attribute only appertains to functions, accepts the proper kind of arguments, etc.

clang/include/clang/Basic/Attr.td

3433

The documentation should explain what this is.

clang/lib/CodeGen/CGCall.cpp

1853

const auto -> bool (and drop the top-level const).

1859

const auto & -> StringRef?

1861–1862

I think this checking should happen in Sema rather than CodeGen.

clang/lib/Sema/SemaDeclAttr.cpp

1076–1079

I think that this should be done in a merge function; there are plenty of examples of how this is typically done elsewhere in the file.

1081–1082

Given that the user has to provide the parens for the attribute anyway, I think this situation should be diagnosed rather than defaulting to the wildcard. It helps catch think-os where the user put in parens and forgot to add the parameter in the first place (the wildcard is not onerous to spell out).

1092

super set -> superset

1093

This silently changes the behavior from what the user might expect, which seems bad. For instance, it would be very reasonable for a user to write [[clang::no_builtin("__builtin_mem*")]] expecting that to behave as a regex pattern, but instead it silently turns into a "disable all builtins".

I think it makes sense to diagnose unexpected input like that rather than silently accept it under perhaps unexpected semantics. It may also make sense to support regex patterns in the strings or at least keep the design such that we can add that functionality later without breaking users.

1098

Please don't use auto here as the type is not spelled out in the initialization.

1101–1102

I think this needs to happen in a merge function.

[gchatelet]

• Address aaron ballman comments

[gchatelet]

@aaron.ballman thx a lot for the review. I really appreciate it, especially because I'm not too familiar with this part of the codebase.

clang/lib/Sema/SemaDeclAttr.cpp

1093

The code looks for a function name that is exactly * and not "a function name that contains *", it would not turn [[clang::no_builtin("__builtin_mem*")]] into [[clang::no_builtin("*")]].
That said, I fully agree that an error should be returned if the function name is not valid.
I'll work on this.

[gchatelet]

• Checks function name validity and errors when passed 0 argument.

[gchatelet]

@tejohnson I believe this is the missing part for D67923.

I'm unsure if we still need the BitVector at all in the TLI since it could be a simple attribute lookup on the function. Do you see any problematic interactions with the inlining phase?

[gchatelet]

• Update documentation and rebase

[tejohnson]

In D68028#1685912, @gchatelet wrote:

@tejohnson I believe this is the missing part for D67923.

Thanks, yep I will take a closer look at the patch today.

I'm unsure if we still need the BitVector at all in the TLI since it could be a simple attribute lookup on the function.

If we didn't save the info on the TLI we would instead need to save the Function object in the TLI and query the attribute info off the Function on every lookup, which seems heavier weight. I think caching the info in that object for fast lookup is going to be better. And as noted in the comments there, we can replace it with the existing AvailableArray moved from the base Impl object into the TLI and remove the override bitvector once this goes in, we use these attributes to set the TLI info on construction, and we remove the clang code that sets the unavailability from the CodeGenOpts which will no longer be needed. If this patch goes in first I can just modify my TLI patch to do that all in one go. Maybe that is best...

Do you see any problematic interactions with the inlining phase?

The inliner will need to be modified to respect the function attributes.

[aaron.ballman]

clang/lib/Sema/SemaDeclAttr.cpp

1073

You're missing a call to this function within mergeDeclAttribute() in SemaDecl.cpp.

1079–1080

Instead of doing hasAttr<> followed by getAttr<>, this should be:

if (const auto *NBA = D->getAttr<NoBuiltinAttr>()) {
  for (StringRef FunctionName : NBA->functionNames())
    ...
}

But are you able to use llvm::copy() instead of a manual loop?

1083–1084

Same question here about using llvm::copy().

1087–1090

Rather than walking the entire set like this, would it make more sense to look for the wildcard in the above loop before inserting the name, and set a local variable if found, so that you can do the clear without having to rescan the entire list?

1093

Ah, I missed that -- great!

1099–1100

Just making sure I understand the semantics you want: redeclarations do not have to have matching lists of builtins, but instead the definition will use a merged list? e.g.,

[[clang::no_builtin("memset")]] void whatever();
[[clang::no_builtin("memcpy")]] void whatever();

[[clang::no_builtin("memmove")]] void whatever() {
 // Will not use memset, memcpy, or memmove builtins.
}

That seems a bit strange, to me. In fact, being able to apply this attribute to a declaration seems a bit like a mistake -- this only impacts the definition of the function, and I can't imagine good things coming from hypothetical code like:

[[clang::no_builtin("memset")]] void whatever();
#include "whatever.h" // Provides a library declaration of whatever() with no restrictions on it

WDYT about restricting this attribute to only appear on definitions?

clang/test/Sema/no-builtin.c

1 ↗

(On Diff #222163)

This should not be emitting LLVM -- it should be -fsyntax-only, however.

9 ↗

(On Diff #222163)

We'll need more tests for redeclarations or declaration vs definition once we decide how we want to handle that.

You should also add some positive tests that show the attribute applying as expected.

[gchatelet]

thx @aaron.ballman , I'm waiting for your reply before uploading the new patch (some of my comments won't have the accompanying code update sorry)

clang/lib/Sema/SemaDeclAttr.cpp

1073

Thx, I rearranged the signature a bit, do you happen to know how mergeDeclAttribute is tested?

1079–1080

I had to use a vector instead of a set and uniquify by hand.

1087–1090

This is is conflict with the llvm::copy suggestion above. Which one do you prefer?

1099–1100

That's a very good point. Thx for noticing.
Indeed I think it only makes sense to have the attribute on the function definition.

I've tried to to use FunctionDecl->hasBody() during attribute handling in the Sema phase but it seems like the FunctionDecl is not complete at this point.
All calls to hasBody() return false, if I repeat the operation in CGCall then hasBody returns true and I can see the CompoundStatement.

Do you have any recommendations on where to perform the check?

[gchatelet]

clang/lib/Sema/SemaDeclAttr.cpp

1099–1100

So after some investigations it turns out that FunctionDecl::isThisDeclarationADefinition is buggy (returns always false) when called from ProcessDeclAttribute.
I believe it's because the CompoundStatement is not parsed at this point.

As a matter of fact all code using this function in ProcessDeclAttribute is dead code (see D68379 which disables the dead code, tests are still passing)

I'm unsure of how to fix this, it may be possible of using FunctionDecl::setWillHaveBodyin this switch.

[aaron.ballman]

clang/lib/Sema/SemaDeclAttr.cpp

1073

Through redeclarations, e.g.,

[[some_attr]] void func();
[[some_other_attr]] void func();

[[a_third_attr, some_attr]] void func() {
}

1087–1090

Walking the list and not calling llvm::copy.

1099–1100

So after some investigations it turns out that FunctionDecl::isThisDeclarationADefinition is buggy (returns always false) when called from ProcessDeclAttribute.

That is a bit odd to me; we call it in a half dozen places in SemaDeclAttr.cpp, all of which get called from ProcessDeclAttribute. Are ALL of those uses broken currently?!

As a matter of fact all code using this function in ProcessDeclAttribute is dead code (see D68379 which disables the dead code, tests are still passing)

You got four of the six. What about the use in handleObjCSuppresProtocolAttr() and the second use in handleAliasAttr()?

I'm unsure of how to fix this, it may be possible of using FunctionDecl::setWillHaveBodyin this switch.

I'm also unsure of a good way forward. @rsmith may have ideas too.

[gchatelet]

clang/lib/Sema/SemaDeclAttr.cpp

1099–1100

That is a bit odd to me; we call it in a half dozen places in SemaDeclAttr.cpp, all of which get called from ProcessDeclAttribute. Are ALL of those uses broken currently?!
You got four of the six. What about the use in handleObjCSuppresProtocolAttr() and the second use in handleAliasAttr()?

It really is FunctionDecl::isThisDeclarationADefinition that is buggy, the other places where we call isThisDeclarationADefinition in SemaDeclAttr.cpp are ok, i.e. VarDecl::isThisDeclarationADefinition and ObjCProtocolDecl::isThisDeclarationADefinition

I tried to use FunctionDecl::setWillHaveBodybut it broke many tests so it seems inappropriate.

[gchatelet]

clang/lib/Sema/SemaDeclAttr.cpp

1099–1100

I've tried to fix it in D68701. I'm pretty sure this is not the right way to do it though,
@rsmith any idea on how to proceed?

In the meantime I'll add a FIXME and move on with this patch if you don't mind.

[gchatelet]

• Address aaron ballman comments
• Checks function name validity and errors when passed 0 argument.
• Update documentation and rebase
• Rewrote mergeNoBuiltinAttr
• Address comments

[gchatelet]

• Remove leftovers

[gchatelet]

• rebase

[gchatelet]

It should be ready to submit, @aaron.ballman @courbet can you take a final look?

[courbet]

clang/include/clang/Basic/Attr.td

3433

[nit] Should this be BuiltinNames ? builtins are not necessarily functions.

clang/include/clang/Basic/AttrDocs.td

4409

I would phrase it as The compiler is not allowed to add any builtin to foo's body. This covers inserting as well as replacing code with builtins.

[gchatelet]

• Address comments

[gchatelet]

• Fix missing rename

[gchatelet]

A gentle ping @aaron.ballman

[aaron.ballman]

In D68028#1707931, @gchatelet wrote:

A gentle ping @aaron.ballman

Sorry for the delay, I was traveling for last week.

clang/include/clang/Basic/AttrDocs.td

4402

I'd appreciate a blurb in here to the effect: The attribute may also be applied to a virtual function but has no effect on the behavior of overriding functions in a derived class.

clang/include/clang/Basic/DiagnosticSemaKinds.td

3627

I think this should be a warning rather than an error.

Imagine the situation where the user uses Clang 11 to write their code and they disable a Clang 11-specific builtin from being used, but then they try to compile the code in Clang 10 which doesn't know about the builtin.

Rather than err, it seems reasonable to warn about the unknown builtin name (under a new warning flag group under -Wattributes) and then just ignore the attribute. Because the builtin is unknown anyway, we won't transform any constructs to use it.

clang/lib/Sema/SemaDecl.cpp

9534

handleNoBuiltin -> handleNoBuiltinAttr

clang/lib/Sema/SemaDeclAttr.cpp

1073

Sorry for the code churn, but we realized we wanted this only on definitions after we started down the declaration merging path. I think we do not need mergeNoBuiltinAttr() any longer and can remove most of this logic entirely -- you cannot re-define functions, so we don't have any attributes to merge together. e.g., we no longer have this situation:

void whatever() __attribute__((no_builtin("memcpy")));
void whatever() __attribute__((no_builtin("memmove"))) {} // Should ignore both memcpy and memmove

I think the only part that needs to move over to handleNoBuiltinAttr() is the part diagnosing a combination of wildcard and non-wildcard arguments.

As a thought for a future patch (not requesting this be done now, unless you want to), should we fix-it (or possibly support) this situation?

void whatever() __attribute__((no_builtin("memcpy, memmove"))) {} // Note that it's only one string literal

1104

Can you rename this to handleNoBuiltinAttr for consistency?

clang/test/CodeGen/no-builtin.c

20 ↗

(On Diff #224286)

I'd like to see a codegen case that ensures we don't do bad things with virtual function overrides:

struct S {
  virtual void foo() __attribute__((no_builtin("memcpy"))) {}
};

struct D : S {
  void foo() __attribute__((no_builtin("memmove"))) {}
};

We should ensure that S::foo() only prohibits memcpy and D::foo() only prohibits memmove.

clang/test/Sema/no-builtin.c

26 ↗

(On Diff #224286)

You should probably add another test case for this situation:

void whatever() __attribute__((no_builtin("*", "*"))) {}

and for member functions in C++ as well:

struct S {
  void whatever() __attribute__((no_builtin("memcpy"))) {}
  virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
};

1 ↗

(On Diff #222163)

This still has -S -emit-llvm -o -; that should be removed, no?

[rsmith]

clang/lib/Sema/SemaDecl.cpp

9534–9536

I am not convinced that this is a bug -- the function declaration does not become a definition until the parser reaches the definition.

In any case, I don't think the predicate you want is "is this declaration a definition?". Rather, I think what you want is, "does this declaration introduce an explicit function body?". In particular, we should not permit uses of this attribute on defaulted or deleted functions, nor on functions that have a definition by virtue of using __attribute__((alias)). So it probably should be a syntactic check on the form of the declarator (that is, the check you're perrforming here), and the check should probably be D.getFunctionDefinitionKind() == FDK_Definition. (A custom diagnostic for using the attribute on a defaulted or deleted function would be nice too, since the existing diagnostic text isn't really accurate in those cases.)

clang/test/Sema/no-builtin.c

7 ↗

(On Diff #224286)

I find this surprising. I would expect this to work, and to mean the same as -fno-builtin for the function (that is, same as the * form).

I think might be a better design to remove the magical "*" form entirely, and use an empty argument list to mean "all builtins". Have you considered that? (That would better mirror how -fno-builtin vs -fno-builtin-foo works.)

[aaron.ballman]

clang/lib/Sema/SemaDecl.cpp

9534–9536

In particular, we should not permit uses of this attribute on defaulted or deleted functions

Deleted functions, sure. Defaulted functions... not so sure. I could sort of imagine wanting to instruct a defaulted assignment operator that does memberwise copy that it's not allowed to use a builtin memcpy, for instance. Or is this a bad idea for some reason I'm not thinking of?

clang/test/Sema/no-builtin.c

7 ↗

(On Diff #224286)

I think might be a better design to remove the magical "*" form entirely, and use an empty argument list to mean "all builtins".

I like this approach.

[rsmith]

clang/lib/Sema/SemaDecl.cpp

9534–9536

-fno-builtin does not turn off using llvm.memcpy for copying memory, and it doesn't turn off llvm.memcpy being lowered to a call to memcpy. Allowing this for defaulted functions would only give a false sense of security, at least for now (though I could imagine we might find some way to change that in the future).

Also, trivial defaulted functions (where we're most likely to end up with memcpy calls) are often not emitted at all, instead being directly inlined by the frontend, so there's nowhere to attach a no-builtin-memcpy LLVM attribute (we'd need to put the attribute on all callers of those functions) even if LLVM learned to not emit calls to memcpy to implement llvm.memcpy when operating under a no-builtin-memcpy constraint.

[aaron.ballman]

clang/lib/Sema/SemaDecl.cpp

9534–9536

Ah, good to know! We're in agreement on how this should proceed, thank you for the insights.

[gchatelet]

• Address comments
• Fix missing rename
• Address comments
• Add warning category

[gchatelet]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3627

Makes perfect sense, thx for pointing this out.

clang/lib/Sema/SemaDecl.cpp

9534–9536

I am not convinced that this is a bug -- the function declaration does not become a definition until the parser reaches the definition.

@rsmith It may not be a bug but it is currently used in a buggy manner (See D68379).
An assert to prevent misuse would be welcome IMHO, I've added a note on the function meanwhile.

clang/test/Sema/no-builtin.c

26 ↗

(On Diff #224286)

void whatever() __attribute__((no_builtin("*", "*"))) {}

I've added a few ones.

struct S {
  void whatever() __attribute__((no_builtin("memcpy"))) {}
  virtual void intentional() __attribute__((no_builtin("memcpy"))) {}
};

What do you want me to test for this one?

[aaron.ballman]

clang/include/clang/Basic/Attr.td

3431

I think we do not want this to be an inheritable attribute, but just Attr instead, because this can only be written on definitions (so there's no way to inherit the attribute from previous declarations).

clang/test/Sema/no-builtin.c

26 ↗

(On Diff #224286)

What do you want me to test for this one?

Ensuring that applying the attribute to member function definitions, including virtual ones, is supported and doesn't cause diagnostics. The codegen side of things will test that overridden virtual methods do not inherit the attribute.

Actually, there's another interesting test hiding in there for when we do want a diagnostic (I think):

struct S {
  void whatever() __attribute__((no_builtin("memcpy"))); // Should diagnose as not a definition
};

[gchatelet]

• Change NoBuiltin from InheritableAttr to Attr
• Improve tests

[gchatelet]

• Improve tests

[gchatelet]

• Reverting whole file formatting

[gchatelet]

• Call sites to virtual functions are not annotated

[aaron.ballman]

clang/include/clang/Basic/AttrDocs.td

4407

This mention of * is now out of date.

4412

This one too.

clang/include/clang/Basic/DiagnosticSemaKinds.td

3629

Hmm, I thought there was a way you could do:

InGroup<DiagGroup<"invalid-no-builtin-names", IgnoredAttributes>>;

but it seems I've got the order reversed and you'd have to add a new diagnostic group that IgnoredAttributes could subsume. We don't do that for other attributes, so I think you should just do:

InGroup<DiagGroup<"invalid-no-builtin-names">>;

(Feel free to pick a better diagnostic group name if you have a better idea, I'm not tied to my suggestion.)

[gchatelet]

• Fix documentation and Warning category

[gchatelet]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3629

"invalid-no-builtin-names" LGTM

[gchatelet]

LGTY @aaron.ballman ?

[aaron.ballman]

LGTM!

[gchatelet]

In D68028#1723568, @aaron.ballman wrote:

LGTM!

Thx a lot for bearing with me and for your valuable comments !
Really appreciate it!

clang/include/clang/AST/Decl.h

Stmt *getBody() const override {
     return getBody(Definition);
   }
 
   /// Returns whether this specific declaration of the function is also a
   /// definition that does not contain uninstantiated body.
   ///
   /// 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();
   }
 
   /// Returns whether this specific declaration of the function has a body.
   bool doesThisDeclarationHaveABody() const {
     return Body || isLateTemplateParsed();

clang/include/clang/Basic/Attr.td

 }
 
 def ObjCExternallyRetained : InheritableAttr {
   let LangOpts = [ObjCAutoRefCount];
   let Spellings = [Clang<"objc_externally_retained">];
   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];
+}

clang/include/clang/Basic/AttrDocs.td

 
 def ArmMveAliasDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
 This attribute is used in the implementation of the ACLE intrinsics
 for the Arm MVE instruction set. It allows the intrinsic functions to
 be declared using the names defined in ACLE, and still be recognized
 as clang builtins equivalent to the underlying name. For example,
 ``arm_mve.h`` declares the function ``vaddq_u32`` with
 ``__attribute__((__clang_arm_mve_alias(__builtin_arm_mve_vaddq_u32)))``,
 and similarly, one of the type-overloaded declarations of ``vaddq``
 will have the same attribute. This ensures that both functions are
 recognized as that clang builtin, and in the latter case, the choice
 of which builtin to identify the function as can be deferred until
 after overload resolution.
 
 This attribute can only be used to set up the aliases for the MVE
 intrinsic functions; it is intended for use only inside ``arm_mve.h``,
 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;
+  }
+  }];
+}

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_attribute_overloadable_mismatch : Error<
   "redeclaration of %0 must %select{not |}1have the 'overloadable' attribute">;
 def note_attribute_overloadable_prev_overload : Note<
   "previous %select{unmarked |}0overload of function is here">;
 def err_attribute_overloadable_no_prototype : Error<
   "'overloadable' function %0 must have a prototype">;
 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">,
   InGroup<IgnoredAttributes>;
 def err_ns_attribute_wrong_parameter_type : Error<
   "%0 attribute only applies to "
   "%select{Objective-C object|pointer|pointer-to-CF-pointer}1 parameters">;
 def warn_ns_attribute_wrong_parameter_type : Warning<

clang/lib/CodeGen/CGCall.cpp

if (TargetDecl->hasAttr<NoReturnAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
     if (TargetDecl->hasAttr<ColdAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::Cold);
     if (TargetDecl->hasAttr<NoDuplicateAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::NoDuplicate);
     if (TargetDecl->hasAttr<ConvergentAttr>())
       FuncAttrs.addAttribute(llvm::Attribute::Convergent);
 
     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()))
+      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.
     if (TargetDecl->hasAttr<ConstAttr>()) {
       FuncAttrs.addAttribute(llvm::Attribute::ReadNone);
       FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
     } else if (TargetDecl->hasAttr<PureAttr>()) {
       FuncAttrs.addAttribute(llvm::Attribute::ReadOnly);

clang/lib/Sema/SemaDecl.cpp

if (const auto *attr = NewFD->getAttr<AvailabilityAttr>()) {
     }
     if (NewFD->hasAttr<DestructorAttr>()) {
       Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
           << 2;
       NewFD->dropAttr<AvailabilityAttr>();
     }
   }
 
+  // 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;
 }
 
 /// Return a CodeSegAttr from a containing class.  The Microsoft docs say
 /// when __declspec(code_seg) "is applied to a class, all member functions of
 /// the class and nested classes -- this includes compiler-generated special
 /// member functions -- are put in the specified segment."
 /// The actual behavior is a little more complicated. The Microsoft compiler

clang/lib/Sema/SemaDeclAttr.cpp

static void handleDiagnoseIfAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   bool ArgDependent = false;
   if (const auto *FD = dyn_cast<FunctionDecl>(D))
     ArgDependent = ArgumentDependenceChecker(FD).referencesArgs(Cond);
   D->addAttr(::new (S.Context) DiagnoseIfAttr(
       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;
     return;
   }
 
   Expr *E = AL.getArgAsExpr(0);
   uint32_t Type;
case ParsedAttr::AT_Destructor:
     handleDestructorAttr(S, D, AL);
     break;
   case ParsedAttr::AT_EnableIf:
     handleEnableIfAttr(S, D, AL);
     break;
   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;
   case ParsedAttr::AT_ExternalSourceSymbol:
     handleExternalSourceSymbolAttr(S, D, AL);
     break;
   case ParsedAttr::AT_MinSize:
     handleMinSizeAttr(S, D, AL);

clang/test/CodeGen/no-builtin.cpp

+// 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" }

clang/test/Misc/pragma-attribute-supported-attributes-list.test

 // CHECK-NEXT: MinSize (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: MinVectorWidth (SubjectMatchRule_function)
 // CHECK-NEXT: Mips16 (SubjectMatchRule_function)
 // CHECK-NEXT: MipsLongCall (SubjectMatchRule_function)
 // CHECK-NEXT: MipsShortCall (SubjectMatchRule_function)
 // 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)
 // CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
 // CHECK-NEXT: NoEscape (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: NoInline (SubjectMatchRule_function)
 // CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)

clang/test/Sema/no-builtin.cpp

+// 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}}