- 
                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 2 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.