hmelder
commented
6 months ago I am not quite sure where this "remapping" happens. Probably related to https://github.com/llvm/llvm-project/issues/86384
Closed hmelder closed 6 months ago
hmelder
commented
6 months ago I am not quite sure where this "remapping" happens. Probably related to https://github.com/llvm/llvm-project/issues/86384
hmelder
commented
6 months ago @davidchisnall If x0 is used in the MSVC ABI when returning C++ objects, this means it would only be practical to have something like objc_msgSendCXX (where x1 is receiver and x2 selector) as further remapping of arguments before msgSend will break the ABI when tail-calling IMP.
When returning large structures, x8 is used as defined in the ABI and objc_msgSendStret works just fine.
davidchisnall
commented
6 months ago 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.
hmelder
commented
6 months ago This looks like a result of calling the non-sret version when it should be calling the sret one
But the sret version was called. It is just that sret also expects receiver in x0 and selector in x1.
-> 0x7ff738e61048 <+60>: bl 0x7ff738e62384 ; objc_msgSend_stretBut we cannot change this because normal struct returns (of the correct size) use x8 and not x0 for the pointer to mem.
davidchisnall
commented
6 months ago Hmm, does the Visual Studio ABI have different registers for sret C++ classes vs C ones?
hmelder
commented
6 months ago 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 objc_msgSendSret.
auto pair = [Test bar];: x0: Implicit argument, x1: Receiver, x2: Selectorauto fooStruct = [Test fooStruct]: x0: Receiver, x1: Selector
davidchisnall
commented
6 months ago 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 sret pointer is used here, which should match whether it emits the sret argument. Can you show me the LLVM IR for the main function here?
hmelder
commented
6 months ago Clang tries to determine whether the sret pointer is used here, which should match whether it emits the sret argument.
Thank you :)
Can you show me the LLVM IR for the main function here
Sure!
davidchisnall
commented
6 months ago 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?
hmelder
commented
6 months ago Maybe when mapping the parameters to registers, the Visual Studio ABI uses x8 for normal structs and x0 for C++ sret?
davidchisnall
commented
6 months ago I guess we may need another variant of the message send for that calling convention.
efriedma-quic
commented
6 months ago hmelder
commented
6 months ago For return-by-value that cannot be passed via registers, the caller shall reserve a block of memory of sufficient size and alignment to hold the result. The address of the memory block shall be passed as an additional argument to the function in x8 for POD type, or in x0 (or x1 if $this is passed in x0) for non-POD type.
I guess we do not need to care about this: x1 if $this is passed in x0, and only the case where non-POD (POD meaning plain old data) types are returned in x0.
// 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 sret here, we additionally check if the inreg attribute is set on argument 0. If set, a new msgSend function (objc_msgSendSret2_np, objc_msgSendSretCXX_np, or perhaps objc_msgSendSretInreg_np?) with different parameter mapping is used instead.
Thank you @efriedma-quic for the link. It was very helpful!
hmelder
commented
6 months ago I am now working on a patch for CodeGen and libobjc2
davidchisnall
commented
6 months ago 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.
hmelder
commented
6 months ago so it should be fine to use the non-stret version for calls that treat x8 as (effectively) an argument register.
That would mean we still need to modify the objc_msgSendStret, as it currently expects receiver to be in x0 and selector in x1.
hmelder
commented
6 months ago 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.
efriedma-quic
commented
6 months ago Do you think this makes sense to cherry-pick to 18.x?
davidchisnall
commented
6 months ago Probably nice to have. We’ll do a new point release of the runtime to include the new function.
triplef
commented
6 months ago 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.
efriedma-quic
commented
6 months ago /cherry-pick 3dcd2cc
llvmbot
commented
6 months ago /pull-request llvm/llvm-project#90176
llvmbot
commented
6 months ago @llvm/issue-subscribers-clang-codegen
Author: Hugo Melder (hmelder)
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