32-bit Support for _AdjustLaunchApp(...)

[alexeSGN]

The implementation of _AdjustLaunchApp specifies longs, which are 32 bit on older iOS devices, whereas Unity longs are converted to Int64s via il2cpp. This was causing string mangling and invalid values for long info1 and the other values after that in this method (NaN for delayStart) from 4.12.

We believe these should be long longs or int64_t in order to support older 32-bit devices correctly, while being correct for 64 bit as per https://developer.apple.com/library/content/documentation/Darwin/Conceptual/64bitPorting/MakingCode64-BitClean/MakingCode64-BitClean.html.

[uerceg]

@alexeSGN

Thanks for reporting. We will test long longs and make an update if everything seems fine.

Are you saying that usage of long data type in the parameter list caused weird behaviour with strings (const char*s)? Can you elaborate on the reason behind it?

Also, are you able right now to reproduce problem where longs are not reaching iOS bridge method with proper values and if yes, with which device/OS version are you managing to achieve that?

[uerceg]

Also, are you seeing this issue happening with latest 4.12.2 version? If yes and if you can reproduce it, does simple switch of long parameters inside of _AdjustLaunchApp to int64_t type solve the issue?

[alexeSGN]

We were able to repro on our 5S with the original 4.12 version code. The long data is set to -1 by default, so the binary representation of this is 0xffffffffffffffff, so all the longs themselves LOOK correct when only considering 32 bits (0xffffffff), however a double does not support 0xffffffffffffffff, treating that as >infinity. We also saw the strings pointed to garbage after the conversion to _AdjustLaunchApp, but looked fine in our Bulk_Assembly call.

We are able to run with long longs (current release), and int64_t builds for us using the base 4.12 method. I will look into updating to 4.12.2 once I am finished tracking down an Android crash.

[uerceg]

Thanks for the explanation. I am curious to see if you are seeing issues with 4.12.2 as well. Also another question - you said you have noticed that const char* pointers pointed to junk - did you make any custom fix on your side for that and if yes, what did you do?

[alexeSGN]

Once you fix the 32 -> 64 bit size of the Unity longs, the PInvoke maps the strings correctly, so our change fixed what other users reported as #119 and #118. We didn't need to reorder the strings as your 4.12.2 changes did. Putting the strings first fixes those crashes, but obfuscates the fact that the longs are overrunning through each other and mangling delayStart (NaN) and the other is[...]CallbackImplemented parameters.

[uerceg]

Awesome information, thank you so much. So to recap - switching our data types in AdjustUnity.mm from long (32-bit in Obj-C) to int64_t in order to accommodate Unity long values (64-bit) would solve PInvoke mapping issues and pretty much issues that you and guys from 118/119 issues noticed? You mentioned callback implemented parameters, so conversion of all ints to int64_t should be done as well, right?

[uerceg]

And also, Android crash you mentioned you are solving - is it related to our plugin and if yes, can you share some more detail on that?

[alexeSGN]

The Unity longs are il2cpped to int64_t. You just need to change the longs; your is[...]CallbackImplemented parameters are mapped to int32_t in il2cpp, and standard ints on 32 & 64 bit C methods for iOS should be 4 bytes either way, so those are fine as is.

The Android crash may be related, but I will follow up once we track down whether it's a build step failure on our end, or an sdk issue.

[uerceg]

👍Thanks and looking forward to hear from you on Android issue. Update with these suggested changes will be live as soon as possible. Will still wait for your Android crash investigation to see if there's anything else that maybe needs to be changed.

[alexeSGN]

We are not able to reproduce with 4.12.2 for now, so these Android crashes were most likely a local build issue.

[uerceg]

Great. I will prepare PR and ping you in here once ready to check the proposed changes and see if there’s anything to be added in addition to them.

[uerceg]

@alexeSGN

v4.12.3 is now live: https://github.com/adjust/unity_sdk/releases/tag/v4.12.3 and should solve this issue. Feel free to reopen the issue if you still encounter the issue.

Thank you one more for reporting and for your help on this issue.

Cheers.