[llvm] [LangRef] Disallow accessing byval arguments from tail-called functions (PR #110093)

[Oliver Stannard via llvm-commits]

https://github.com/ostannard created https://github.com/llvm/llvm-project/pull/110093

We already disallow accessing the callee's allocas from a tail-called function, because their stack memory will have been de-allocated before the tail call. I think this should apply to byval arguments too, as they also occupy space in the caller's stack frame.

This was originally part of #109943, spilt out for separate review.

>From 5a640a78c0fc97400f49a1f0c07021e82ef70def Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Tue, 24 Sep 2024 11:07:19 +0100
Subject: [PATCH 1/2] [LangRef] Disallow accessing byval arguments from
 tail-called functions

We already disallow accessing the callee's allocas from a tail-called
function, because their stack memory will have been de-allocated before
the tail call. I think this should apply to byval arguments too, as they
also occupy space in the celler's stack frame.
---
 llvm/docs/LangRef.rst                 |  8 ++++----
 llvm/test/CodeGen/ARM/struct_byval.ll | 19 -------------------
 2 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 91c3e60bb0acb1..441a1998a04606 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -12658,10 +12658,10 @@ This instruction requires several arguments:
       the return value of the callee is returned to the caller's caller, even
       if a void return type is in use.
 
-   Both markers imply that the callee does not access allocas from the caller.
-   The ``tail`` marker additionally implies that the callee does not access
-   varargs from the caller. Calls marked ``musttail`` must obey the following
-   additional  rules:
+   Both markers imply that the callee does not access allocas or ``byval``
+   arguments from the caller. The ``tail`` marker additionally implies that the
+   callee does not access varargs from the caller. Calls marked ``musttail``
+   must obey the following additional rules:
 
    - The call must immediately precede a :ref:`ret <i_ret>` instruction,
      or a pointer bitcast followed by a ret instruction.
diff --git a/llvm/test/CodeGen/ARM/struct_byval.ll b/llvm/test/CodeGen/ARM/struct_byval.ll
index 73a1b5ee33bca9..2bc4f9c816d539 100644
--- a/llvm/test/CodeGen/ARM/struct_byval.ll
+++ b/llvm/test/CodeGen/ARM/struct_byval.ll
@@ -63,25 +63,6 @@ declare i32 @e1(ptr nocapture byval(%struct.SmallStruct) %in) nounwind
 declare i32 @e2(ptr nocapture byval(%struct.LargeStruct) %in) nounwind
 declare i32 @e3(ptr nocapture byval(%struct.LargeStruct) align 16 %in) nounwind
 
-; rdar://12442472
-; We can't do tail call since address of s is passed to the callee and part of
-; s is in caller's local frame.
-define void @f3(ptr nocapture byval(%struct.SmallStruct) %s) nounwind optsize {
-; CHECK-LABEL: f3
-; CHECK: bl _consumestruct
-entry:
-  tail call void @consumestruct(ptr %s, i32 80) optsize
-  ret void
-}
-
-define void @f4(ptr nocapture byval(%struct.SmallStruct) %s) nounwind optsize {
-; CHECK-LABEL: f4
-; CHECK: bl _consumestruct
-entry:
-  tail call void @consumestruct(ptr %s, i32 80) optsize
-  ret void
-}
-
 ; We can do tail call here since s is in the incoming argument area.
 define void @f5(i32 %a, i32 %b, i32 %c, i32 %d, ptr nocapture byval(%struct.SmallStruct) %s) nounwind optsize {
 ; CHECK-LABEL: f5

>From 23d692b08ac7645d954101cf714f9ede56121aaa Mon Sep 17 00:00:00 2001
From: Oliver Stannard <oliver.stannard at arm.com>
Date: Thu, 26 Sep 2024 10:22:29 +0100
Subject: [PATCH 2/2] Allow forwarding byvals, add examples

---
 llvm/docs/LangRef.rst | 53 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 441a1998a04606..4cb2ddf9a01ac7 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -12658,10 +12658,55 @@ This instruction requires several arguments:
       the return value of the callee is returned to the caller's caller, even
       if a void return type is in use.
 
-   Both markers imply that the callee does not access allocas or ``byval``
-   arguments from the caller. The ``tail`` marker additionally implies that the
-   callee does not access varargs from the caller. Calls marked ``musttail``
-   must obey the following additional rules:
+   Both markers imply that the callee does not access allocas or varargs from
+   the caller. They also imply that the callee does not access the memory
+   pointed to by a byval argument to the caller, unless that pointer is passed
+   to the callee as a byval argument. For example:
+
+.. code-block:: llvm
+
+      declare void @take_byval(ptr byval(i64))
+      declare void @take_ptr(ptr)
+
+      ; Invalid, %local may be de-allocated before call to @take_ptr.
+      define void @invalid_alloca() {
+      entry:
+        %local = alloca i64
+        tail call void @take_ptr(ptr %local)
+        ret void
+      }
+
+      ; Invalid, because @use_global_va_list uses the variadic arguments from @invalid_va_list.
+      %struct.va_list = type { ptr }
+      @va_list = external global %struct.va_list
+      define void @use_global_va_list() {
+      entry:
+        %arg = va_arg ptr @va_list, i64
+        ret void
+      }
+      define void @invalid_va_list(i32 %a, ...) {
+      entry:
+        call void @llvm.va_start.p0(ptr @va_list)
+        tail call void @use_global_va_list()
+        ret void
+      }
+
+      ; Valid, byval argument forwarded to tail call as another byval argument.
+      define void @forward_byval(ptr byval(i64) %x) {
+      entry:
+        tail call void @take_byval(ptr byval(i64) %x)
+        ret void
+      }
+
+      ; Invalid, byval argument passed to tail callee as non-byval ptr.
+      define void @invalid_byval(ptr byval(i64) %x) {
+      entry:
+        tail call void @take_ptr(ptr %x)
+        ret void
+      }
+
+
+   Calls marked ``musttail`` must obey the following additional rules:
 
    - The call must immediately precede a :ref:`ret <i_ret>` instruction,
      or a pointer bitcast followed by a ret instruction.