Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/mobile: gomobile's JNI crashes under android x86 with ARM translation #15986

Closed
wxiaoguang opened this issue Jun 7, 2016 · 6 comments
Closed
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile
Milestone

Comments

@wxiaoguang
Copy link

wxiaoguang commented Jun 7, 2016

  1. What version of Go are you using (go version)?
    go version go1.6.2 darwin/amd64
  2. What operating system and processor architecture are you using (go env)?
    Host: x86_64
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"

Target:
genymotion android 4.2/4.3(x86) with arm translation 1.1

  1. What did you do?

https://play.golang.org/p/zUeejSnLmO

armeabi-v7a under android x86 with arm translation, the CompareAndSwapUint64 crashes like this:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xb code=0x1 addr=0x0 pc=0x38273e98]

I only want to build armeabi-v7a JNIs to reduce my APK's size.
The crash only occurs under x86 with arm translation.
The code under armeabi-v7a and x86_64 with arm translation works correctly.

I made a very dirty hack to make the code work:

    1. clean gomobile compiled packages:  rm -rf ~/go/pkg/gomobile/pkg_*
    2. modify /usr/local/go/src/sync/atomic/asm_linux_arm.s
       comment out "__kuser_cmpxchg64"
       force arm arch to 7: MOVW     $7, R0

TEXT setupAndCallCAS64<>(SB),NOSPLIT,$-4-21
    //MOVB  runtime·armArch(SB), R0
    MOVW    $7, R0
    // LDREXD, STREXD only present on ARMv6K or higher
    CMP $6, R0
    MOVW.CS $·armCompareAndSwapUint64(SB), R1
    MOVW.CS R1, armCAS64(SB)
    MOVW.CS R1, R15

    3. re-init gomobile:  gomobile init
    4. re-compile library & app
@hyangah
Copy link
Contributor

hyangah commented Jun 7, 2016

Can you include the crash stack trace, and the output of 'gomobile bind -x -v'?

Is the problem present with go1.7 (or tip) as well?

@wxiaoguang
Copy link
Author

wxiaoguang commented Jun 7, 2016

Attached files are about the bind command and the panic stack trace.

gomobile-bind.txt.zip
gomobile-panic.txt

The crash occurred in the net module, I debugged for a while and found that it was caused by CompareAndSwapUint64.

I am pretty sure that the problem is caused by using golang's linux-kernel atomic CAS64(__kuser_cmpxchg64) under a android x86 with ARM translation.

You can refer to my "dirty solution", I just removed the code for "__kuser_cmpxchg64" in /usr/local/go/src/sync/atomic/asm_linux_arm.s, and force golang to use "armCompareAndSwapUint64", then everything works.

I will try go 1.7 later, but I don't think it works if it still uses the same code for "__kuser_cmpxchg64".

Maybe this problem is not very serious, because seldom people do what I do: unpack the aar and remove everything(x86/x86_64) else besides armeabi-v7a to keep the APK as small as possible :)

@wxiaoguang
Copy link
Author

I have some new thoughts about this problem.

Since gomobile only supports armeabi-v7a and above, can we make a special CompareAndSwapUint64 for armeabi-v7a and above? Then this special implementation can call armCompareAndSwapUint64 directly which is efficient enough. I read the source code about __kuser_cmpxchg64 and thought that it's not necessary to use it under an arm-v7a and above CPU.

@hyangah
Copy link
Contributor

hyangah commented Jun 14, 2016

cc @crawshaw

@wxiaoguang do you see the same problem when running it on real arm devices?
btw, you can configure -target flag to limit the target architecture. e.g. -target=android/arm

@wxiaoguang
Copy link
Author

Real arm devices works OK. This problem only occurs under x86(i386 only) with arm translation.

I have made a cleaner fix, just change the detection order:

/src/sync/atomic/asm_linux_arm.s

TEXT setupAndCallCAS64<>(SB),NOSPLIT,$-4-21
    //1. try to use armCAS64(LDREXD, STREXD)
    MOVB    runtime·armArch(SB), R0
    // LDREXD, STREXD only present on ARMv6K or higher
    CMP $6, R0 // TODO(minux): how to differentiate ARMv6 with ARMv6K?
    MOVW.CS $·armCompareAndSwapUint64(SB), R1
    MOVW.CS R1, armCAS64(SB)
    MOVW.CS R1, R15
    //2. try to use __kuser_cmpxchg64
    MOVW    $0xffff0ffc, R0 // __kuser_helper_version
    MOVW    (R0), R0
    // __kuser_cmpxchg64 only present if helper version >= 5
    CMP     $5, R0
    MOVW.CS $kernelCAS64<>(SB), R1
    MOVW.CS R1, armCAS64(SB)
    MOVW.CS R1, R15 // R15 = hardware PC
    //3. we are out of luck, can only use runtime's emulated 64-bit cas
    MOVW    $·generalCAS64(SB), R1
    MOVW    R1, armCAS64(SB)
    MOVW    R1, R15

@quentinmit quentinmit added this to the Unreleased milestone Jun 17, 2016
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Jul 20, 2017
@wxiaoguang
Copy link
Author

Too old, I am not using go-mobile at the moment.

@wxiaoguang wxiaoguang closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
@golang golang locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge mobile Android, iOS, and x/mobile
Projects
None yet
Development

No branches or pull requests

4 participants