WeakReferences tests fail on Android ARMv7 and x86

[triplef]

Running the WeakReferences* tests on Android ARMv7 fail with the following messages. The same tests succeed on ARM64.

RUNNING WeakReferences_arc...
Insert failed [repeated 2179 times]
../Test/WeakReferences_arc.m:34: int main(int, const char **): assertion "refs[i] == nil" failed
Aborted 
FAILED: WeakReferences_arc

RUNNING WeakReferences_arc_legacy...
Insert failed [repeated 2179 times]
../Test/WeakReferences_arc.m:34: int main(int, const char **): assertion "refs[i] == nil" failed
Aborted 
FAILED: WeakReferences_arc_legacy

RUNNING WeakReferences_arc_legacy_optimised...
Insert failed [repeated 2179 times]
../Test/WeakReferences_arc.m:34: int main(int, const char **): assertion "refs[i] == nil" failed
Aborted 
FAILED: WeakReferences_arc_legacy_optimised

RUNNING WeakReferences_arc_optimised...
Insert failed [repeated 2179 times]
../Test/WeakReferences_arc.m:34: int main(int, const char **): assertion "refs[i] == nil" failed
Aborted 
FAILED: WeakReferences_arc_optimised

From the "Insert failed" message it looks like an issue with the hash table implementation. I’d appreciate any pointers to track this down further.

This is with libobjc2 compiled using the GNUstep Android toolchain, i.e. using the latest master version of libobjc2 built with clang from the Android NDK.

[davidchisnall]

The insert failed message can happen if you have too many hash collisions, such that increasing the size of the hash table does not remove collisions. We may need to change the hash function.

[davidchisnall]

I believe this is fixed by 8809f01

[triplef]

I re-run the tests with the latest changes including 8809f01, but they still fail the same way. I believe you mean that it should be fixed with #119?

[davidchisnall]

8809f01 contains a back-port of the fix from #119. Does it work with #119?

[triplef]

The WeakReferences_arc tests succeed with #119 on both armeabi-v7a and arm64-v8a.

[davidchisnall]

But not with the back-port of the fix to the master branch? Interesting, that implies another bug in the hash table implementation...

[triplef]

Nope, I double checked and the WeakReferences_arc tests still fail with the latest master branch (containing 8809f01) on armeabi-v7a.

If I understand it correctly though, 8809f01 fixed a memory corruption, whereas the WeakReferences_arc tests failed due to the hash table implementation, which was improved in #119. So I think it’s all how it should be (i.e. this issue will be resolved by #119)?

[davidchisnall]

Ah, it's failing due to hash collisions. We can probably fix it in the master branch by improving the hash function...

[jordo]

we experienced some weak ref issues with latest commit, and posted #123. Was potentially unrelated to hash table impl

[triplef]

I did some more testing and found that the test also fails on Android x86, but not on x86_64.

[davidchisnall]

Makes sense - we probably experience a lot more hash collisions on 32-bit. Is this fixed for you in the #119 branch?

[triplef]

Just tried with the arc-cxx branch... the WeakReferences_arc tests pass, but these don’t on both x86 and x86_64:

RUNNING WeakRefLoad...
../Test/WeakRefLoad.m:15: int main(int, const char **): assertion "w1 == nil" failed
Aborted 
FAILED: WeakRefLoad

RUNNING WeakRefLoad_optimised...
../Test/WeakRefLoad.m:15: int main(int, const char **): assertion "w1 == nil" failed
Aborted 
FAILED: WeakRefLoad_optimised