FFI doesn't work on some arm-v7a android devices #54948
Comments
VladyslavBondarenko
added
e: device-specific
|
Seems really strange that it only affects some devices, ABI should be the same. /cc @dcharkes |
|
/cc @mkustermann |
|
I added the test, but it does not repro on the phone that I'm using to test, a Nexus 5X. It also doesn't repro on QEMU, but that uses a slightly different ABI: armabihf. And unfortunately QEMU doesn't come with Cortex A53 core simulation, which are the ones in the Snapdragon 625, in the Moto G5 Plus. @britannio It would be useful to know how exactly the FFI call fails.
|
From Wikipedia it looks like that phone has a Snapdragon S808, which according to this is 64 bit. I guess that shouldn't make a difference when running a 32-bit app but it looks like this bug is specific to some few devices. Note that I couldn't reproduce this myself, but from this comment it sounds like the test should fail on a Moto G5 Plus.
If you mean the last parameter, I was just trying to iterate over random values to reproduce this. The function was supposed to mimic I'll try to reproduce this in Firebase Test Lab. If that works I can try to move the 64 bit arg to a different position as well. |
|
@mraleph The cross compiler ( Neither does not cross compiling ( I was also trying to double check if there is any discrepancy between standalone dart (tested to We can specify a target platform when building, but not when running: Is it possible to get |
|
Firstly thank you very much for the detailed bug report! I have tried to follow the reproduction instructions from #54948 (comment) on Moto 4G / Nexus 5X without success. @simolus3 @pY4x3g @britannio Could I ask you confirm that this issue persists with newest master of flutter/flutter and tell us which device you can confirm this on? If we have confirmation, we can purchase such a device and get to the bottom of it. |
|
We don't support a targetPlatform argument to run anymore (we really should add it back). But you can simulate it by building the APK variant you want with |
Huawei Honor 8S (MediaTek Helio A22 MT6761), 64 bit Cortex-A53 |
|
@andreystavitsky Thank you! We have ordered a Honor 8S 32GB, Dual SIM, Android 9.0 (Pie), MediaTek MT6761. It will take some time until it arrives. We'll update the thread once it arrives and we can verify the problem. |
|
@andreystavitsky The good news is that our Honor 8S arrived today. The bad news is that I cannot reproduce the issue. First I prepare my flutter checkout to have a close version to what was reported: Then I build the app via: => I can launch the app without any issues on Huawei Honor 8S. When digging a little deeper it seems like the CPU in Honor 8S is capable of 64-bit but the Android OS runs in 32-bit system. The next hypothesis I had was that this was somehow a Windows specific problem, so I've tried to build the APK the same way on windows and install it but it sill succeeds. @andreystavitsky Could you make the APK you built (and crashes) publicly available? |
|
I'm using flutter run --release https://send.firefox.com/download/3daf11e2a8d15d64/#de2J7kIPiH7RfJdplzkYZQ I can try to build on Mac and then provide access to my Mac. |
|
Well, I cant reproduce this bug on Mac, only on Windows |
|
given that is a Windows only issue this might mean a bug similar to one fixed by dart-lang/sdk@164948a (maybe even the same bug) |
|
Test on latest master apk: https://send.firefox.com/download/871713d512320c32/#YPByhsmXGidTTYLQTMlnhw |
|
@mkustermann forgot to say, but APK is not crashes, it's just write some debug log to console. |
|
@andreystavitsky Thanks a lot. I can reproduce the issue now also with on my Windows (probably I did something wrong when I first tried it). Issue 1 (harmless)It seems like the flutter tools
(**) gen_snapshot is our AOT (cross)compiler. This is a bit weird and we might want to change that. Though it should be safe either way. Though they compile to slightly different code in a special case: Our assembler_arm.cc looks like this: void Assembler::ReserveAlignedFrameSpace(intptr_t frame_space) {
...
if (OS::ActivationFrameAlignment() > 1) {
bic(SP, SP, Operand(OS::ActivationFrameAlignment() - 1));
}
}This uses intptr_t OS::ActivationFrameAlignment() {
#if defined(TARGET_ARCH_ARM64)
return 16;
#elif defined(TARGET_ARCH_ARM)
return 8;
...
}and in os_macos.cc: intptr_t OS::ActivationFrameAlignment() {
#if HOST_OS_IOS // <-- This is not defined
...
#else
// OS X activation frames must be 16 byte-aligned; see "Mac OS X ABI
// Function Call Guide".
return 16;
#endif
}This means we use Though the ABI only requires 8 so we should be safe. Issue 2Without having verified it, it seems to me that the FFI implementation relies on undefined evaluation order, see compiler/ffi/native_calling_convention.cc: const NativeLocation& AllocateArgument(const NativeType& payload_type) {
if (payload_type_converted.IsFloat()) {
...
} else {
...
if (target::kWordSize == 4 && payload_type.SizeInBytes() == 8) {
...
if (cpu_regs_used + 2 <= CallingConventions::kNumArgRegs) {
return *new (zone_) NativeRegistersLocation(
payload_type, container_type,
AllocateCpuRegister(), AllocateCpuRegister());
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Evaluation order undefined
}
} else {
...
}
}
}C++ doesn't guarantee any order of evaluation for the two => I think this is the bug. |
|
I have manually confirmed that this fixes the problem. |
|
I have filed a cherry-pick request into beta/stable branches: dart-lang/sdk#42184 |
|
|
@dcharkes When approximately will be the next release, I need This but my app is on production, so i like keep using stable channel. |
|
The date of the next stable release is not defined yet, but it looks like it is targeting sometime in July. |
|
/cc @eonoe - is this the issue you're hitting? |
@pcsosinski yes we think that is related with this one. |
|
how is the situation? the 17.5 version seems to have still the problem |
|
The cherry pick request was denied, so it has not been merged into 1.17.5. So this fix will only be available in Flutter 1.18.x, and is available on Flutter master. |
ok, is it possible to have an idea of when the 1.18 will be released? weeks, months? ty |
|
any updates on this issue? |
|
As mentioned, it's available in master, and will be released in the next stable release. See Flutters release cadence on the wiki: https://github.com/flutter/flutter/wiki/Roadmap#release-channels-and-cadence. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
Problem
I maintain a package providing
sqlite3bindings withdart:ffi(calledmoor_ffi). Some users were reporting that on some devices, inserting a (64 bit) integer will write bogus values to the database (link to issue).I could reproduce this with a very minimal native library:
I'm binding to this function in Dart using
Calling
test2(Pointer.fromAddress(0xdeadbeef), 123, 1);returns1on some devices. I would expect it to return0, as it does on most devices. While it looks like this problem only happens on arm-v7a devices, it doesn't happen on every arm-v7a device, and also not on each android version.Steps to Reproduce
This requires the NDK to be installed and available.
flutter run --releaseon an affected deviceAffected devices
@pY4x3g could reproduce this on a Moto G5 Plus, running Android 8.1. Their
flutter doctoroutput wasIf the small repro has the same underlying problem as the one in
moor_ffi, devicelab failures provided by @britannio indicate that the devices listed here are affected as well.I don't know if it helps, but it seems like this does not happen on all arm-v7a devices. The only 32 bit arm device I have is an old Galaxy S3, where this does not happen.
The text was updated successfully, but these errors were encountered: