[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 232667.
serge-sans-paille added a comment.

Fix interaction with fortify warnings (cc @efriedma)

Also I've checked interactions with the test suite I wrote here https://github.com/serge-sans-paille/fortify-test-suite/ and clang now passes all the dynamic check, and only fails a few static checks, which is a great improvement. I still need to double check all the examples pointed out by @george.burgess.iv .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71082/new/

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 %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -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 (!ConsiderWrapperFunctions && 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;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71082.232667.patch
Type: text/x-patch
Size: 3450 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20191206/e846aae1/attachment.bin>