Crash using NSObject's -valueForKey: IMP with collections on arm64

[nolanw]

I must apologize up front for this less than helpful report. As I don't have an arm64 device at hand, I can't personally reproduce this crash, and thus don't trust myself to fix it. I'd appreciate any help others can provide.

There seems to be an issue using NSObject's -valueForKey: implementation for collections on arm64. Here's an example crash report. It points to GRMustacheKeyAccess.m line 80. A collaborator with an arm64 device caught this crash in the debugger and found that the key path was contactInfo.count. The key contactInfo correctly returns an NSArray, so it seems it's the .count that causes something to go haywire.

All reports of this crash come from arm64 devices.

Here's a test case that may trigger the crash. Be sure to run it on an arm64 device; it is not reproducible in the simulator (either x86 or x86_64).

[GRMustacheTemplate renderObject:@{ @"things": @[ @1, @2 ] }
                      fromString:@"{{#things.count}}There's things!{{/}}"
                           error:nil];

In the linked crash report, as well as in the test case above, I was following the runtime guide's suggestion for rendering a section only when a collection is nonempty. It's easy enough to work around the problem, but that section of the guide needs updating until a solution can be found.

---

As an aside, while I haven't done much research into the matter, I suspect GRMustache's current practice, of retrieving NSObject's -valueForKey: implementation and calling it on instances of NSArray et al, is not documented to work. I'm happy to file a radar asking for clarification and/or a fix, if that would be helpful, but I'm not the ideal candidate given my lack of arm64 hardware.

[ultramiraculous]

To add a little more color, the problem isn't specific to collections. If you attempt to use GRValueForKeyNSObjectIMP on anything at all, it goes equally haywire. i.e. replacing

        return [object valueForKey:key];

with

        return GRValueForKeyNSObjectIMP(object, @selector(valueForKey:), key);

causes crashes to happen for any object when you go to access a key value.

I can also confirm that Nolan's code snippet crashes gloriously on my Retina iPad Mini running 7.1 beta 5 and my iPhone 5s running 7.0.4.

[groue]

Thanks to all for your reports. I'm on vacation right now, so I won't attack this issue before two weeks. GRMustacheKeyAccess.m has a detailed comment on how NSArray, NSSet and NSOrderedSet are handled by GRMustache. Feel free to dig deeper on why IMPs can't be used directly on ARM64. Another option to look at is objcMsgSendSuper, should IMPs be banned for any reason.

[groue]

Hi @nolanw, @ultramiraculous.

I just pushed a build of GRMustache on the issue70 branch.

In this branch, GRMustacheKeyAccess.m has been updated, as well as the static library lib/libGRMustache6-iOS.a, so that objc_msgSendSuper is used instead of NSObject's implementation of valueForKey: (see commit 1c94c24).

Can you tell me if this solves the issue (I don't have any arm64 device)?

[ultramiraculous]

Same crash, unfortunately, but good idea. I threw up a radar just now, so I'll let you know if anything comes of it. I messed around with it more, and it's really just doing anything other than [object valueForKey:] seems to make it crash. Even objc_msgSend-ing valueForKey: to a vanilla NSObject doesn't work. Kinda bizarre.

[groue]

@nolanw wrote:

I was following the runtime guide's suggestion for rendering a section only when a collection is nonempty. It's easy enough to work around the problem, but that section of the guide needs updating until a solution can be found.

Yep. Let's update the doc.

Could anyone of you push a full stack trace of the exception here, please? (Enter the bt command in LLDB, and paste the output here)

[groue]

BTW, thanks @ultramiraculous for your test. I'm quite disappointed :-)

[groue]

All right, here we go for another try, with a home-made implementation of valueForKey: (see commit 3133a3b). @ultramiraculous, I hope your device won't crash this time :-)

[groue]

@nolanw Thanks for the Crashlytics report, I didn't notice it the first time.

[ultramiraculous]

Yeah, that certainly appears to work. Wish it didn't have to come to this, but happy for the fix :)

[groue]

OK, so I'll merge this into master, and issue a new release. Thanks a lot, @ultramiraculous !!

[groue]

Hello @ultramiraculous, @nolanw. Thank you again for your much appreciated reports and help. GRMustache v6.9.2 is out, with a fix for this issue.