-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[Clang] Add __builtin_stack_address
#148281
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.
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
029d9fc
0b5e0ed
2b8084a
020909e
88db438
0669180
aea6944
20d0259
da68490
6cdcf58
c56be28
1ed8a5d
1339b31
64eb4ad
7c38381
d47cb56
1ba60ce
4f1fad5
6ced7f8
fdb520a
1f77682
57e9fe5
ae3b2b6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| // RUN: %clang -target x86_64 -S -emit-llvm %s -o - | FileCheck %s --check-prefix=llvm | ||
| // RUN: %clang -target x86_64 -S %s -o - | FileCheck %s --check-prefix=x64 | ||
|
|
||
| extern void f(int, int, int, long, long, long, long, long, long, long, long); | ||
|
|
||
| // llvm-LABEL: define {{[^@]+}} @a() | ||
| // llvm: call {{[^@]+}} @llvm.stackaddress.p0() | ||
| // | ||
| // x64-LABEL: a: | ||
| // x64: movq %rsp, %rax | ||
| void *a() { | ||
| f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); | ||
| return __builtin_stack_address(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // RUN: %clang -target x86_64 -S -emit-llvm %s -o - | llvm-cxxfilt | FileCheck %s --check-prefix=llvm | ||
| // RUN: %clang -target x86_64 -S %s -o - | llvm-cxxfilt | FileCheck %s --check-prefix=x64 | ||
|
|
||
| extern void f(int, int, int, long, long, long, long, long, long, long, long); | ||
|
|
||
| struct S { | ||
| void *a(); | ||
| }; | ||
|
|
||
| // llvm-LABEL: define {{[^@]+}} @S::a() | ||
| // llvm: call {{[^@]+}} @llvm.stackaddress.p0() | ||
| // | ||
| // x64-LABEL: S::a(): | ||
| // x64: movq %rsp, %rax | ||
| void *S::a() { | ||
| void *p = __builtin_stack_address(); | ||
| f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); | ||
| return p; | ||
| } | ||
|
|
||
| // llvm-LABEL: define {{[^@]+}} @two() | ||
| // llvm: call {{[^@]+}} @"two()::$_0::operator()() const" | ||
| // | ||
| // llvm-LABEL: define {{[^@]+}} @"two()::$_0::operator()() const" | ||
| // llvm: call {{[^@]+}} @llvm.stackaddress.p0() | ||
| // | ||
| // x64-LABEL: two()::$_0::operator()() const: | ||
| // x64: movq %rsp, %rax | ||
| void *two() { | ||
| auto l = []() { | ||
| void *p = __builtin_stack_address(); | ||
| f(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11); | ||
| return p; | ||
| }; | ||
| return l(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| // RUN: %clang_cc1 -verify %s -triple x86_64-unknown-unknown -DTEST_x64 | ||
| // RUN: %clang_cc1 -verify %s -triple i386-unknown-unknown -DTEST_x86 | ||
| // RUN: %clang_cc1 -verify %s -triple riscv32-unknown-unknown -DTEST_riscv32 | ||
| // RUN: %clang_cc1 -verify %s -triple riscv64-unknown-unknown -DTEST_riscv64 | ||
| // RUN: %clang_cc1 -verify %s -triple aarch64-unknown-unknown -DTEST_aarch64 | ||
|
|
||
| #if defined(TEST_x64) || defined(TEST_x86) | ||
| // expected-no-diagnostics | ||
| void *a() { | ||
| return __builtin_stack_address(); | ||
| } | ||
| #else | ||
| void *a() { | ||
| return __builtin_stack_address(); // expected-error {{builtin is not supported on this target}} | ||
| } | ||
| #endif | ||
Sirraide marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -36,3 +36,8 @@ void* h(unsigned x) { | |||||||||
| // expected-error@+1 {{argument value 1048575 is outside the valid range [0, 65535]}} | ||||||||||
| return __builtin_frame_address(0xFFFFF); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| void *i() { | ||||||||||
| // expected-error@+1 {{too many arguments to function call, expected 0, have 1}} | ||||||||||
| return __builtin_stack_address(0); | ||||||||||
|
||||||||||
| // expected-error@+1 {{too many arguments to function call, expected 0, have 1}} | |
| return __builtin_stack_address(0); | |
| // expected-error@+1 {{too many arguments to function call, expected 0, have 1}} | |
| return __builtin_stack_address(0); |
Other test cases I would like to see are:
// As a global variable where there is no stack address to get.
// This should be diagnosed as an error?
void *ptr = __builtin_stack_address();
inline void *what() {
return __builtin_stack_address();
}
void func() {
void *ptr = what(); // Is this getting the stack address of? The inline function or is it getting the stack address of the caller? Or depends on optimization level?
}
Should we also have tests for what happens with naked functions or other kind of odd situations where the stack may be different from the normal case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// As a global variable where there is no stack address to get.
// This should be diagnosed as an error?
void *ptr = __builtin_stack_address();
It looks like GCC doesn't diagnose this as an error (https://godbolt.org/z/x35e114de) and returns the stack address of the initialization function. I'm not sure if it's intended or not.
Should we still diagnose this as an error?
Should we also have tests for what happens with naked functions or other kind of odd situations where the stack may be different from the normal case?
Noted, will add them, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// As a global variable where there is no stack address to get.
// This should be diagnosed as an error?
void *ptr = __builtin_stack_address();It looks like GCC doesn't diagnose this as an error (https://godbolt.org/z/x35e114de) and returns the stack address of the initialization function. I'm not sure if it's intended or not. Should we still diagnose this as an error?
My intuition is that it's kinder to the user to reject trying to get a stack address when there's no stack involved, but I don't know that it's a strongly held opinion. CC @nikic @efriedma-quic for more opinions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't evaluate this at compile-time, so we know this is running at runtime. And at runtime, we have a stack: the stack of __cxx_global_var_init(). So the semantics here seem fine, if maybe a little weird at first glance. And trying to restrict the set of builtins you can use from runtime init leads to weird questions, like what happens if you have a function static inline void* stackaddr() { return __builtin_stack_address(); }.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes a lot more sense to me, thank you! Then I'm happy with the current behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also have tests for what happens with naked functions or other kind of odd situations where the stack may be different from the normal case?
I might need a little bit of help with this, it seems like you can't have non-ASM code inside an __attribute__((naked)) function, so you can't call __builtin_stack_address() inside one.
I'm also not very sure about the odd situations where the stack may be different from the normal case. Are you referring to something similar to an interrupt handler (i.e. __attribute__((interrupt)) void isr(void*)?
Using something like __attribute__((interrupt)) simply translates to an additional function attribute in the generated IR (e.g. x86_intrcc). Is it more appropriate to test those scenarios in llvm/test/CodeGen/${Target}/ instead?
Uh oh!
There was an error while loading. Please reload this page.