Skip to content
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

EXC_CRASH on arm64 only in TyphoonRuntimeArguments #212

Closed
literator opened this issue Apr 30, 2014 · 13 comments · Fixed by CocoaPods/Specs#11189
Closed

EXC_CRASH on arm64 only in TyphoonRuntimeArguments #212

literator opened this issue Apr 30, 2014 · 13 comments · Fixed by CocoaPods/Specs#11189

Comments

@literator
Copy link
Contributor

So I have following definitions in my assembly:

- (id)detailsViewControllerForProduct:(Product *)product {
    return [TyphoonDefinition withClass:[CollectionViewController class]
                          configuration:^(TyphoonDefinition *definition) {
                              definition.initializer.selector = @selector(initWithDataSource:layout:);
                              [definition.initializer injectParameterWith:[self dataSourceForProduct:product]];
                              [definition.initializer injectParameterWith:[UICollectionViewLayout listLayout]];
                          }];
}

- (id)dataSourceForProduct:(Product *)product {
    return [TyphoonDefinition withClass:[CollectionViewDataSource class]
                          configuration:^(TyphoonDefinition *definition) {
                              definition.initializer.selector = @selector(initWithManager:cellClass:);
                              [definition.initializer injectParameterWith:product];
                              [definition.initializer injectParameterWith:[TransactionViewCell class]];
                              [definition injectProperty:@selector(headerViewClass) with:[ProductDetailHeaderView class]];
                          }];
}

Note that view controller gets runtime argument and passes it to data source definition.

Now, when populating caches I'm experiencing EXC_CRASH in line 57 in TyphoonRuntimeArguments.m.

It's an issue with variadic arguments and it occurs only on arm64 devices (I have tested this on iPhones and iPads). I'm aware that va_args is handled a little bit differently on 64bit architecture but I don't have any clue now why it crashes. It's probably something with Typhoon implementation, because functions like printf or NSLog work well.

@jasperblues
Copy link
Member

Hmmm . . . I haven't used the approach you're doing below before. Let's look there first.

What happens when you change:

[definition.initializer injectParameterWith:[self dataSourceForProduct:product]];

. . to a hard-coded reference, eg

[definition.initializer injectParameterWith:[self aPreConfiguredDataSource]];

Also, are you able to give us access to the (or a sample) project?

@jasperblues
Copy link
Member

We do have a test case for that feature - propagating runtime args through to a dependency, though.

Trying now on an arm64 device.

@jasperblues
Copy link
Member

Logged #213, it turns out the way we have our tests set up, we can't run time on the device. Will convert them tomorrow. . . meantime will debug manually.

@literator
Copy link
Contributor Author

@jasperblues unfortunalety I can't share this project with you.

Regarding [self aPreConfiguredDataSource] - yes, it works, but in this scenario it doesn't call TyphoonRuntimeArguments argumentsFromVAList method where the crash occurs and var_args is used.

@jasperblues
Copy link
Member

OK, that sounds like enough information. Trying to reproduce it now. Will report back shortly.

Re arm64: We've found that simulator and 32bit can sometimes hide memory issues, so it will be important to also do #213, and test on 64bit devices.

@jasperblues
Copy link
Member

@literator , I have reproduced it.

@literator
Copy link
Contributor Author

OK, so it looks like a wider issue. Any solutions?

@jasperblues
Copy link
Member

We can surely fix it. Will get back to you shortly.

Sorry about that.

@alexgarbarev
Copy link
Contributor

Looks like a critical issue.Will try to fix now. Arm64 brings a lot of surprises for me

@literator
Copy link
Contributor Author

@alexgarbarev thanks for that.

As far as I can see it about method calling convention on arm64. While va_args expects to have all arguments in the stack, complier in 64bit mode moves some of them to registers and in result we have different object address in ImplementationToConstructDefinitionAndCatchArguments and different in argumentsFromVAList:selector.

If it was on i386 it would be fixed with regparm attribute/flag (http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html) but on arm64 I have no idea how to deal with it. Nevertheless it may be a clue. I'd love to know the solution.

@jasperblues
Copy link
Member

@literator

We use resolveInstanceMethod to do some proxying on the assembly. This approach requires creating a pointer to an IMP, with the neat va_args trick as you saw. . . . unfortunately this isn't very robust.

Fortunately NSInvocation works, so we can convert our assembly proxying to use fowardIncvocation/NSInvocation . . . this is quite a big change.

In the meantime, we can support up to 10 runtime arguments by dispatching to one of 10 function pointers for IMP. . . ugly but should work fine.

. . testing now.

@jasperblues
Copy link
Member

Temporary fix confirmed. Will push as 2.02 shortly.

@jasperblues
Copy link
Member

@literator Fix pushed as 2.0.2, on its way in to CoocaPods.

Meantime, would you please test by using:

pod 'Typhoon', :head

As the functionality has been corrected, we'll close this issue. Meanwhile, raising a new issue to address the code quality #214

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

Successfully merging a pull request may close this issue.

3 participants