[PATCH] D123308: Handle a subtle hole in inline builtin handling

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: nickdesaulniers, aaron.ballman, tstellar.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123308

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/fread-inline-builtin-late-redecl.c


Index: clang/test/CodeGen/fread-inline-builtin-late-redecl.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/fread-inline-builtin-late-redecl.c
@@ -0,0 +1,26 @@
+// 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);
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===================================================================
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4948,6 +4948,16 @@
   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());
 
@@ -4955,7 +4965,7 @@
     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() &&
+    if (OnlyHasInlineBuiltinDeclaration(FD) &&
         CGF.CurFn->getName() != FDInlineName) {
       llvm::Constant *CalleePtr = EmitFunctionDeclPointer(CGF.CGM, GD);
       llvm::Function *Fn = llvm::cast<llvm::Function>(CalleePtr);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123308.421187.patch
Type: text/x-patch
Size: 2286 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220407/8c8a1aaf/attachment.bin>