Fix interaction between clang and some inline builtins from glibc under _FORTIFY_SOURCE

[serge-sans-paille]

Summary

Clang considers trivially recursive a function that may call itself, not a function that always call itself.
This leads to some inline definition of fortified libc builtins no being emitted, and thus ignored.

Work around the situation by detecting the pattern and generate a combination of builtin / nobuiltin attributes to have it work as expected.

The basic problem to solve is the compilation of the following code, which exhibits the behavior used in the glibc. Note that currently clang generates a call to wmemcpy ad not __wmemcpy_chk as it should, leading to a less secure code.

typedef long unsigned int size_t;
typedef int wchar_t;

extern wchar_t *__wmemcpy_chk (wchar_t *__restrict __s1, const wchar_t
                               *__restrict __s2, size_t __n, size_t __ns1)
__attribute__ ((__nothrow__ ));

extern wchar_t *__wmemcpy_alias (wchar_t *__restrict __s1,
                                 const wchar_t *__restrict __s2, size_t __n)
__asm__("wmemcpy") __attribute__ ((__nothrow__ ));

extern __inline __attribute__ ((__always_inline__))
__attribute__ ((__gnu_inline__))
__attribute__ ((__nothrow__ ))
wchar_t *
wmemcpy (wchar_t *__restrict __s1, const wchar_t *__restrict __s2, size_t __n)

{
  if (__builtin_object_size (__s1, 0) != (size_t) -1
      && (!__builtin_constant_p (__n)
          || __n > __builtin_object_size (__s1, 0) / sizeof (wchar_t)))
    {
        return __wmemcpy_chk (__s1, __s2, __n,
                              (__builtin_object_size (__s1, 0)
                               / sizeof (wchar_t)));
    }
  return __wmemcpy_alias (__s1, __s2, __n);
}

wchar_t wbuf[10];

int
main (int argc, char **argv)
{
  wmemcpy (wbuf + 1, L"abcdefghij", 10);
  return 0;
}

[serge-sans-paille]

Update documentation and tests

[serge-sans-paille]

@nickdesaulniers I added you to the review because you bumped into the non-inling issue at the kernel level at some point.

[nickdesaulniers]

llvm/lib/Analysis/InlineCost.cpp

2638–2640

shouldn't we do something here with RecursiveIsViable?

2679

should the second parameter here be true? If so, implies missing test coverage.

2680

rather than splitting this up and adding parameters, I think we can just check for the alwaysinline fn attr in llvm::isInlineViable.

[serge-sans-paille]

Improve trivially recursive function detection

No modification on LLVM side, only at clang level

[serge-sans-paille]

llvm/lib/Analysis/InlineCost.cpp

2638–2640

Updated to match the change at clang level.

[rnk]

clang/lib/CodeGen/CodeGenModule.cpp

38

I see a dep from clangCodeGen to clangAnalysis in the CMakeLists, but this is the first include of clang/Analysis/* from CodeGen. I think we should consider this new dependency more carefully.

@rjmccall, can you please advise about the new dependency?

2938–2940

These are both really expensive. Is this really necessary?

clang/test/CodeGen/memmove-always-inline-definition-used.c

2

I'd like to see the generated IR before LLVM optimization passes run. Please add a RUN line with -disable-llvm-passes and add checks that show what attributes get applied where. From the code, I think noinline goes on the call site, but I wanted to check my understanding in the test.

[serge-sans-paille]

Update test case

[serge-sans-paille]

clang/lib/CodeGen/CodeGenModule.cpp

2938–2940

I'd be very happy to remove them. I could remove these use by designing an ad-hoc analysis to detect basic always recursive functions patterns (basically some kind of CFG-like analysis that would build the CFG on the fly with early branch exit when it finds a call to self)

[serge-sans-paille]

Updated approach, much less costly: match the pattern of functions forwarding to self, as detecting if they're recursive doesn't match the reality of inline builtins.
The rational could be that if it's doing anything other than forwarding to self, then it's there on purpose and we shouldn't skip it.

[serge-sans-paille]

@rnk the new approach doesn't have the compilation time impact the previous had, still passes validation and a new test case has been added.

[rnk]

This code was originally added to solve https://llvm.org/pr9614, which has a fair amount of context for how we got here today.

clang/test/CodeGen/memmove-always-inline-definition-used.c

18–20

Even if this happens to work in practice, memmove is still mutually recursive, and some other IPO transform could break this. The code pattern seems fragile.

If we are serious about implementing glibc fortify, which, to be clear, is a new direction for us, we should think about what IR pattern we really want to see out of clang.

I think this IR better represents the intention of the original program, and is better for optimizations and other IR analysis consumers:

define void @usercode() {
...
  call void @__fortify_memmove(...)
...
}
define i8* @__fortify_memmove(...) {
...
  call i8* @llvm.memcpy(...)
...
  call i8* @memmove(...)
...
}
declare i8* @memmove(...)

If we're going to have to live with this glibc fortify code pattern, maybe clang should try to spot it and rewrite it into more helpful and analyzable LLVM IR.

---

If people agree that this is a reasonable direction, let's think about how to implement it.

One way to get there would be to implement asm labels by waiting until after IRGen to do some value replacement. So, clang would emit code for the inline definition of @memmove, and then later, it would rename @memmove_alias to @memmove. At that point, it would invent a new name for the original @memmove. The least effort thing to do would be to rename it to @memmove.1 using the normal value renaming. We would never see this symbol in the ELF symbol table, because these are usually marked always inline.

[serge-sans-paille]

As suggested by @rnk, spot the pattern more accurately at clang level, and use a combination of nobuiltin / builtin attributes to flag it at LLVM IR level.
What I like with that approach is that we're not preventing the emission a function Body, but just handling it in a decent way.

That's still only a possible approach, I'm of course open to other approaches!

[serge-sans-paille]

@rnk what do you find of this approach?

[serge-sans-paille]

Rebased on main.

[serge-sans-paille]

Up ?

[serge-sans-paille]

@rnk : are you happy with how the patch looks now?

[serge-sans-paille]

@rnk I don't want that patch to bitrot , as it addresses a defect in _FORTIFY_SOURCE, i.e. it fills an (arguably small) security defect. Can you please have a look?

[rnk]

I apologize for taking an unreasonably long time to respond, this feature doesn't fill me with a sense of joy and fulfillment, but I do want to help get things unblocked.

Let me try getting more input and visibility from some folks: @bkramer @efriedma @mcgrathr

My basic input is that the frontend should try to untangle the complexity of the glibc fortify implementation so that the middle-end can remain blissfully unaware of it.

clang/test/CodeGen/memmove-always-inline-definition-used.c

18–20

I think my concern about having an infinitely recursive function in the IR is still here. I still think the right way to handle this is to rename the fortify wrappers to something else, memmove.1 or something. You'd also have to change the linkage to internal from available_externally.

I think the main case where that doesn't work is when you take the address of a fortify wrapper. In this case, you will end up with the address of the wrapper, and the compiler is forced to emit a standalone copy of the implementation.

llvm/lib/Transforms/IPO/Inliner.cpp

624

Changing the inliner doesn't seem like the right way to address this.

[efriedma]

I'm not really happy with this approach. I'm concerned at the IR level about functions that appear to be recursive, but aren't really supposed to recurse.

I'd recommend teaching clang to rename the inline function. So in the LLVM IR, you have the definition of the inline (with internal linkage), and the declaration of the external function. Then the users refer to the actual function they want (either the inline, or the original), depending on the context. If it's a recursive call, or not a call, refer to the external declaration. If it's a call, it refers to the inline function. That makes the semantics of the IR obvious, and preserves the intended functionality.

(gcc can make the choice to inline after certain optimizations run, so it can optimize certain edge cases involving function pointers to library functions. But that's unlikely to matter for normal usage.)

[serge-sans-paille]

Closing the patch, it is obsoleted by https://reviews.llvm.org/D109967

clang/lib/CodeGen/CGCall.cpp

#endif
 
   // Compute the calling convention and attributes.
   unsigned CallingConv;
   llvm::AttributeList Attrs;
   CGM.ConstructAttributeList(CalleePtr->getName(), CallInfo,
                              Callee.getAbstractInfo(), Attrs, CallingConv,
                              /*AttrOnCallSite=*/true);
 
+  // Calling a function that aliases to self through an AsmLabel is a pattern
+  // used by glibc for fortified functions. Mark the call as Builtin and the
+  // called function as NoBuiltin.
+  if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(TargetDecl)) {
+    bool MarkBuiltin = false;
+    if (AsmLabelAttr *Attr = FD->getAttr<AsmLabelAttr>()) {
+      MarkBuiltin |= CurFn->getName() == Attr->getLabel() &&
+                     Builtin::Context::isBuiltinFunc(Attr->getLabel());
+    }
+    unsigned BuiltinID = FD->getBuiltinID();
+    Builtin::Context &BI = getContext().BuiltinInfo;
+    if (BuiltinID && BI.isLibFunction(BuiltinID)) {
+      StringRef BuiltinName = BI.getName(BuiltinID);
+      MarkBuiltin |= BuiltinName.startswith("__builtin_") &&
+                     CurFn->getName() == BuiltinName.slice(strlen("__builtin_"),
+                                                           StringRef::npos);
+    }
+    if (MarkBuiltin) {
+      Attrs = Attrs.addAttribute(getLLVMContext(),
+                                 llvm::AttributeList::FunctionIndex,
+                                 llvm::Attribute::Builtin);
+      CurFn->addAttribute(llvm::AttributeList::FunctionIndex,
+                          llvm::Attribute::NoBuiltin);
+    }
+  }
+
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(CurFuncDecl))
     if (FD->hasAttr<StrictFPAttr>())
       // All calls within a strictfp function are marked strictfp
       Attrs =
         Attrs.addAttribute(getLLVMContext(), llvm::AttributeList::FunctionIndex,
                            llvm::Attribute::StrictFP);
 
   // Add nomerge attribute to the call-site if the callee function doesn't have

clang/lib/CodeGen/CodeGenModule.h

private:
 
   /// Lazily create the Objective-C runtime
   void createObjCRuntime();
 
   void createOpenCLRuntime();
   void createOpenMPRuntime();
   void createCUDARuntime();
 
-  bool isTriviallyRecursive(const FunctionDecl *F);
   bool shouldEmitFunction(GlobalDecl GD);
   bool shouldOpportunisticallyEmitVTables();
   /// Map used to be sure we don't emit the same CompoundLiteral twice.
   llvm::DenseMap<const CompoundLiteralExpr *, llvm::GlobalVariable *>
       EmittedCompoundLiterals;
 
   /// Map of the global blocks we've emitted, so that we don't have to re-emit
   /// them if the constexpr evaluator gets aggressive.

clang/lib/CodeGen/CodeGenModule.cpp

 #include "clang/AST/CharUnits.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Basic/Version.h"
if (const auto *RT = T->getBaseElementTypeUnsafe()->getAs<RecordType>())
     if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl()))
       if (RD->getDestructor() && !RD->getDestructor()->hasAttr<DLLImportAttr>())
         return true;
 
   return false;
 }
 
 namespace {
-  struct FunctionIsDirectlyRecursive
-      : public ConstStmtVisitor<FunctionIsDirectlyRecursive, bool> {
-    const StringRef Name;
-    const Builtin::Context &BI;
-    FunctionIsDirectlyRecursive(StringRef N, const Builtin::Context &C)
-        : Name(N), BI(C) {}
-
-    bool VisitCallExpr(const CallExpr *E) {
-      const FunctionDecl *FD = E->getDirectCallee();
-      if (!FD)
-        return false;
-      AsmLabelAttr *Attr = FD->getAttr<AsmLabelAttr>();
-      if (Attr && Name == Attr->getLabel())
-        return true;
-      unsigned BuiltinID = FD->getBuiltinID();
-      if (!BuiltinID || !BI.isLibFunction(BuiltinID))
-        return false;
-      StringRef BuiltinName = BI.getName(BuiltinID);
-      if (BuiltinName.startswith("__builtin_") &&
-          Name == BuiltinName.slice(strlen("__builtin_"), StringRef::npos)) {
-        return true;
-      }
-      return false;
-    }
-
-    bool VisitStmt(const Stmt *S) {
-      for (const Stmt *Child : S->children())
-        if (Child && this->Visit(Child))
-          return true;
-      return false;
-    }
-  };
 
   // Make sure we're not referencing non-imported vars or functions.
   struct DLLImportFunctionVisitor
       : public RecursiveASTVisitor<DLLImportFunctionVisitor> {
     bool SafeToInline = true;
 
     bool shouldVisitImplicitCode() const { return true; }
 
struct DLLImportFunctionVisitor
 
     bool VisitCXXNewExpr(CXXNewExpr *E) {
       SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
       return SafeToInline;
     }
   };
 }
 
-// isTriviallyRecursive - Check if this function calls another
-// decl that, because of the asm attribute or the other decl being a builtin,
-// ends up pointing to itself.
-bool
-CodeGenModule::isTriviallyRecursive(const FunctionDecl *FD) {
-  StringRef Name;
-  if (getCXXABI().getMangleContext().shouldMangleDeclName(FD)) {
-    // asm labels are a special kind of mangling we have to support.
-    AsmLabelAttr *Attr = FD->getAttr<AsmLabelAttr>();
-    if (!Attr)
-      return false;
-    Name = Attr->getLabel();
-  } else {
-    Name = FD->getName();
-  }
-
-  FunctionIsDirectlyRecursive Walker(Name, Context.BuiltinInfo);
-  const Stmt *Body = FD->getBody();
-  return Body ? Walker.Visit(Body) : false;
-}
 
 bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
   if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage)
     return true;
   const auto *F = cast<FunctionDecl>(GD.getDecl());
   if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
     return false;
 
   if (F->hasAttr<DLLImportAttr>()) {
     // Check whether it would be safe to inline this dllimport function.
     DLLImportFunctionVisitor Visitor;
     Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F));
     if (!Visitor.SafeToInline)
       return false;
 
     if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(F)) {
       // Implicit destructor invocations aren't captured in the AST, so the
       // check above can't see them. Check for them manually here.
       for (const Decl *Member : Dtor->getParent()->decls())
         if (isa<FieldDecl>(Member))
           if (HasNonDllImportDtor(cast<FieldDecl>(Member)->getType()))
             return false;
       for (const CXXBaseSpecifier &B : Dtor->getParent()->bases())
         if (HasNonDllImportDtor(B.getType()))
           return false;
     }
   }
 
-  // PR9614. Avoid cases where the source code is lying to us. An available
-  // externally function should have an equivalent function somewhere else,
-  // but a function that calls itself through asm label/`__builtin_` trickery is
-  // clearly not equivalent to the real implementation.
-  // This happens in glibc's btowc and in some configure checks.
-  return !isTriviallyRecursive(F);
+  return true;
 }
 
 bool CodeGenModule::shouldOpportunisticallyEmitVTables() {
   return CodeGenOpts.OptimizationLevel > 0;
 }
 
 void CodeGenModule::EmitMultiVersionFunctionDefinition(GlobalDecl GD,
                                                        llvm::GlobalValue *GV) {

clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c

-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -disable-llvm-passes -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NO-OPT %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-OPT %s
 //
 // Verifies that clang doesn't mark an inline builtin definition as `nobuiltin`
 // if the builtin isn't emittable.
 
 typedef unsigned long size_t;
 
-// always_inline is used so clang will emit this body. Otherwise, we need >=
-// -O1.
+// always_inline is used so clang will emit this body.
 #define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
     __attribute__((gnu_inline))
 
+// When always_inliner is on, @memcpy is removed and inlined
+// CHECK-OPT-NOT: define i8* @memcpy
+// CHECK-OPT-LABEL: define void @foo
+// CHECK-OPT: call void @llvm.memcpy
+
+// When always_inliner is off, @memcpy is generated and called
+// CHECK-NO-OPT-LABEL: define void @foo
+// CHECK-NO-OPT: call i8* @memcpy
+// CHECK-NO-OPT: define available_externally i8* @memcpy
+
+// This is inlined and suppressed at -O0 thanks to gnu_inline + always_inline
 AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) {
   return __builtin_memcpy(a, b, c);
 }
 
 // CHECK-LABEL: define{{.*}} void @foo
 void foo(void *a, const void *b, size_t c) {
   // Clang will always _emit_ this as memcpy. LLVM turns it into @llvm.memcpy
-  // later on if optimizations are enabled.
-  // CHECK: call i8* @memcpy
+  // even at -O0
   memcpy(a, b, c);
 }
 
-// CHECK-NOT: nobuiltin

clang/test/CodeGen/memmove-always-inline-definition-used.c

+// Verifies that even at -O0, the inline definition of memmove has precedence over the builtin
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -disable-llvm-passes -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-NO-INLINED %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=CHECK-INLINED %s
+
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
+__attribute__((gnu_inline))
+
+typedef unsigned long size_t;
+
+extern void *memmove_alias(void *a, const void *b, size_t c) __asm__("memmove");
+
+// Under -disable-llvm-passes foo is calling the actual memmove function and not the builtin.
+// The call to memmove through memmove_alias is marked as builtin to avoid the recursive call.
+// CHECK-NO-INLINED-LABEL: define void @foo
+// CHECK-NO-INLINED:       call i8* @memmove
+//
+// CHECK-NO-INLINED:       define available_externally i8* @memmove(i8* %{{[a-z0-9]*}}, i8* %{{[a-z0-9]*}}, i64 %{{[a-z0-9]*}}) #[[ATTR1:[0-9]+]]
+// CHECK-NO-INLINED:       call void @llvm.memcpy
+// CHECK-NO-INLINED:       call i8* @memmove({{.*}}) #[[ATTR2:[0-9]+]]
+// CHECK-NO-INLINED:       attributes #[[ATTR1]] = { alwaysinline nobuiltin
+// CHECK-NO-INLINED:       attributes #[[ATTR2]] = { builtin }
+
+// Without -disable-llvm-passes always_inline triggers, the call to memmove in foo is inlined.
+// CHECK-INLINED-NOT:   define available_externally i8* @memmove
+// CHECK-INLINED-LABEL: define void @foo
+// CHECK-INLINED:       call void @llvm.memcpy
+// CHECK-INLINED:       call i8* @memmove({{.*}}) #[[ATTR:[0-9]+]]
+// CHECK-INLINED:       declare i8* @memmove
+// CHECK-INLINED:       attributes #[[ATTR]] = { builtin
+
+AVAILABLE_EXTERNALLY void *memmove(void *a, const void *b, size_t c) {
+  if (c == 1 && a != b) {
+    return __builtin_memcpy(a, b, c);
+  } else {
+    return memmove_alias(a, b, c);
+  }
+}
+
+void foo(void *a, const void *b, size_t c) {
+  memmove(a, b, c);
+}
+

clang/test/CodeGen/pr9614.c

-// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-llvm %s -o - | FileCheck --check-prefix CHECK-INLINED %s
 
 extern void foo_alias (void) __asm ("foo");
 inline void foo (void) {
   return foo_alias ();
 }
 extern int abs_alias (int) __asm ("abs");
 inline __attribute__ ((__always_inline__)) int abs (int x) {
   return abs_alias(x);

 void f(void) {
   foo();
   abs(0);
   strrchr_foo("", '.');
   prefetch();
   memchr("", '.', 0);
 }
 
+// With -disable-llvm-passes, the always_inliner doesn't run so we can observe
+// how clang handles always inline + gnu inline redefinition of builtins.
+// each builtin (re)definition calls the actual builtin using either the
+// intrinsic or a call to selfe flagged as 'builtin', while the function itself
+// is flagged as 'nobuiltin'
+
 // CHECK-LABEL: define{{.*}} void @f()
 // CHECK: call void @foo()
 // CHECK: call i32 @abs(i32 0)
 // CHECK: call i8* @strrchr(
-// CHECK: call void @llvm.prefetch.p0i8(
+// CHECK: call void @prefetch(
 // CHECK: call i8* @memchr(
 // CHECK: ret void
-
-// CHECK: declare void @foo()
-// CHECK: declare i32 @abs(i32
-// CHECK: declare i8* @strrchr(i8*, i32)
-// CHECK: declare i8* @memchr(
-// CHECK: declare void @llvm.prefetch.p0i8(
+//
+// CHECK-LABEL: define available_externally i32 @abs({{.*}}) #2
+// CHECK: call i32 @abs({{.*}}) #7
+//
+// CHECK-LABEL: define available_externally i8* @strrchr({{.*}}) #3
+// CHECK: call i8* @strrchr({{.*}}) #8
+//
+// CHECK-LABEL: define available_externally void @prefetch() #4
+// CHECK: call void @llvm.prefetch.p0i8
+//
+// CHECK-LABEL: define available_externally i8* @memchr({{.*}}) #3
+// CHECK: call i8* @memchr({{.*}}) #8
+//
+// CHECK: attributes #2 = { alwaysinline nobuiltin nounwind
+// CHECK: attributes #3 = { alwaysinline nobuiltin nounwind
+// CHECK: attributes #4 = { alwaysinline nounwind
+// CHECK: attributes #7 = { builtin }
+// CHECK: attributes #8 = { builtin nounwind }
+
+// With always_inliner one, the inlined definition all gets removed
+// how clang handles always inline + gnu inline redefinition of builtins.
+// each builtin (re)definition calls the actual builtin using either the
+// intrinsic or a call to selfe flagged as 'builtin', while the function itself
+// is flagged as 'nobuiltin'
+
+// CHECK-INLINED-NOT: define void @foo
+// CHECK-INLINED-NOT: define i32 @abs
+// CHECK-INLINED-NOT: define i8* @strrchr
+// CHECK-INLINED-NOT: define i8* @memchr
+// CHECK-INLINED-NOT: define void @llvm.prefetch.p0i8
+//
+// CHECK-INLINED: define void @f()
+// CHECK-INLINED: call void @foo
+// CHECK-INLINED: call i8* @strrchr
+// CHECK-INLINED: call void @llvm.prefetch.p0i8
+// CHECK-INLINED: call i8* @memchr
+//
+// CHECK-INLINED: declare void @foo
+// CHECK-INLINED: declare i32 @abs
+// CHECK-INLINED: declare i8* @strrchr
+// CHECK-INLINED: declare i8* @memchr
+// CHECK-INLINED: declare void @llvm.prefetch.p0i8

llvm/lib/Analysis/InlineCost.cpp

if (BB.hasAddressTaken())
         if (!isa<CallBrInst>(*U))
           return InlineResult::failure("blockaddress used outside of callbr");
 
     for (auto &II : BB) {
       CallBase *Call = dyn_cast<CallBase>(&II);
       if (!Call)
         continue;
 
-      // Disallow recursive calls.
+      // Disallow recursive calls, unless marked as builtin
       Function *Callee = Call->getCalledFunction();
-      if (&F == Callee)
+      if (!Call->hasFnAttr(Attribute::Builtin) && &F == Callee)
         return InlineResult::failure("recursive call");
 
       // Disallow calls which expose returns-twice to a function not previously
       // attributed as such.
       if (!ReturnsTwice && isa<CallInst>(Call) &&
           cast<CallInst>(Call)->canReturnTwice())
         return InlineResult::failure("exposes returns-twice attribute");
 
       if (Callee)
InlineResult llvm::isInlineViable(Function &F) {
   return InlineResult::success();
 }
 
 // APIs to create InlineParams based on command line flags and/or other
 // parameters.
 
 InlineParams llvm::getInlineParams(int Threshold) {
   InlineParams Params;
 
   // This field is the threshold to use for a callee by default. This is
   // derived from one or more of:
   //  * optimization or size-optimization levels,
   //  * a value passed to createFunctionInliningPass function, or
   //  * the -inline-threshold flag.
   //  If the -inline-threshold flag is explicitly specified, that is used
   //  irrespective of anything else.
   if (InlineThreshold.getNumOccurrences() > 0)
     Params.DefaultThreshold = InlineThreshold;

llvm/lib/IR/Function.cpp

 
 bool Function::isDefTriviallyDead() const {
   // Check the linkage
   if (!hasLinkOnceLinkage() && !hasLocalLinkage() &&
       !hasAvailableExternallyLinkage())
     return false;
 
   // Check if the function is used by anything other than a blockaddress.
-  for (const User *U : users())
+  for (const User *U : users()) {
     if (!isa<BlockAddress>(U))
       return false;
+  }
 
   return true;
 }
 
 /// callsFunctionThatReturnsTwice - Return true if the function has a call to
 /// setjmp or other function that gcc recognizes as "returning twice".
 bool Function::callsFunctionThatReturnsTwice() const {
   for (const Instruction &I : instructions(this))

llvm/lib/Transforms/IPO/Inliner.cpp

 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/BuildLibCalls.h"
 #include "llvm/Transforms/Utils/CallPromotionUtils.h"
 #include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ImportedFunctionsInliningStatistics.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <algorithm>
 #include <cassert>
 #include <functional>
for (const auto &I : CG) {
     // between here and the InlineAlways pass.
     if (AlwaysInlineOnly && !F->hasFnAttribute(Attribute::AlwaysInline))
       continue;
 
     // If the only remaining users of the function are dead constants, remove
     // them.
     F->removeDeadConstantUsers();
 
+    // If all remaining users are calling F and are marked as Builtin while F is
+    // NoBuiltin, then we can just promote F to the equivalent builtin. This
+    // situation happens after inlining always_inline + gnu_inline redefinition
+    // of builtins as found in glibc headers.
+    if (F->hasFnAttribute(Attribute::NoBuiltin) &&
+        F->hasFnAttribute(Attribute::AlwaysInline)) {
+      bool AllUsesAreBuiltin = true;
+      for (User *U : F->users()) {
+        if (auto const *CB = dyn_cast<CallBase>(U)) {
+          if (CB->getCalledFunction() == F &&
+              CB->hasFnAttr(Attribute::Builtin)) {
+            continue;
+          }
+        }
+        AllUsesAreBuiltin = false;
+        break;
+      }
+      if (AllUsesAreBuiltin) {
+        F->deleteBody();
+        F->setAttributes({});
+        inferLibFuncAttributes(
+            *F, getAnalysis<TargetLibraryInfoWrapperPass>().getTLI(*F));
+      }
+    }
+
     if (!F->isDefTriviallyDead())
       continue;
 
     // It is unsafe to drop a function with discardable linkage from a COMDAT
     // without also dropping the other members of the COMDAT.
     // The inliner doesn't visit non-function entities which are in COMDAT
     // groups so it is unsafe to do so *unless* the linkage is local.
     if (!F->hasLocalLinkage()) {