[Clang][Fortify] drop inline decls when redeclared

[serge-sans-paille]

Summary

When an inline builtin declaration is shadowed by an actual declaration, we must
reference the actual declaration, even if it's not the last, following GCC
behavior.

This fixes #54715

[nickdesaulniers]

Thanks for the quick fix!

clang/lib/CodeGen/CGExpr.cpp

4967–4969

If OnlyHasInlineBuiltinDeclaration returning false is unusual, can we get away with switching the order of the checks here, so that CGF.CurFn->getName() != FDInlineName which might be more likely might short circuit the conditional? That might help us avoid walking the redecl list in OnlyHasInlineBuiltinDeclaration occasionally.

[nickdesaulniers]

Also, for the initial lines of commit messages, please try to make them less ambiguous about which parts of the project they touch. For example [Clang][Fortify] drop inline decls when redeclared is more descriptive should this get bucketed with other commits in a buildbot failure. The oneline helps triagers quickly ascertain whether failures they observe look related to which change or not.

[serge-sans-paille]

@nickdesaulniers changes taken into account and pushed, thanks for the review!

clang/lib/CodeGen/CGExpr.cpp

 
 /// Emit a CallExpr without considering whether it might be a subclass.
 RValue CodeGenFunction::EmitSimpleCallExpr(const CallExpr *E,
                                            ReturnValueSlot ReturnValue) {
   CGCallee Callee = EmitCallee(E->getCallee());
   return EmitCall(E->getCallee()->getType(), Callee, E, ReturnValue);
 }
 
+// Detect the unusual situation where an inline version is shadowed by a
+// non-inline version. In that case we should pick the external one
+// everywhere. That's GCC behavior too.
+static bool OnlyHasInlineBuiltinDeclaration(const FunctionDecl *FD) {
+  for (const FunctionDecl *PD = FD; PD; PD = PD->getPreviousDecl())
+    if (!PD->isInlineBuiltinDeclaration())
+      return false;
+  return true;
+}
+
 static CGCallee EmitDirectCallee(CodeGenFunction &CGF, GlobalDecl GD) {
   const FunctionDecl *FD = cast<FunctionDecl>(GD.getDecl());
 
   if (auto builtinID = FD->getBuiltinID()) {
     std::string FDInlineName = (FD->getName() + ".inline").str();
     // When directing calling an inline builtin, call it through it's mangled
     // name to make it clear it's not the actual builtin.
-    if (FD->isInlineBuiltinDeclaration() &&
-        CGF.CurFn->getName() != FDInlineName) {
+    if (CGF.CurFn->getName() != FDInlineName &&
+        OnlyHasInlineBuiltinDeclaration(FD)) {
       llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
       llvm::Function *Fn = llvm::cast<llvm::Function>(CalleePtr);
       llvm::Module *M = Fn->getParent();
       llvm::Function *Clone = M->getFunction(FDInlineName);
       if (!Clone) {
         Clone = llvm::Function::Create(Fn->getFunctionType(),
                                        llvm::GlobalValue::InternalLinkage,
                                        Fn->getAddressSpace(), FDInlineName, M);

clang/test/CodeGen/fread-inline-builtin-late-redecl.c

+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+//
+// Verifies that clang-generated *.inline are removed when shadowed by an
+// external definition, even when that definition appears at the end of the
+// file.
+
+// CHECK-NOT: strlen.inline
+
+extern unsigned long strlen(char const *s);
+
+extern __inline __attribute__((__always_inline__)) __attribute__((__gnu_inline__)) unsigned long strlen(char const *s) {
+  return 1;
+}
+
+static unsigned long chesterfield(char const *s) {
+  return strlen(s);
+}
+static unsigned long (*_strlen)(char const *ptr);
+
+unsigned long blutch(char const *s) {
+  return chesterfield(s);
+}
+
+unsigned long strlen(char const *s) {
+  return _strlen(s);
+}