Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Crash calling -expressionValueWithObject:context: on vararg expression functions #11541

Open
1ec5 opened this issue Mar 27, 2018 · 2 comments
Open
Labels
crash gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling

Comments

@1ec5
Copy link
Contributor

1ec5 commented Mar 27, 2018

When initializing an NSExpression, passing four or more arguments to a custom vararg function causes a crash due to an uncaught exception in NSExpression’s implementation. This crash reproduces when using mgl_match: (#11464), mgl_case: (#11278), or any of the aftermarket control flow functions being added in #11472. For example:

NSExpression *expression = [NSExpression expressionWithFormat:@"FUNCTION(num = 1, 'mgl_case:', 1, num = 2, 2, 3)"]
[expression expressionValueWithObject:nil context:nil];
-[MGLExpressionTests testGenericExpressionObject] : failed: caught "NSInvalidArgumentException", "-[NSInvocation setArgument:atIndex:]: index (3) out of bounds [-1, 2]"
/Users/mxn/hub/mapbox-gl-native-3/platform/darwin/test/MGLExpressionTests.mm:676: error: -[MGLExpressionTests testGenericExpressionObject] : failed: caught "NSInvalidArgumentException", "-[NSInvocation setArgument:atIndex:]: index (3) out of bounds [-1, 2]"
(
	0   CoreFoundation                      0x00007fff459d6fcb __exceptionPreprocess + 171
	1   libobjc.A.dylib                     0x00007fff6c678c76 objc_exception_throw + 48
	2   CoreFoundation                      0x00007fff4594e0a4 -[NSInvocation setArgument:atIndex:] + 452
	3   Foundation                          0x00007fff47a40140 -[NSFunctionExpression expressionValueWithObject:context:] + 1147
	4   test                                0x00000001074ce0cf -[MGLExpressionTests testGenericExpressionObject] + 3599
	5   CoreFoundation                      0x00007fff4594e9dc __invoking___ + 140
	6   CoreFoundation                      0x00007fff4594e8b0 -[NSInvocation invoke] + 320
	7   XCTest                              0x0000000100361b36 __24-[XCTestCase invokeTest]_block_invoke.272 + 50
	8   XCTest                              0x00000001003b1416 -[XCTMemoryChecker _assertInvalidObjectsDeallocatedAfterScope:] + 37
	9   XCTest                              0x00000001003618cc __24-[XCTestCase invokeTest]_block_invoke + 722
	10  XCTest                              0x00000001003aa897 -[XCUITestContext performInScope:] + 183
	11  XCTest                              0x00000001003615ef -[XCTestCase invokeTest] + 141
	12  XCTest                              0x000000010036272d __26-[XCTestCase performTest:]_block_invoke.379 + 42
	13  XCTest                              0x00000001003ae246 +[XCTContext runInContextForTestCase:block:] + 163
	14  XCTest                              0x00000001003620c9 -[XCTestCase performTest:] + 608
	15  XCTest                              0x000000010035df8e __27-[XCTestSuite performTest:]_block_invoke + 363
	16  XCTest                              0x000000010035d8f5 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 26
	17  XCTest                              0x000000010035daf2 -[XCTestSuite performTest:] + 239
	18  XCTest                              0x000000010035df8e __27-[XCTestSuite performTest:]_block_invoke + 363
	19  XCTest                              0x000000010035d8f5 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 26
	20  XCTest                              0x000000010035daf2 -[XCTestSuite performTest:] + 239
	21  XCTest                              0x000000010035df8e __27-[XCTestSuite performTest:]_block_invoke + 363
	22  XCTest                              0x000000010035d8f5 -[XCTestSuite _performProtectedSectionForTest:testSection:] + 26
	23  XCTest                              0x000000010035daf2 -[XCTestSuite performTest:] + 239
	24  XCTest                              0x00000001003bd0e2 __44-[XCTTestRunSession runTestsAndReturnError:]_block_invoke + 40
	25  XCTest                              0x0000000100378dca -[XCTestObservationCenter _observeTestExecutionForBlock:] + 477
	26  XCTest                              0x00000001003bcf80 -[XCTTestRunSession runTestsAndReturnError:] + 281
	27  XCTest                              0x000000010034daf9 -[XCTestDriver runTestsAndReturnError:] + 314
	28  XCTest                              0x00000001003ad586 _XCTestMain + 833
	29  xctest                              0x00000001000023d2 xctest + 9170
	30  libdyld.dylib                       0x00007fff6d268115 start + 1
)

Apparently NSExpression assumes a fixed size for varargs when building an NSInvocation. This Stack Overflow post suggests that we should pair any vararg method with a va_list method. Perhaps NSExpression somehow knows to look for that method when building the invocation; not sure.

This is a mostly academic point. The conversion to JSON objects in +[NSExpression mgl_expressionWithJSONObject:] works just fine, and underlying Objective-C implementations of these vararg methods all raise a “not implemented” exception anyways. But it would be a better user experience to see that exception than the one about out-of-bounds access.

/cc @fabian-guerra @anandthakker

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS crash runtime styling labels Mar 27, 2018
@stale
Copy link

stale bot commented Oct 25, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the archived Archived because of inactivity label Oct 25, 2018
@1ec5 1ec5 removed the archived Archived because of inactivity label Oct 25, 2018
@stale stale bot added the archived Archived because of inactivity label Apr 23, 2019
@stale
Copy link

stale bot commented Apr 23, 2019

This issue has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this as completed Apr 23, 2019
@julianrex julianrex reopened this Apr 23, 2019
@stale stale bot removed the archived Archived because of inactivity label Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
crash gl-ios iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS runtime styling
Projects
None yet
Development

No branches or pull requests

3 participants