[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

3398

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

1075–1078

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.

1080–1081

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).

1091

super set -> superset

1092

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.

1097

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

1100–1101

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

1092

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

1072

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

1078–1079

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?

1082–1083

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

1086–1089

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?

1092

Ah, I missed that -- great!

1098–1099

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

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

9

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

1072

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

1078–1079

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

1086–1089

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

1098–1099

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

1098–1099

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

1072

Through redeclarations, e.g.,

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

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

1086–1089

Walking the list and not calling llvm::copy.

1098–1099

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

1098–1099

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

1098–1099

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

3398

[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

3600 ↗

(On Diff #224286)

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

9508 ↗

(On Diff #224286)

handleNoBuiltin -> handleNoBuiltinAttr

clang/lib/Sema/SemaDeclAttr.cpp

1072

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

1103

Can you rename this to handleNoBuiltinAttr for consistency?

clang/test/CodeGen/no-builtin.c

21

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

1

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

27

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"))) {}
};

[rsmith]

clang/lib/Sema/SemaDecl.cpp

9508–9510 ↗

(On Diff #224286)

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

8

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

9508–9510 ↗

(On Diff #224286)

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

8

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

9508–9510 ↗

(On Diff #224286)

-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

9508–9510 ↗

(On Diff #224286)

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

3600 ↗

(On Diff #224286)

Makes perfect sense, thx for pointing this out.

clang/lib/Sema/SemaDecl.cpp

9508–9510 ↗

(On Diff #224286)

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

27

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

3396

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

27

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

3609 ↗

(On Diff #225593)

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

3609 ↗

(On Diff #225593)

"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/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 : InheritableAttr {
+  let Spellings = [Clang<"no_builtin">];
+  let Args = [VariadicStringArgument<"FunctionNames">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [NoBuiltinDocs];
+}

clang/include/clang/Basic/AttrDocs.td

if (true) {
       IntOwner O(7);
       P = IntPointer(O); // P "points into" O
     } // P is dangling
     return P.get(); // error: Using a dangling Pointer.
   }
 
 }];
 }
+
+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.
+
+It accepts one or more strings corresponding to the name of the builtin
+(e.g. "memcpy", "memset") or "*" which disables all builtins at once.
+
+.. code-block:: c++
+
+  // The compiler is not allowed to replace parts of foo's body with builtins.
+  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 replace parts of bar's body with the memcpy
+  // builtin.
+  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/Sema/Sema.h

VisibilityAttr *mergeVisibilityAttr(Decl *D, const AttributeCommonInfo &CI,
                                       VisibilityAttr::VisibilityType Vis);
   UuidAttr *mergeUuidAttr(Decl *D, const AttributeCommonInfo &CI,
                           StringRef Uuid);
   DLLImportAttr *mergeDLLImportAttr(Decl *D, const AttributeCommonInfo &CI);
   DLLExportAttr *mergeDLLExportAttr(Decl *D, const AttributeCommonInfo &CI);
   MSInheritanceAttr *
   mergeMSInheritanceAttr(Decl *D, const AttributeCommonInfo &CI, bool BestCase,
                          MSInheritanceAttr::Spelling SemanticSpelling);
+  NoBuiltinAttr *mergeNoBuiltinAttr(Sema &S, Decl *D,
+                                    const AttributeCommonInfo &CI,
+                                    llvm::ArrayRef<StringRef> FunctionNames);
   FormatAttr *mergeFormatAttr(Decl *D, const AttributeCommonInfo &CI,
                               IdentifierInfo *Format, int FormatIdx,
                               int FirstArg);
   SectionAttr *mergeSectionAttr(Decl *D, const AttributeCommonInfo &CI,
                                 StringRef Name);
   CodeSegAttr *mergeCodeSegAttr(Decl *D, const AttributeCommonInfo &CI,
                                 StringRef Name);
   AlwaysInlineAttr *mergeAlwaysInlineAttr(Decl *D,

clang/lib/CodeGen/CGCall.cpp

if (TargetDecl) {
     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 auto *Attr = TargetDecl->getAttr<NoBuiltinAttr>()) {
+      bool HasWildcard = llvm::is_contained(Attr->functionNames(), "*");
+      if (HasWildcard)
+        FuncAttrs.addAttribute("no-builtins");
+      else
+        for (StringRef FunctionName : Attr->functionNames()) {
+          SmallString<32> AttributeName;
+          AttributeName += "no-builtin-";
+          AttributeName += FunctionName;
+          FuncAttrs.addAttribute(AttributeName);
+        }
+    }
 
     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()))

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)));
 }
 
+NoBuiltinAttr *
+Sema::mergeNoBuiltinAttr(Sema &S, Decl *D, const AttributeCommonInfo &CI,
+                         llvm::ArrayRef<StringRef> FunctionNames) {
+  const StringRef Wildcard = "*";
+  llvm::SmallSetVector<StringRef, 8> FunctionNamesSet;
+
+  // Insert previous NoBuiltin attributes.
+  if (D->hasAttr<NoBuiltinAttr>())
+    for (StringRef FunctionName : D->getAttr<NoBuiltinAttr>()->functionNames())
+      FunctionNamesSet.insert(FunctionName);
+  // Insert new NoBuiltin attributes.
+  for (StringRef FunctionName : FunctionNames)
+    FunctionNamesSet.insert(FunctionName);
+
+  // Wildcard is a superset of all builtins, we keep only this one.
+  if (FunctionNamesSet.count(Wildcard) > 0) {
+    FunctionNamesSet.clear();
+    FunctionNamesSet.insert(Wildcard);
+  }
+
+  assert((FunctionNamesSet.count(Wildcard) == 0) ||
+         (FunctionNamesSet.size() == 1) && "Wildcard must be on its own");
+
+  llvm::SmallVector<StringRef, 8> UniqFunctionNames =
+      FunctionNamesSet.takeVector();
+  llvm::sort(UniqFunctionNames);
+
+  if (D->hasAttr<NoBuiltinAttr>())
+    D->dropAttr<NoBuiltinAttr>();
+
+  return ::new (S.Context) NoBuiltinAttr(
+      S.Context, CI, UniqFunctionNames.data(), UniqFunctionNames.size());
+}
+
+static void handleNoBuiltin(Sema &S, Decl *D, const ParsedAttr &AL) {
+  if (!checkAttributeAtLeastNumArgs(S, AL, 1))
+    return;
+
+  llvm::SmallVector<StringRef, 8> FunctionNames;
+  for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
+    StringRef FunctionName;
+    SourceLocation LiteralLoc;
+    if (!S.checkStringLiteralArgumentAttr(AL, I, FunctionName, &LiteralLoc))
+      return;
+
+    bool IsValidBuiltin = Builtin::Context::isBuiltinFunc(FunctionName.data());
+    bool IsValidFunctionName = FunctionName == "*" || IsValidBuiltin;
+    if (!IsValidFunctionName)
+      S.Diag(LiteralLoc, diag::warn_builtin_unknown) << FunctionName;
+
+    FunctionNames.push_back(FunctionName);
+  }
+
+  D->addAttr(S.mergeNoBuiltinAttr(S, D, AL, FunctionNames));
+}
+
 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:
+    handleNoBuiltin(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.c

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @foo_no_mempcy() #0
+void foo_no_mempcy() __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @foo_no_builtins() #1
+void foo_no_builtins() __attribute__((no_builtin("*"))) {}
+
+// CHECK-LABEL: define void @foo_no_mempcy_memset() #2
+void foo_no_mempcy_memset() __attribute__((no_builtin("memset", "memcpy"))) {}
+
+// CHECK-LABEL: define void @separate_attrs() #2
+void separate_attrs() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK-LABEL: define void @wildcard_wins() #1
+void wildcard_wins() __attribute__((no_builtin("memset"))) __attribute__((no_builtin("*"))) __attribute__((no_builtin("memcpy"))) {}
+
+// CHECK: attributes #0 = {{{.*}}"no-builtin-memcpy"{{.*}}}
+// CHECK: attributes #1 = {{{.*}}"no-builtins"{{.*}}}
+// CHECK: attributes #2 = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}

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.c

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - -verify %s
+
+void foo() __attribute__((no_builtin)) {} // expected-error {{'no_builtin' attribute takes at least 1 argument}}
+
+void bar() __attribute__((no_builtin())) {} // expected-error {{'no_builtin' attribute takes at least 1 argument}}
+
+void invalid_builtin() __attribute__((no_builtin("not_a_builtin"))) {} // expected-error {{use of unknown builtin not_a_builtin}}
+
+int __attribute__((no_builtin("*"))) variable; // expected-warning {{'no_builtin' attribute only applies to functions}}