Skip to content

[LangRef] Disallow accessing byval arguments from tail-called functions #110093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 27, 2024
Merged

[LangRef] Disallow accessing byval arguments from tail-called functions #110093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5a640a7
[LangRef] Disallow accessing byval arguments from tail-called functions
ostannard Sep 24, 2024
23d692b
Allow forwarding byvals, add examples
ostannard Sep 26, 2024
6569652
Clarify wording around allocas, add example
ostannard Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 61 additions & 4 deletions llvm/docs/LangRef.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12658,10 +12658,67 @@ 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, va_args, or
byval arguments from the caller. As an exception to that, an alloca or byval
argument may be passed to the callee as a byval argument, which can be
dereferenced inside the callee. For example:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Phrasing here is a little unclear; the "unless that pointer is passed to the callee as a byval argument" part applies to allocas as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and added an example of passing an alloca as a byval argument.


.. code-block:: llvm

declare void @take_byval(ptr byval(i64))
declare void @take_ptr(ptr)

; Invalid (assuming @take_ptr dereferences the pointer), because %local
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe say "invalid to access"; passing around the raw pointer bits of a dead pointer is fine, although probably not useful in most cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

; may be de-allocated before the call to @take_ptr.
define void @invalid_alloca() {
entry:
%local = alloca i64
tail call void @take_ptr(ptr %local)
ret void
}

; Valid, the byval attribute causes the memory allocated by %local to be
; copied into @take_byval's stack frame.
define void @byval_alloca() {
entry:
%local = alloca i64
tail call void @take_byval(ptr byval(i64) %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 (assuming @take_ptr dereferences the pointer), 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.
Expand Down
19 changes: 0 additions & 19 deletions llvm/test/CodeGen/ARM/struct_byval.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to delete these tests; they have well-defined behavior. Maybe add a comment if you're concerned someone reading the test will get confused. (If @consumestruct tried to access memory through the pointer, that would be wrong, but it doesn't, as far as the compiler can tell.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to delete them, because the comment above says they are testing something not guaranteed by the LangRef, and with #109943 they will actually be tail-called.

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
Expand Down
Loading