serge-sans-paille created this revision.
serge-sans-paille added reviewers: mstorsjo, george.burgess.iv, efriedma.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If a system header provides an (inline) implementation of some of their 
function, clang still matches on the function name and generate the appropriate 
llvm builtin, e.g. memcpy. This behavior is in line with glibc recommendation « 
users may not provide their own version of symbols » but doesn't account for 
the fact that glibc itself can provide inline version of some functions.

It is the case for the memcpy function when -D_FORTIFY_SOURCE=1 is on. In that 
case an inline version of memcpy calls __memcpy_chk, a function that performs 
extra runtime checks. Clang currently ignores the inline version and thus 
provides no runtime check.

This code fixes the issue by detecting functions whose name is a builtin name 
but also have a system-provided implementation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuitin.c
  clang/test/CodeGen/memcpy-nobuitin.inc


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include <stddef.h>
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+    *idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: clang -S -emit-llvm -o- %s -isystem . -DWITH_DECL | FileCheck 
--check-prefix=CHECK-WITH-DECL %s
+// RUN: clang -S -emit-llvm -o- %s -isystem . -UWITH_DECL | FileCheck 
--check-prefix=CHECK-NO-DECL %s
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+#include <memcpy-nobuitin.inc>
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr<SectionAttr>())
      F->setSection(SA->getName());
 
+  if (FD->isReplaceableSystemFunction()) {
+    F->addAttribute(llvm::AttributeList::FunctionIndex,
+                    llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
     // A replaceable global allocation function does not act like a builtin by
     // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,15 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {
+    const SourceManager &SM = getASTContext().getSourceManager();
+    return SM.isInSystemHeader(Definition->getLocation());
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
@@ -3165,6 +3174,12 @@
   // function. Determine whether it actually refers to the C library
   // function or whether it just has the same name.
 
+  // If a system-level body was provided, use it instead of the intrinsic. Some
+  // C library do that to implement fortified version.
+  if (isReplaceableSystemFunction()) {
+    return 0;
+  }
+
   // If this is a static function, it's not a builtin.
   if (!ConsiderWrapperFunctions && getStorageClass() == SC_Static)
     return 0;
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides the implementation of a system 
Builtin
+  bool isReplaceableSystemFunction() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include <stddef.h>
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+    *idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: clang -S -emit-llvm -o- %s -isystem . -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: clang -S -emit-llvm -o- %s -isystem . -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+#include <memcpy-nobuitin.inc>
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr<SectionAttr>())
      F->setSection(SA->getName());
 
+  if (FD->isReplaceableSystemFunction()) {
+    F->addAttribute(llvm::AttributeList::FunctionIndex,
+                    llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
     // A replaceable global allocation function does not act like a builtin by
     // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/AST/Decl.cpp
===================================================================
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,15 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {
+    const SourceManager &SM = getASTContext().getSourceManager();
+    return SM.isInSystemHeader(Definition->getLocation());
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
@@ -3165,6 +3174,12 @@
   // function. Determine whether it actually refers to the C library
   // function or whether it just has the same name.
 
+  // If a system-level body was provided, use it instead of the intrinsic. Some
+  // C library do that to implement fortified version.
+  if (isReplaceableSystemFunction()) {
+    return 0;
+  }
+
   // If this is a static function, it's not a builtin.
   if (!ConsiderWrapperFunctions && getStorageClass() == SC_Static)
     return 0;
Index: clang/include/clang/AST/Decl.h
===================================================================
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides the implementation of a system Builtin
+  bool isReplaceableSystemFunction() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to