[clang] 6bfc85c - Fix inline builtin handling in case of redefinition

[via cfe-commits]

Author: serge-sans-paille
Date: 2021-11-02T09:53:49+01:00
New Revision: 6bfc85c217e443ecbde37b142a01dc7925792edc

URL: https://github.com/llvm/llvm-project/commit/6bfc85c217e443ecbde37b142a01dc7925792edc
DIFF: https://github.com/llvm/llvm-project/commit/6bfc85c217e443ecbde37b142a01dc7925792edc.diff

LOG: Fix inline builtin handling in case of redefinition

Basically, inline builtin definition are shadowed by externally visible
redefinition. This matches GCC behavior.

The implementation has to workaround the fact that:

1. inline builtin are renamed at callsite during codegen, but
2. they may be shadowed by a later external definition

As a consequence, during codegen, we need to walk redecls and eventually rewrite
some call sites, which is totally inelegant.

Differential Revision: https://reviews.llvm.org/D112059

Added: 
    clang/test/CodeGen/strlen-inline-builtin-redecl.c
    clang/test/CodeGen/user-func-gnu-inline-redecl.c

Modified: 
    clang/lib/CodeGen/CodeGenFunction.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index fc4540f26aabc..c3c2899e93ec7 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1299,18 +1299,40 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
   // When generating code for a builtin with an inline declaration, use a
   // mangled name to hold the actual body, while keeping an external definition
   // in case the function pointer is referenced somewhere.
-  if (FD->isInlineBuiltinDeclaration() && Fn) {
-    std::string FDInlineName = (Fn->getName() + ".inline").str();
-    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);
-      Clone->addFnAttr(llvm::Attribute::AlwaysInline);
+  if (Fn) {
+    if (FD->isInlineBuiltinDeclaration()) {
+      std::string FDInlineName = (Fn->getName() + ".inline").str();
+      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);
+        Clone->addFnAttr(llvm::Attribute::AlwaysInline);
+      }
+      Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
+      Fn = Clone;
+    }
+
+    // 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. Unfortunately, I cannot find a way
+    // to detect that situation before we reach codegen, so do some late
+    // replacement.
+    else {
+      for (const FunctionDecl *PD = FD->getPreviousDecl(); PD;
+           PD = PD->getPreviousDecl()) {
+        if (LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
+          std::string FDInlineName = (Fn->getName() + ".inline").str();
+          llvm::Module *M = Fn->getParent();
+          if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
+            Clone->replaceAllUsesWith(Fn);
+            Clone->eraseFromParent();
+          }
+          break;
+        }
+      }
     }
-    Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
-    Fn = Clone;
   }
 
   // Check if we should generate debug info for this function.

diff  --git a/clang/test/CodeGen/strlen-inline-builtin-redecl.c b/clang/test/CodeGen/strlen-inline-builtin-redecl.c
new file mode 100644
index 0000000000000..c89f843a71a3e
--- /dev/null
+++ b/clang/test/CodeGen/strlen-inline-builtin-redecl.c
@@ -0,0 +1,21 @@
+// 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
+
+// CHECK-NOT: strlen.inline
+
+unsigned long strnlen(const char *, unsigned long);
+void fortify_panic(const char *);
+
+extern inline __attribute__((always_inline)) __attribute__((gnu_inline)) unsigned long strlen(const char *p) {
+  return 1;
+}
+unsigned long mystrlen(char const *s) {
+  return strlen(s);
+}
+unsigned long strlen(const char *s) {
+  return 2;
+}
+unsigned long yourstrlen(char const *s) {
+  return strlen(s);
+}

diff  --git a/clang/test/CodeGen/user-func-gnu-inline-redecl.c b/clang/test/CodeGen/user-func-gnu-inline-redecl.c
new file mode 100644
index 0000000000000..0415cbe1e6c70
--- /dev/null
+++ b/clang/test/CodeGen/user-func-gnu-inline-redecl.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -O1 -o - %s | FileCheck %s
+//
+// Verifies that the gnu_inline version is ignored in favor of the redecl
+
+extern inline __attribute__((gnu_inline)) unsigned long some_size(int c) {
+  return 1;
+}
+unsigned long mycall(int s) {
+  // CHECK-LABEL: i64 @mycall
+  // CHECK: ret i64 2
+  return some_size(s);
+}
+unsigned long some_size(int c) {
+  return 2;
+}
+unsigned long yourcall(int s) {
+  // CHECK-LABEL: i64 @yourcall
+  // CHECK: ret i64 2
+  return some_size(s);
+}