Objective-C++ Wrong Assignment of arguments to registers on Windows ARM64 when returning an instance of C++ Class. #88273
Milestone
Comments
EugeneZelenko
added
clang
|
I am not quite sure where this "remapping" happens. Probably related to #86384 |
|
@davidchisnall If x0 is used in the MSVC ABI when returning C++ objects, this means it would only be practical to have something like When returning large structures, x8 is used as defined in the ABI and |
|
This looks like a result of calling the non-sret version when it should be calling the sret one. This is probably due to the change in the MSVC ABI a few years ago to support NVRO. |
But the sret version was called. It is just that sret also expects receiver in x0 and selector in x1. |
|
But we cannot change this because normal struct returns (of the correct size) use x8 and not x0 for the pointer to mem. |
|
Hmm, does the Visual Studio ABI have different registers for sret C++ classes vs C ones? |
|
Here an extended example. #import <Foundation/Foundation.h>
struct abc {
long long a;
double b;
long long c;
};
class Bar
{
public:
Bar(int i) : m_i(i) {}
private:
int m_i;
};
@interface Test : NSObject
@end
@implementation Test
+ (int)foo
{
return 42;
}
+ (struct abc)fooStruct
{
struct abc a;
a.a = 1;
a.b = 2.0;
a.c = 3;
return a;
}
+ (Bar)bar
{
return Bar(42);
}
@end
int main(int argc, char *argv[])
{
auto pair = [Test bar]; // crashes
auto fooStruct = [Test fooStruct];
return 0;
}Both calls use
Assembly of main functionmain: // @main
.Lfunc_begin3:
.cv_func_id 4
.cv_loc 4 1 48 0 // crash.mm:48:0
.seh_proc main
// %bb.0:
//DEBUG_VALUE: main:argv <- $x1
//DEBUG_VALUE: main:argc <- $w0
sub sp, sp, #64
.seh_stackalloc 64
stp x19, x20, [sp, #32] // 16-byte Folded Spill
.seh_save_regp x19, 32
str x30, [sp, #48] // 8-byte Folded Spill
.seh_save_reg x30, 48
.seh_endprologue
mov x0, #44 // =0x2c
.Ltmp6:
//DEBUG_VALUE: main:argc <- [DW_OP_LLVM_entry_value 1] $w0
.cv_loc 4 1 49 0 // crash.mm:49:0
adrp x19, __imp_NSLog
movk x0, #57344, lsl #16
ldr x19, [x19, :lo12:__imp_NSLog]
movk x0, #26317, lsl #32
movk x0, #37271, lsl #48
blr x19
.Ltmp7:
//DEBUG_VALUE: main:argv <- [DW_OP_LLVM_entry_value 1] $x1
.cv_loc 4 1 51 0 // crash.mm:51:0
adrp x20, ($_OBJC_REF_CLASS_Test)
adrp x2, ".objc_selector_bar_{Bar�i}16@0:8"
add x2, x2, :lo12:".objc_selector_bar_{Bar�i}16@0:8"
ldr x1, [x20, :lo12:($_OBJC_REF_CLASS_Test)]
add x0, sp, #60
bl objc_msgSend_stret
.cv_loc 4 1 52 0 // crash.mm:52:0
ldr x0, [x20, :lo12:($_OBJC_REF_CLASS_Test)]
adrp x1, ".objc_selector_fooStruct_{abc�qdq}16@0:8"
add x1, x1, :lo12:".objc_selector_fooStruct_{abc�qdq}16@0:8"
add x8, sp, #8
bl objc_msgSend_stret
mov x0, #32828 // =0x803c
movk x0, #48377, lsl #16
movk x0, #7740, lsl #32
movk x0, #42967, lsl #48
.cv_loc 4 1 54 0 // crash.mm:54:0
blr x19
.cv_loc 4 1 56 0 // crash.mm:56:0
mov w0, wzr
.seh_startepilogue
ldr x30, [sp, #48] // 8-byte Folded Reload
.seh_save_reg x30, 48
ldp x19, x20, [sp, #32] // 16-byte Folded Reload
.seh_save_regp x19, 32
add sp, sp, #64
.seh_stackalloc 64
.seh_endepilogue
ret |
|
Hmm, from the assembly dump, it looks like the call frame is set up how I'd expect from the old version of the ABI (small struct returned in registers), but it's calling the sret version. Clang tries to determine whether the |
Thank you :)
Sure! LLVM IR of Extended Example |
|
That’s weird. Both of the calls have an sret parameter emitted by the front end, but somehow the second one isn’t using it in the back end? |
|
Maybe when mapping the parameters to registers, the Visual Studio ABI uses x8 for normal structs and x0 for C++ sret? |
|
I guess we may need another variant of the message send for that calling convention. |
I guess we do not need to care about this: // In AAPCS, an SRet is passed in X8, not X0 like a normal pointer parameter.
// However, on windows, in some circumstances, the SRet is passed in X0 or X1
// instead. The presence of the inreg attribute indicates that SRet is
// passed in the alternative register (X0 or X1), not X8:
// - X0 for non-instance methods.
// - X1 for instance methods.
// The "sret" attribute identifies indirect returns.
// The "inreg" attribute identifies non-aggregate types.
// The position of the "sret" attribute identifies instance/non-instance
// methods.
// "sret" on argument 0 means non-instance methods.
// "sret" on argument 1 means instance methods.This means after detecting Thank you @efriedma-quic for the link. It was very helpful! |
|
I am now working on a patch for CodeGen and libobjc2 |
|
I don't think you need to patch libobjc2. We don't touch x8 except to spill and reload it when calling C), so it should be fine to use the non-stret version for calls that treat x8 as (effectively) an argument register. We need to make sure that, in CodeGen, we treat both as structure returns for the purpose of zeroing the return in the caller. It doesn't matter that objc_msgSend zeroes the normal return registers, they're caller-save anyway. |
That would mean we still need to modify the objc_msgSendStret, as it currently expects receiver to be in x0 and selector in x1. |
|
And this in turn would break behaviour when using previous clang versions. I think there is no other way around then to just have a second stret msgSend. |
This was referenced Apr 13, 2024
davidchisnall
pushed a commit
that referenced
this issue
Apr 25, 2024
…8671) Linked to gnustep/libobjc2#289. More information can be found in issue: #88273. My solution involves creating a new message-send function for this calling convention when targeting MSVC. Additional information is available in the libobjc2 pull request. I am unsure whether we should check for a runtime version where objc_msgSend_stret2_np is guaranteed to be present or leave it as is, considering it remains a critical bug. What are your thoughts about this @davidchisnall?
|
Do you think this makes sense to cherry-pick to 18.x? |
|
Probably nice to have. We’ll do a new point release of the runtime to include the new function. |
|
For us it would be helpful to have an official Clang 18 release with this fix and also #86384, so cherry-picking to 18.x would be appreciated if possible. |
|
/cherry-pick 3dcd2cc |
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this issue
Apr 26, 2024
…vm#88671) Linked to gnustep/libobjc2#289. More information can be found in issue: llvm#88273. My solution involves creating a new message-send function for this calling convention when targeting MSVC. Additional information is available in the libobjc2 pull request. I am unsure whether we should check for a runtime version where objc_msgSend_stret2_np is guaranteed to be present or leave it as is, considering it remains a critical bug. What are your thoughts about this @davidchisnall? (cherry picked from commit 3dcd2cc)
|
/pull-request #90176 |
EugeneZelenko
added
clang:codegen
release:backport
and removed
clang
|
@llvm/issue-subscribers-clang-codegen Author: Hugo Melder (hmelder)
The libobjc2 `objc_msgSend` implementation for aarch64 expects the receiver and selector to be passed in x0 and x1 respectively.
On Windows ARM64, the pointer to the (uninitialised) instance of the C++ Class is assigned to x0, shifting receiver and selector into x1 and x2. This results in a crash. This does not happen on Ubuntu aarch64, as seen in the lldb snippet below. I am using the GNUstep Windows SDK from https://github.com/gnustep/tools-windows-msvc. Here is the test code: #import <Foundation/Foundation.h>
class Bar
{
public:
Bar(int i) : m_i(i) {}
private:
int m_i;
};
@<!-- -->interface Test : NSObject
@<!-- -->end
@<!-- -->implementation Test
+ (Bar)bar
{
return Bar(42);
}
@<!-- -->end
int main(int argc, char *argv[])
{
NSLog(@"Hello");
auto pair = [Test bar]; // crashes
NSLog(@"Success");
return 0;
}
Windows 11 ARM64Machine Information WindowsBuildLabEx: 22621.1.arm64fre.ni_release.220506-1250
WindowsProductName: Windows 10 Pro
OSDisplayVersion: 23H2
WindowsKit: 10.0.22621.0Clang Version clang version 18.1.3
Target: aarch64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\LLVM\binBuild Command (Inside MSYS2) clang crash.mm -o crash.exe -g -gcodeview `gnustep-config --objc-flags` `gnustep-config --base-libs` -Xclang -fobjc-dispatch-method=non-legacyLLDB Process 6408 stopped
* thread #<!-- -->1, stop reason = breakpoint 2.1
frame #<!-- -->0: 0x00007ff738e61048 crash.exe`main(argc=867171612, argv=0x00007ff738e640b0) at crash.mm:27
24 {
25 NSLog(@"Hello");
26
-> 27 auto pair = [Test bar]; // crashes
28
29 NSLog(@"Success");
30
(lldb) di
crash.exe`main:
0x7ff738e6100c <+0>: sub sp, sp, #<!-- -->0x20
0x7ff738e61010 <+4>: str x19, [sp, #<!-- -->0x10]
0x7ff738e61014 <+8>: str x30, [sp, #<!-- -->0x18]
0x7ff738e61018 <+12>: mov x0, #<!-- -->0x2c ; =44
0x7ff738e6101c <+16>: adrp x19, 2
0x7ff738e61020 <+20>: movk x0, #<!-- -->0xe000, lsl #<!-- -->16
0x7ff738e61024 <+24>: ldr x19, [x19, #<!-- -->0x5e8]
0x7ff738e61028 <+28>: movk x0, #<!-- -->0x66cd, lsl #<!-- -->32
0x7ff738e6102c <+32>: movk x0, #<!-- -->0x9197, lsl #<!-- -->48
0x7ff738e61030 <+36>: blr x19
0x7ff738e61034 <+40>: adrp x8, 5
0x7ff738e61038 <+44>: adrp x2, 5
0x7ff738e6103c <+48>: add x2, x2, #<!-- -->0x10 ; __start_.objcrt$PCR
0x7ff738e61040 <+52>: ldr x1, [x8]
0x7ff738e61044 <+56>: add x0, sp, #<!-- -->0xc
-> 0x7ff738e61048 <+60>: bl 0x7ff738e62384 ; objc_msgSend_stret
0x7ff738e6104c <+64>: mov x0, #<!-- -->0x803c ; =32828
0x7ff738e61050 <+68>: movk x0, #<!-- -->0xbcf9, lsl #<!-- -->16
0x7ff738e61054 <+72>: movk x0, #<!-- -->0x1e3c, lsl #<!-- -->32
0x7ff738e61058 <+76>: movk x0, #<!-- -->0xa7d7, lsl #<!-- -->48
0x7ff738e6105c <+80>: blr x19
0x7ff738e61060 <+84>: mov w0, wzr
0x7ff738e61064 <+88>: ldr x30, [sp, #<!-- -->0x18]
0x7ff738e61068 <+92>: ldr x19, [sp, #<!-- -->0x10]
0x7ff738e6106c <+96>: add sp, sp, #<!-- -->0x20
0x7ff738e61070 <+100>: ret
(lldb) register read
General Purpose Registers:
x0 = 0x000000e233affd1c
x1 = 0x00007ff738e640b0 $_OBJC_CLASS_Test
x2 = 0x00007ff738e66010 __start_.objcrt$PCR
x3 = 0x0000000010000000
x4 = 0x0000000000000150
x5 = 0x000000006d9d3bcf
x6 = 0x00007ff8b4129000 NlsAnsiCodePage + 26720
x7 = 0x5d4b1dcd6d097720
x8 = 0x00007ff738e66000 __start_.objcrt$CAL
x9 = 0x0000000000000000
x10 = 0x000000007ffe0380
x11 = 0x0000000000000000
x12 = 0x0000000000000000
x13 = 0xa2e64eada2e64ead
x14 = 0x0000000000000001
x15 = 0x0000000000000070
x16 = 0x0000000080000001
x17 = 0x00005859193355a3
x18 = 0x000000e233970000
x19 = 0x00007ff857942570 gnustep-base-1_29.dll`NSLog at NSLog.m:293
x20 = 0x000002a841621080
x21 = 0x0000000000000000
x22 = 0x0000000000000000
x23 = 0x0000000000000000
x24 = 0x0000000000000000
x25 = 0x0000000000000000
x26 = 0x0000000000000000
x27 = 0x0000000000000000
x28 = 0x0000000000000000
fp = 0x000000e233affd30
lr = 0x00007ff738e61034 crash.exe`main + 40 at crash.mm:27
sp = 0x000000e233affd10
pc = 0x00007ff738e61048 crash.exe`main + 60 at crash.mm:27
cpsr = 0x80000000
(lldb)Ubuntu 23.10 aarch64Clang Version Ubuntu clang version 18.1.3 (++20240322073236+ef6d1ec07c69-1~exp1~20240322193248.98)
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/binBuild Command clang-18 ObjcCXXObjectReturnTest.mm -o ObjcCXXObjectReturnTest -g `gnustep-config --objc-flags` `gnustep-config --base-libs` -Xclang -fobjc-dispatch-method=non-legacy -fuse-ld=lld-18* thread #<!-- -->1, name = 'ObjcCXXObjectRe', stop reason = breakpoint 2.1
frame #<!-- -->0: 0x0000aaaaaaab0dcc ObjcCXXObjectReturnTest`main(argc=<unavailable>, argv=<unavailable>) at ObjcCXXObjectReturnTest.mm:27:14
24 {
25 NSLog(@"Hello");
26
-> 27 auto pair = [Test bar]; // crashes
28
29 NSLog(@"Success");
30
(lldb) di
ObjcCXXObjectReturnTest`main:
0xaaaaaaab0d9c <+0>: stp x29, x30, [sp, #-0x10]!
0xaaaaaaab0da0 <+4>: mov x29, sp
0xaaaaaaab0da4 <+8>: mov x0, #<!-- -->0x2c ; =44
0xaaaaaaab0da8 <+12>: movk x0, #<!-- -->0xe000, lsl #<!-- -->16
0xaaaaaaab0dac <+16>: movk x0, #<!-- -->0x66cd, lsl #<!-- -->32
0xaaaaaaab0db0 <+20>: movk x0, #<!-- -->0x9197, lsl #<!-- -->48
0xaaaaaaab0db4 <+24>: bl 0xaaaaaaab0e90 ; symbol stub for: NSLog
0xaaaaaaab0db8 <+28>: adrp x8, 17
0xaaaaaaab0dbc <+32>: nop
0xaaaaaaab0dc0 <+36>: adr x1, 0xaaaaaaad12b0 ; ObjcCXXObjectReturnTest.PT_LOAD[3].__objc_selectors + 0
0xaaaaaaab0dc4 <+40>: ldr x8, [x8, #<!-- -->0xd0]
0xaaaaaaab0dc8 <+44>: ldr x0, [x8]
-> 0xaaaaaaab0dcc <+48>: bl 0xaaaaaaab0ea0 ; symbol stub for: objc_msgSend
0xaaaaaaab0dd0 <+52>: mov x0, #<!-- -->0x803c ; =32828
0xaaaaaaab0dd4 <+56>: movk x0, #<!-- -->0xbcf9, lsl #<!-- -->16
0xaaaaaaab0dd8 <+60>: movk x0, #<!-- -->0x1e3c, lsl #<!-- -->32
0xaaaaaaab0ddc <+64>: movk x0, #<!-- -->0xa7d7, lsl #<!-- -->48
0xaaaaaaab0de0 <+68>: bl 0xaaaaaaab0e90 ; symbol stub for: NSLog
0xaaaaaaab0de4 <+72>: mov w0, wzr
0xaaaaaaab0de8 <+76>: ldp x29, x30, [sp], #<!-- -->0x10
0xaaaaaaab0dec <+80>: ret
(lldb) register read
General Purpose Registers:
x0 = 0x0000aaaaaaad11a0 ._OBJC_CLASS_Test
x1 = 0x0000aaaaaaad12b0 ObjcCXXObjectReturnTest.PT_LOAD[3].__objc_selectors + 0
x2 = 0x0000000000000007
x3 = 0x0000aaaaaaad2010
x4 = 0x0000000000000004
x5 = 0x0000aaaaaaebc7f0
x6 = 0xb3c97132ac52cb5b
x7 = 0x0000fffff7e1f3a8 ._OBJC_CLASS_GSMutableString
x8 = 0x0000aaaaaaad12d0 ObjcCXXObjectReturnTest`._OBJC_REF_CLASS_Test
x9 = 0x0000000000000000
x10 = 0x0000fffff7f884cc libobjc.so.4.6`objc_slot_lookup_super2 + 264
x11 = 0x0000000000000003
x12 = 0x0000fffff7f87bd0 libobjc.so.4.6`objc_msg_lookup_sender + 100
x13 = 0x0000000000000003
x14 = 0x000000000042a2d5
x15 = 0x0000ffffffff8b88
x16 = 0x0000fffff7fb00d0
x17 = 0x0000fffff76d47f0 libc.so.6`free
x18 = 0x0000000000000034
x19 = 0x0000fffffffff0d8
x20 = 0x0000000000000001
x21 = 0x0000aaaaaaac0ed8
x22 = 0x0000aaaaaaab0d9c ObjcCXXObjectReturnTest`main at ObjcCXXObjectReturnTest.mm:24
x23 = 0x0000fffffffff0e8
x24 = 0x0000fffff7ffdb90 ld-linux-aarch64.so.1`_rtld_global_ro
x25 = 0x0000000000000000
x26 = 0x0000fffff7ffe008 _rtld_global
x27 = 0x0000aaaaaaac0ed8
x28 = 0x0000000000000000
fp = 0x0000ffffffffef50
lr = 0x0000aaaaaaab0db8 ObjcCXXObjectReturnTest`main + 28 at ObjcCXXObjectReturnTest.mm:27:14
sp = 0x0000ffffffffef50
pc = 0x0000aaaaaaab0dcc ObjcCXXObjectReturnTest`main + 48 at ObjcCXXObjectReturnTest.mm:27:14
cpsr = 0x40001000
|
tstellar
pushed a commit
to llvmbot/llvm-project
that referenced
this issue
Apr 29, 2024
…vm#88671) Linked to gnustep/libobjc2#289. More information can be found in issue: llvm#88273. My solution involves creating a new message-send function for this calling convention when targeting MSVC. Additional information is available in the libobjc2 pull request. I am unsure whether we should check for a runtime version where objc_msgSend_stret2_np is guaranteed to be present or leave it as is, considering it remains a critical bug. What are your thoughts about this @davidchisnall? (cherry picked from commit 3dcd2cc)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The libobjc2
objc_msgSendimplementation for aarch64 expects the receiver and selector to be passed in x0 and x1 respectively.On Windows ARM64, the pointer to the (uninitialised) instance of the C++ Class is assigned to x0, shifting receiver and selector into x1 and x2. This results in a crash.
This does not happen on Ubuntu aarch64, as seen in the lldb snippet below.
I am using the GNUstep Windows SDK from https://github.com/gnustep/tools-windows-msvc.
Here is the test code:
Windows 11 ARM64
Machine Information
Clang Version
Build Command (Inside MSYS2)
LLDB
Ubuntu 23.10 aarch64
Clang Version
Build Command
The text was updated successfully, but these errors were encountered: