Skip to content

-rac_signalForSelector: may fail for struct returns #783

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

Closed
jspahrsummers opened this issue Sep 6, 2013 · 14 comments
Closed

-rac_signalForSelector: may fail for struct returns #783

jspahrsummers opened this issue Sep 6, 2013 · 14 comments
Assignees
Labels

Comments

@jspahrsummers
Copy link
Member

As pointed out on Twitter by @numist and @steipete, there's an _objc_msgForward_stret which we're not currently using. This could cause -rac_signalForSelector: to fail when targeting a method that returns a struct.

Since this is only an issue for existent methods, it's easy enough to retrieve their Objective-C type encoding. However, we can't simply check for a struct type, since a union may or may not contain a struct, and the <objc/message.h> documentation says that _stret variants should only be used "for some struct return types" (emphasis mine).

Maybe we should just state that struct returns are unsupported here.

@Coneko
Copy link
Member

Coneko commented Sep 6, 2013

https://developer.apple.com/library/mac/documentation/DeveloperTools/Conceptual/LowLevelABI/000-Introduction/introduction.html

The low level ABI expands on what "some struct return types" means. The details might differ between architecture but the gist of it is "structs (or unions that can be structs) bigger than 2 registers unless it's a struct with two floating point fields".

One could get the size of the return type using NSGetSizeAndAlignment, check for the complex number struct special case and then call the appropriate _objc_msgForward variant.

Struct return types are probably a common enough case to justify this.

I thought this had something to do with the lifting, but since it's only for -rac_signalForSelector:, and that is mostly used with void return types not supporting structs and unions shouldn't be a problem.

@kastiglione
Copy link
Member

What about exposing the underlying crack and letting the users call a rac_signalForStructSelector:?

I'm also fine with just documenting that structs aren't supported.

@jspahrsummers
Copy link
Member Author

I'd rather hide the runtime ugliness if we can, since it's a really low-level thing — this is one reason why NSInvocation exists on top of objc_msgSend, for example.

However, I also don't really want to jump through those special casing hoops.

We should probably start with some unit tests, to see whether this is a real problem or just theoretical, because it's really unclear to me why the runtime would behave differently upon finding an IMP that's _objc_msgForward, versus not finding an existing Method at all.

@Coneko
Copy link
Member

Coneko commented Sep 7, 2013

I'll take it as an excuse to procrastinate writing my résumé.

@Coneko
Copy link
Member

Coneko commented Sep 7, 2013

I found the part in clang where the logic for figuring out whether a function is supposed to return the value through registers or memory: lib/CodeGen/TargetInfo.cpp line 2016 for x86_64 and line 3395 for ARM.

Assuming _objc_msgForward_stret must be called for selectors that return the value in memory, and _objc_msgForward for those that return it through registers, we can use that as a base.

I'm still of the opinion we're better off documenting -rac_signalForSelector: doesn't support all return types.

@kastiglione
Copy link
Member

it's really unclear to me why the runtime would behave differently upon finding an IMP that's _objc_msgForward, versus not finding an existing Method at all.

I was wondering the same. Looks like objc has an private intermediary version, _objc_msgForward_internal, which calls the appropriate variant of _objc_msgForward based on a register/condition.

The docs for class_getMethodImplementation() say:

The function pointer returned may be a function internal to the runtime instead of an actual method implementation. For example, if instances of the class do not respond to the selector, the function pointer returned will be part of the runtime's message forwarding machinery.

@Coneko
Copy link
Member

Coneko commented Sep 8, 2013

Sources say _objc_msgForward_internal isn't callable from C code, so we can't use that.

The function pointer returned may be a function internal to the runtime instead of an actual method implementation.

This might be referring to _objc_msgForward and _objc_msgForward_stret since they used to be private. Maybe the docs haven't been updated.

In fact you if look at the source class_getMethodImplementation has this snippet:

    // Translate forwarding function to C-callable external version
    if (imp == _objc_msgForward_internal) {
        return _objc_msgForward;
    }

@kastiglione
Copy link
Member

Yep, there be dead ends. My comment was to answer how the runtime handles the two, I had no actual good suggestions. Call-ability aside, _objc_msgForward_internal is also not public.

A really bad idea is we could make our own _objc_msgForward_internal implementations, which would rely on the internal implementation of objc_msgSend and objc_msgSend_stret, both of which set a specific register/flag that _objc_msgForward_internal relies on.

I agree, we document it unsupported, and if the need comes up, deal with it then.

@Coneko
Copy link
Member

Coneko commented Sep 8, 2013

I've found out NSMethodSignature's debugDescription shows whether the method is stret or not, but its public API doesn't expose that information.

@kastiglione
Copy link
Member

Nice find. I just dug in to see where it was getting the info from, but not surprisingly it's another dead end. NSMethodSignature has a private _frameDescriptor struct, which contains a bit field that includes a bit for _stret or not.

@jspahrsummers
Copy link
Member Author

In 732726b, I tried to hijack -forwardInvocation: for our benefit (and basically use the runtime's internal logic for determining a stret method), but this introduces a slew of other problems and is just generally messy as hell.

So yeah, this seems like a fairly lost cause, unless we want to add libffi as a dependency and use that to determine struct returns. I'm 👍 on just documenting that they're unsupported.

@kastiglione
Copy link
Member

:slow: 👏

Absolutely amazing! I thought about how could we use -forwardInvocation:, but never did I think of cloning the class' hierarchy.

@numist
Copy link
Contributor

numist commented Sep 10, 2013

Sorry I'm late to this; I went through exactly this problem for another project, and the only public way to determine if a method is special struct return is using NSMethodSignature -debugDescription.

I filed a Radar for this; it should really be public API, and NSMethodSignature's implementation determining special struct return is really hairy and not worth duplicating.

@kastiglione
Copy link
Member

Closing, resolved by #787.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants