Index: clang/lib/CodeGen/CGCall.cpp =================================================================== --- clang/lib/CodeGen/CGCall.cpp +++ clang/lib/CodeGen/CGCall.cpp @@ -4866,6 +4866,18 @@ 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 these as NoInline so that + // they're not considered recursive. + if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) { + if (AsmLabelAttr *Attr = FD->getAttr()) { + if (CurFn->getName() == Attr->getLabel()) + Attrs = Attrs.addAttribute(getLLVMContext(), + llvm::AttributeList::FunctionIndex, + llvm::Attribute::NoInline); + } + } + if (const FunctionDecl *FD = dyn_cast_or_null(CurFuncDecl)) if (FD->hasAttr()) // All calls within a strictfp function are marked strictfp Index: clang/lib/CodeGen/CodeGenModule.h =================================================================== --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -506,7 +506,7 @@ void createOpenMPRuntime(); void createCUDARuntime(); - bool isTriviallyRecursive(const FunctionDecl *F); + bool isTriviallyAlwaysRecursive(const FunctionDecl *F); bool shouldEmitFunction(GlobalDecl GD); bool shouldOpportunisticallyEmitVTables(); /// Map used to be sure we don't emit the same CompoundLiteral twice. Index: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -32,9 +32,12 @@ #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/Mangle.h" +#include "clang/AST/ParentMap.h" #include "clang/AST/RecordLayout.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/StmtVisitor.h" +#include "clang/Analysis/Analyses/Dominators.h" +#include "clang/Analysis/CFGStmtMap.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/CharInfo.h" #include "clang/Basic/CodeGenOptions.h" @@ -2855,38 +2858,47 @@ } namespace { - struct FunctionIsDirectlyRecursive - : public ConstStmtVisitor { - 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(); - 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; - } +struct FunctionIsDirectlyAlwaysRecursive + : public ConstStmtVisitor { + const StringRef Name; + const Builtin::Context &BI; + CFGStmtMap &SM; + CFGPostDomTree Dom; + CFGBlock &Entry; + FunctionIsDirectlyAlwaysRecursive(StringRef N, const Builtin::Context &C, + CFG &cfg, CFGStmtMap &SM) + : Name(N), BI(C), SM(SM), Entry(cfg.getEntry()) { + Dom.buildDominatorTree(&cfg); + } + + bool VisitCallExpr(const CallExpr *E) { + const FunctionDecl *FD = E->getDirectCallee(); + if (!FD) + return false; + AsmLabelAttr *Attr = FD->getAttr(); + 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)) + bool VisitStmt(const Stmt *S) { + for (const Stmt *Child : S->children()) + if (Child && this->Visit(Child)) { + if (Dom.dominates(SM.getBlock(S), &Entry)) { return true; - return false; - } - }; + } + } + return false; + } +}; // Make sure we're not referencing non-imported vars or functions. struct DLLImportFunctionVisitor @@ -2952,11 +2964,10 @@ }; } -// isTriviallyRecursive - Check if this function calls another +// isTriviallyAlwaysRecursive - Check if this function always 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) { +bool CodeGenModule::isTriviallyAlwaysRecursive(const FunctionDecl *FD) { StringRef Name; if (getCXXABI().getMangleContext().shouldMangleDeclName(FD)) { // asm labels are a special kind of mangling we have to support. @@ -2968,9 +2979,18 @@ Name = FD->getName(); } - FunctionIsDirectlyRecursive Walker(Name, Context.BuiltinInfo); - const Stmt *Body = FD->getBody(); - return Body ? Walker.Visit(Body) : false; + Stmt *Body = FD->getBody(); + if (Body) { + std::unique_ptr cfg = + CFG::buildCFG(FD, Body, &Context, CFG::BuildOptions()); + ParentMap PM(Body); + auto SM = std::unique_ptr(CFGStmtMap::Build(&*cfg, &PM)); + FunctionIsDirectlyAlwaysRecursive Walker(Name, Context.BuiltinInfo, *cfg, + *SM); + return Walker.Visit(Body); + } else { + return false; + } } bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) { @@ -3005,7 +3025,7 @@ // 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 !isTriviallyAlwaysRecursive(F); } bool CodeGenModule::shouldOpportunisticallyEmitVTables() { Index: clang/test/CodeGen/memmove-always-inline-definition-used.c =================================================================== --- /dev/null +++ clang/test/CodeGen/memmove-always-inline-definition-used.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s +// +// Verifies that even at O0, the inline definition of memmove has precedence +// over the builtin + +typedef unsigned long size_t; + +#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \ +__attribute__((gnu_inline)) + +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 __builtin_memmove(a, b, c); +} + +// CHECK-LABEL: define void @foo +void foo(void *a, const void *b, size_t c) { + // Clang should use the inline definition of memmove here. + // CHECK: call void @llvm.memcpy + // CHECK: call void @llvm.memmove + memmove(a, b, c); +} + +// CHECK-NOT: nobuiltin Index: llvm/lib/Analysis/InlineCost.cpp =================================================================== --- llvm/lib/Analysis/InlineCost.cpp +++ llvm/lib/Analysis/InlineCost.cpp @@ -2449,9 +2449,9 @@ if (!Call) continue; - // Disallow recursive calls. + // Disallow recursive calls, unless marked as no-inline Function *Callee = Call->getCalledFunction(); - if (&F == Callee) + if (!Call->hasFnAttr(Attribute::NoInline) && &F == Callee) return InlineResult::failure("recursive call"); // Disallow calls which expose returns-twice to a function not previously