x/mobile: byte slice causes application crash if after string in function parameters

[ChrisSmith]

What version of Go are you using
go version devel +c7eb966 Wed Dec 24 07:30:28 2014 +0000 darwin/amd64

What operating system and processor architecture are you using?
osx 10.9.5 cross compiled for arm on Android 5.0.1 Nexus 5

What did you do?
Tried to call a go function from java that takes a byte[] and a string. The inverse go to java call has the same issue. If the byte[] is preceded by an int, it behaves normally.

What did you expect to see?
fmt.Printf to get called

What did you see instead?
Application crash, no error messages generated.

To reproduce, modify the libhello example by adding these functions

// This ordering of the parameters works
func OkFunc(someBytes []byte, str string) {
    fmt.Printf("Got a string %s and some bytes %v!\n", str, someBytes)
}
// This ordering causes a crash
func CrashFunc(str string, someBytes []byte) {
    fmt.Printf("Got a string %s and some bytes %v!\n", str, someBytes)
}

run gobind, then back in java add this after the initialization

// create a byte array, size/content isn't important 
byte[] bytes = new byte[1024];
for(int i=0;i < bytes.length; i++){
    bytes[i] = 8; 
}

// This one works
Hi.OkFunc(bytes, "stuff");
// This one crashes
Hi.CrashFunc("stuff", bytes);

[ChrisSmith]

Running on my Android 4.3.1 device I was able to get the panic

01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ fatal error: unexpected signal during runtime execution
01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ [signal 0x7 code=0x80 addr=0x0 pc=0x5e7459b4]
01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ [ 01-01 23:15:29.409 25930:25930 E/Go       ]
    runtime stack:
01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ runtime.gothrow(0x5e8502b0, 0x2a)
01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ /usr/local/go/src/runtime/panic.go:504 +0x8c
01-01 23:15:29.409  25930-25930/com.example.hello E/Go﹕ runtime.sigpanic()

[crawshaw]

I don't have the machinery to reproduce this bug today, but here's a possibility: we are not honoring the JNI semantics for GetByteArrayElements / ReleaseByteArrayElements. In particular in seq_android.c we have:

    jbyte* b = (*env)->GetByteArrayElements(env, v, &isCopy);
    ...
    MEM_WRITE(int64_t) = (int64_t)((intptr_t)b);
    (*env)->ReleaseByteArrayElements(env, v, (jbyte*)b, 0);

At the point that ReleaseByteArrayElements is called, the pointer b is no longer valid. ART is free to stop the world, and as part of garbage collection move the memory. That would fit with the SIGBUS @ChrisSmith is seeing.

What we need to do is:

    jbyte* b = (*env)->GetByteArrayElements(env, v, &isCopy);
    // put b on a list of bytearrays maintained by the Seq instance
    MEM_WRITE(int64_t) = (int64_t)((intptr_t)b);

Then later inside Java_go_Seq_send, have a cleanup phase after the call

    Send(desc, (GoInt)code, src->buf, src->len, &dst->buf, &dst->len);
    // iterate list, call ReleaseByteArrayElements

After the Send returns, the Go code will have decoded the Seq, which includes copying the bytes out of the pointer.