[PATCH] ARM: Do not tail-call to an externally-defined function with weak linkage

[Oliver Stannard]

I originally stated that "we cannot rely on the linker replacing the tail call with a return", but after further thought I now realise that it is impossible for the linker to replace a tail-call with a return, as it may need to insert a stack pointer adjustment, but it does not know how big the adjustment must be.

Both armlink and arm-none-eabi-ld replace branch instructions to undefined weak symbols with a NOP, so this is not just an armlink problem.

The AAELF spec only states this for "platforms that do not support dynamic pre-emption of symbols", so I have modified the patch to only prevent the tail call for bare-metal systems.

http://reviews.llvm.org/D4906

Files:
  lib/Transforms/Scalar/TailRecursionElimination.cpp
  test/Transforms/TailCallElim/AArch64/lit.local.cfg
  test/Transforms/TailCallElim/AArch64/weak-linkage.ll
  test/Transforms/TailCallElim/ARM/lit.local.cfg
  test/Transforms/TailCallElim/ARM/weak-linkage.ll

Index: lib/Transforms/Scalar/TailRecursionElimination.cpp
===================================================================
--- lib/Transforms/Scalar/TailRecursionElimination.cpp
+++ lib/Transforms/Scalar/TailRecursionElimination.cpp
@@ -54,6 +54,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/CFG.h"
 #include "llvm/Analysis/InlineCost.h"
@@ -298,6 +299,22 @@
       if (!CI || CI->isTailCall())
         continue;
 
+      // Externally-defined functions with weak linkage should not be
+      // tail-called on ARM or AArch64 when the OS does not support dynamic
+      // pre-emption of symbols, as the AAELF spec requires normal calls
+      // to undefined weak functions to be replaced with a NOP or jump to the
+      // next instruction. The behaviour of branch instructions in this
+      // situation (as used for tail calls) is implementation-defined, so we
+      // cannot rely on the linker replacing the tail call with a return.
+      GlobalValue *Callee = dyn_cast<GlobalValue>(CI->getCalledValue());
+      Triple T(F.getParent()->getTargetTriple());
+      if ((T.getArch() == Triple::arm || T.getArch() == Triple::armeb ||
+          T.getArch() == Triple::thumb || T.getArch() == Triple::thumbeb ||
+          T.getArch() == Triple::aarch64 || T.getArch() == Triple::aarch64_be)
+          && T.getOS() == Triple::UnknownOS
+          && Callee && Callee->hasExternalWeakLinkage())
+        continue;
+
       if (CI->doesNotAccessMemory()) {
         // A call to a readnone function whose arguments are all things computed
         // outside this function can be marked tail. Even if you stored the
Index: test/Transforms/TailCallElim/AArch64/lit.local.cfg
===================================================================
--- /dev/null
+++ test/Transforms/TailCallElim/AArch64/lit.local.cfg
@@ -0,0 +1,3 @@
+if not 'AArch64' in config.root.targets:
+    config.unsupported = True
+
Index: test/Transforms/TailCallElim/AArch64/weak-linkage.ll
===================================================================
--- /dev/null
+++ test/Transforms/TailCallElim/AArch64/weak-linkage.ll
@@ -0,0 +1,12 @@
+; RUN: opt -mtriple=aarch64--none-eabi  < %s -tailcallelim -S | FileCheck %s --check-prefix=NONE
+; RUN: opt -mtriple=aarch64--linux-eabi < %s -tailcallelim -S | FileCheck %s --check-prefix=LINUX
+
+declare extern_weak void @weak_function()
+
+; Don't tail call to a weak symbol on bare metal AArch64
+define void @func1() {
+; NONE-NOT: tail call
+; LINUX: tail call
+  call void @weak_function()
+  ret void
+}
Index: test/Transforms/TailCallElim/ARM/lit.local.cfg
===================================================================
--- /dev/null
+++ test/Transforms/TailCallElim/ARM/lit.local.cfg
@@ -0,0 +1,3 @@
+if not 'ARM' in config.root.targets:
+    config.unsupported = True
+
Index: test/Transforms/TailCallElim/ARM/weak-linkage.ll
===================================================================
--- /dev/null
+++ test/Transforms/TailCallElim/ARM/weak-linkage.ll
@@ -0,0 +1,12 @@
+; RUN: opt -mtriple=armv7a--none-eabi  < %s -tailcallelim -S | FileCheck %s --check-prefix=NONE
+; RUN: opt -mtriple=armv7a--linux-eabi < %s -tailcallelim -S | FileCheck %s --check-prefix=LINUX
+
+declare extern_weak void @weak_function()
+
+; Don't tail call to a weak symbol on bare metal ARM
+define void @func1() {
+; NONE-NOT: tail call
+; LINUX: tail call
+  call void @weak_function()
+  ret void
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4906.12551.patch
Type: text/x-patch
Size: 3590 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140815/25d5af99/attachment.bin>