When a method returns a type that the platform ABI says is returned in memory, the Objective-C frontend (for the NeXT runtime ABIs) emits a call to `objc_msgSend_stret`; the `_stret` variant of the message dispatcher knows to look at the *second* argument for `self`, and so on, since the first argument is the pointer to where the return value will go. On some platforms, `_Complex double` is too large to be returned through registers, so it's returned through memory. But the Objective-C frontend still insists on using `objc_msgSend`, not `objc_msgSend_stret`. `objc_msgSend` is thoroughly confused when the first argument, which it expects to be `self`, is actually a pointer to garbage-filled stack. I believe this happens because this check: ``` /* If we are returning a struct in memory, and the address of that memory location is passed as a hidden first argument, then change which messenger entry point this expr will call. NB: Note that sender_cast remains unchanged (it already has a struct return type). */ if (!targetm.calls.struct_value_rtx (0, 0) && (TREE_CODE (ret_type) == RECORD_TYPE || TREE_CODE (ret_type) == UNION_TYPE) && targetm.calls.return_in_memory (ret_type, 0)) { ``` limits the use of `stret` to record/union types. Primitive types that are returned in memory should also be made to use `stret` (the name's meaning notwithstanding).
(In reply to Matt Jacobson from comment #0) > When a method returns a type that the platform ABI says is returned in > memory, the Objective-C frontend (for the NeXT runtime ABIs) emits a call to > `objc_msgSend_stret`; the `_stret` variant of the message dispatcher knows > to look at the *second* argument for `self`, and so on, since the first > argument is the pointer to where the return value will go. > > On some platforms, `_Complex double` is too large to be returned through > registers, so it's returned through memory. can you give me some idea of the platform? it would help with testing at least (assuming that there is a suitable machine on the cfarm). Having said that, there could be some types for which this would fire even on Darwin. > But the Objective-C frontend still insists on using `objc_msgSend`, not > `objc_msgSend_stret`. > > `objc_msgSend` is thoroughly confused when the first argument, which it > expects to be `self`, is actually a pointer to garbage-filled stack. > > I believe this happens because this check: > > ``` > /* If we are returning a struct in memory, and the address > of that memory location is passed as a hidden first > argument, then change which messenger entry point this > expr will call. NB: Note that sender_cast remains > unchanged (it already has a struct return type). */ > if (!targetm.calls.struct_value_rtx (0, 0) > && (TREE_CODE (ret_type) == RECORD_TYPE > || TREE_CODE (ret_type) == UNION_TYPE) > && targetm.calls.return_in_memory (ret_type, 0)) > { > ``` > > limits the use of `stret` to record/union types. Primitive types that are > returned in memory should also be made to use `stret` (the name's meaning > notwithstanding). makes sense, but the fix might be interesting given the "NB: Note that sender_cast remains unchanged (it already has a struct return type)." which would no longer be true for a primitive type.
(In reply to Iain Sandoe from comment #1) > can you give me some idea of the platform? it would help with testing at > least (assuming that there is a suitable machine on the cfarm). Sure, but I'm guessing it's not going to be very helpful! The platform I'm using is an AVR microcontroller, with my own custom NeXT-v2-ABI-compliant runtime. <https://mjacobson.net/blog/2021-07-objc-avr.html> <https://github.com/mhjacobson/avr-objc> > Having said that, there could be some types for which this would fire even > on Darwin. Hm -- I'm not so sure. The x86_64 calling convention allocates *two* (64-bit) registers to integer/pointer return values, so `__int128` doesn't require memory. Similarly, it can return `_Complex double` via multiple SSE registers. Finally, there's also `_Complex long double`, which is returned via the x87 stack (and for which there is yet another messenger routine, `objc_msgSend_fp2ret`). In fact, a close reading of the ABI spec suggests that the *only* types that would ever be returned through memory are structs and unions. So the code here would work.
(In reply to Matt Jacobson from comment #2) > (In reply to Iain Sandoe from comment #1) > > can you give me some idea of the platform? it would help with testing at > > least (assuming that there is a suitable machine on the cfarm). > > Sure, but I'm guessing it's not going to be very helpful! The platform I'm > using is an AVR microcontroller, with my own custom NeXT-v2-ABI-compliant > runtime. > > <https://mjacobson.net/blog/2021-07-objc-avr.html> > <https://github.com/mhjacobson/avr-objc> As you say, maybe not immediately helpful - but coul even so. > > Having said that, there could be some types for which this would fire even > > on Darwin. > > Hm -- I'm not so sure. It is possible to find an example - if AVX is disabled, GCC returns __m256 in memory (so can be tried for manual testing). However, the clang version of this on Darwin platforms actually returns in xmm0/xmm1 (which has an ABI bug filed against it on LLVM IIRC) - so not useful outside manual testing. My guess is that there are no [other] non-struct cases for the powerpc(64b) and x86 cases, I've not poked at the aarch64 case yet, so the bug went unnoticed all this time. initial patch under test now.
Created attachment 51310 [details] trial fix So it seems that the logic was slightly off, but in a way that would not fire unless a non-aggregate return-in-memory occurred, and those are very rare (non-existent, with the clang ABI) for Darwin which is the only NeXT v2 user with test coverage. the change is: if (ret_type && !VOID_TYPE_P (ret_type) && targetm.calls.return_in_memory (ret_type, 0) && !(targetm.calls.struct_value_rtx (0, 0) && (TREE_CODE (ret_type) == RECORD_TYPE || TREE_CODE (ret_type) == UNION_TYPE))) So this says, "if there's a return value and it's returned in memory and the target makes no special provision for returning aggregates then use the _stret versions". Probably some scope for factoring some of that code - but bug fixes first. Does this work for your platform?
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>: https://gcc.gnu.org/g:1cef3039b880a21fbdf4153e6fc42026619fd4ad commit r12-3292-g1cef3039b880a21fbdf4153e6fc42026619fd4ad Author: Iain Sandoe <iain@sandoe.co.uk> Date: Mon Aug 16 21:22:13 2021 +0100 Objective-C, NeXT: Fix messenging non-aggregate return-in-memory. When a method returns a type that the platform ABI says should be returned in memory, and that is done by a hidden 'sret' parameter, the message send calls must be adjusted to inform the runtime that the sret parameter is present. As reported in the PR, this is not working for non-aggregate types that use this mechanism. The fix here is to adjust the logic such that all return values that flag 'in memory' are considered to use the mechanism *unless* they provide a struct_value_rtx *and* the return object is an aggregate. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> PR objc/101718 - Objective-C frontend emits wrong code to call methods returning scalar types returned in memory PR objc/101718 gcc/objc/ChangeLog: * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Revise for cases where scalar objects use an sret parameter. (next_runtime_abi_02_build_objc_method_call): Likwise.
The releases/gcc-10 branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>: https://gcc.gnu.org/g:903c18c65c4eb135eb3c67a3c14fb6c20f537feb commit r10-10807-g903c18c65c4eb135eb3c67a3c14fb6c20f537feb Author: Iain Sandoe <iain@sandoe.co.uk> Date: Mon Aug 16 21:22:13 2021 +0100 Objective-C, NeXT: Fix messenging non-aggregate return-in-memory. When a method returns a type that the platform ABI says should be returned in memory, and that is done by a hidden 'sret' parameter, the message send calls must be adjusted to inform the runtime that the sret parameter is present. As reported in the PR, this is not working for non-aggregate types that use this mechanism. The fix here is to adjust the logic such that all return values that flag 'in memory' are considered to use the mechanism *unless* they provide a struct_value_rtx *and* the return object is an aggregate. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> PR objc/101718 - Objective-C frontend emits wrong code to call methods returning scalar types returned in memory PR objc/101718 gcc/objc/ChangeLog: * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Revise for cases where scalar objects use an sret parameter. (next_runtime_abi_02_build_objc_method_call): Likwise. (cherry picked from commit 1cef3039b880a21fbdf4153e6fc42026619fd4ad)
still needed on 11.x.
The releases/gcc-11 branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>: https://gcc.gnu.org/g:106cfc476a55c7423dca23be1eb0a5fb5da736b5 commit r11-11302-g106cfc476a55c7423dca23be1eb0a5fb5da736b5 Author: Iain Sandoe <iain@sandoe.co.uk> Date: Mon Aug 16 21:22:13 2021 +0100 Objective-C, NeXT: Fix messenging non-aggregate return-in-memory. When a method returns a type that the platform ABI says should be returned in memory, and that is done by a hidden 'sret' parameter, the message send calls must be adjusted to inform the runtime that the sret parameter is present. As reported in the PR, this is not working for non-aggregate types that use this mechanism. The fix here is to adjust the logic such that all return values that flag 'in memory' are considered to use the mechanism *unless* they provide a struct_value_rtx *and* the return object is an aggregate. Signed-off-by: Iain Sandoe <iain@sandoe.co.uk> PR objc/101718 - Objective-C frontend emits wrong code to call methods returning scalar types returned in memory PR objc/101718 gcc/objc/ChangeLog: * objc-next-runtime-abi-02.c (build_v2_build_objc_method_call): Revise for cases where scalar objects use an sret parameter. (next_runtime_abi_02_build_objc_method_call): Likwise. (cherry picked from commit 1cef3039b880a21fbdf4153e6fc42026619fd4ad)
fixed on open branches (and gcc-10)