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

cmd/cgo: jobject is not mapped to uintptr if building with the Android NDK #26213

Closed
eliasnaur opened this issue Jul 4, 2018 · 11 comments

Comments

@eliasnaur
Copy link
Contributor

commented Jul 4, 2018

What version of Go are you using (go version)?

go version devel +36c623046b Wed Jul 4 00:22:11 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

With tip.

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/elias/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/elias/dev/go"
GOPROXY=""
GORACE=""
GOROOT="/home/elias/dev/go-tip"
GOTMPDIR=""
GOTOOLDIR="/home/elias/dev/go-tip/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build267724991=/tmp/go-build -gno-record-gcc-switches"
VGOMODROOT=""

What did you do?

Compiled the program

package blah

/*
#include <jni.h>
*/
import "C"

var blah C.jobject = 0

with the Android NDK:

$ CC=/path/to/ndk-standalone-x86_64/bin/clang CGO_ENABLED=1 GOOS=android go build program.go

What did you expect to see?

No error.

What did you see instead?

# command-line-arguments
./program.go:8: cannot use 0 (type int) as type _Ctype_jobject in assignment

The short program is a condensed version of the gomobile internal/mobileinit/ctx_android.go file. There is no direct assignment to a jobject variables in gomobile; I found out because I was alerted to a write barrier related crash in the function SetContext that operates on jobject values.

Note that the program successfully builds with my system's JNI headers:

$ CGO_CFLAGS="-I/usr/lib/jvm/java/include/ -I/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.172-9.b11.fc28.x86_64/include/linux" CGO_ENABLED=1 go build program.go

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2018

I'm marking this release-blocker since it causes crashes in the wild.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2018

CC @randall77 who did CL 81876.

@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Here is a stack trace:

07-02 17:32:37.513 28901     0 E Go      : runtime: writebarrierptr *0x771bc8ac18 = 0x7ca
07-02 17:32:37.513 28901     0 E Go      : fatal error: bad pointer in write barrier
07-02 17:32:37.513 28901 28944 E GoLog   : runtime: writebarrierptr *0x771bc8ac18 = 0x7ca
07-02 17:32:37.519 28901     0 E Go      : 
07-02 17:32:37.519 28901     0 E Go      : runtime stack:
07-02 17:32:37.519 28901     0 E Go      : runtime.throw(0x771ac781b5, 0x1c)
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/panic.go:593 +0x58 fp=0x7fc8f384e0 sp=0x7fc8f384c0 pc=0x771a225408
07-02 17:32:37.519 28901     0 E Go      : runtime.writebarrierptr.func1()
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mbarrier.go:215 +0x90 fp=0x7fc8f38510 sp=0x7fc8f384e0 pc=0x771a24c120
07-02 17:32:37.519 28901     0 E Go      : runtime.systemstack(0x782eb51671ac0e4e)
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:229 +0x8c fp=0x7fc8f38520 sp=0x7fc8f38510 pc=0x771a24ed3c
07-02 17:32:37.519 28901     0 E Go      : runtime.mstart()
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:1175 fp=0x7fc8f38520 sp=0x7fc8f38520 pc=0x771a2298a0
07-02 17:32:37.519 28901     0 E Go      : 
07-02 17:32:37.519 28901     0 E Go      : goroutine 17 [running, locked to thread]:
07-02 17:32:37.519 28901     0 E Go      : runtime.systemstack_switch()
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:178 +0x8 fp=0x4420074e60 sp=0x4420074e50 pc=0x771a24ec98
07-02 17:32:37.519 28901     0 E Go      : runtime.writebarrierptr(0x771bc8ac18, 0x7ca)
07-02 17:32:37.519 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mbarrier.go:213 +0x7c fp=0x4420074e90 sp=0x4420074e60 pc=0x771a20cafc
07-02 17:32:37.520 28901     0 E Go      : golang.org/x/mobile/internal/mobileinit.SetCurrentContext(...)
07-02 17:32:37.520 28901     0 E Go      : 	/Users/administrator/go/src/golang.org/x/mobile/internal/mobileinit/ctx_android.go:83
07-02 17:32:37.520 28901     0 E Go      : golang.org/x/mobile/bind/java.setContext(...)
07-02 17:32:37.520 28901     0 E Go      : 	/Users/administrator/go/src/golang.org/x/mobile/bind/java/context_android.go:20
07-02 17:32:37.520 28901     0 E Go      : golang.org/x/mobile/bind/java._cgoexpwrap_720489891265_setContext(0x7748490080, 0x7ca)
07-02 17:32:37.520 28901     0 E Go      : 	_cgo_gotypes.go:61 +0x70 fp=0x4420074eb0 sp=0x4420074e90 pc=0x771ab9b960
07-02 17:32:37.520 28901     0 E Go      : runtime.call32(0x0, 0x7fc8f38578, 0x7fc8f38660)
07-02 17:32:37.520 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:411 +0x6c fp=0x4420074ee0 sp=0x4420074eb0 pc=0x771a24f10c
07-02 17:32:37.520 28901     0 E Go      : runtime.cgocallbackg1(0x0)
07-02 17:32:37.520 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/cgocall.go:316 +0x140 fp=0x4420074f60 sp=0x4420074ee0 pc=0x771a1fd880
07-02 17:32:37.520 28901     0 E Go      : runtime.cgocallbackg(0x0)
07-02 17:32:37.520 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/cgocall.go:194 +0x98 fp=0x4420074fc0 sp=0x4420074f60 pc=0x771a1fd6c8
07-02 17:32:37.520 28901     0 E Go      : runtime.cgocallback_gofunc(0x0, 0x0, 0x0, 0x0)
07-02 17:32:37.520 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:642 +0x90 fp=0x4420074fe0 sp=0x4420074fc0 pc=0x771a250ec0
07-02 17:32:37.520 28901     0 E Go      : runtime.goexit()
07-02 17:32:37.520 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:1037 +0x4 fp=0x4420074fe0 sp=0x4420074fe0 pc=0x771a251354
07-02 17:32:37.520 28901     0 E Go      : 
07-02 17:32:37.520 28901     0 E Go      : goroutine 2 [force gc (idle)]:
07-02 17:32:37.521 28901     0 E Go      : runtime.gopark(0x771b388050, 0x771bc68160, 0x771ac6a6a6, 0xf, 0x771b387f14, 0x1)
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:291 +0xe8 fp=0x4420062760 sp=0x4420062730 pc=0x771a226f78
07-02 17:32:37.521 28901     0 E Go      : runtime.goparkunlock(0x771bc68160, 0x771ac6a6a6, 0xf, 0x14, 0x1)
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:297 +0x4c fp=0x44200627a0 sp=0x4420062760 pc=0x771a22705c
07-02 17:32:37.521 28901     0 E Go      : runtime.forcegchelper()
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:248 +0xb8 fp=0x44200627d0 sp=0x44200627a0 pc=0x771a226df8
07-02 17:32:37.521 28901     0 E Go      : runtime.goexit()
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:1037 +0x4 fp=0x44200627d0 sp=0x44200627d0 pc=0x771a251354
07-02 17:32:37.521 28901     0 E Go      : created by runtime.init.3
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:237 +0x28
07-02 17:32:37.521 28901     0 E Go      : 
07-02 17:32:37.521 28901     0 E Go      : goroutine 3 [GC sweep wait]:
07-02 17:32:37.521 28901     0 E Go      : runtime.gopark(0x771b388050, 0x771bc68320, 0x771ac6837d, 0xd, 0x771a24cb14, 0x1)
07-02 17:32:37.521 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:291 +0xe8 fp=0x4420062f60 sp=0x4420062f30 pc=0x771a226f78
07-02 17:32:37.522 28901     0 E Go      : runtime.goparkunlock(0x771bc68320, 0x771ac6837d, 0xd, 0x14, 0x1)
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:297 +0x4c fp=0x4420062fa0 sp=0x4420062f60 pc=0x771a22705c
07-02 17:32:37.522 28901     0 E Go      : runtime.bgsweep(0x4420090000)
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mgcsweep.go:71 +0x148 fp=0x4420062fd0 sp=0x4420062fa0 pc=0x771a219578
07-02 17:32:37.522 28901     0 E Go      : runtime.goexit()
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:1037 +0x4 fp=0x4420062fd0 sp=0x4420062fd0 pc=0x771a251354
07-02 17:32:37.522 28901     0 E Go      : created by runtime.gcenable
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mgc.go:216 +0x4c
07-02 17:32:37.522 28901     0 E Go      : 
07-02 17:32:37.522 28901     0 E Go      : goroutine 4 [finalizer wait]:
07-02 17:32:37.522 28901     0 E Go      : runtime.gopark(0x771b388050, 0x771bc8af50, 0x771ac696ed, 0xe, 0x14, 0x1)
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:291 +0xe8 fp=0x44200636f0 sp=0x44200636c0 pc=0x771a226f78
07-02 17:32:37.522 28901     0 E Go      : runtime.goparkunlock(0x771bc8af50, 0x771ac696ed, 0xe, 0x14, 0x1)
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/proc.go:297 +0x4c fp=0x4420063730 sp=0x44200636f0 pc=0x771a22705c
07-02 17:32:37.522 28901     0 E Go      : runtime.runfinq()
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mfinal.go:175 +0xa8 fp=0x44200637d0 sp=0x4420063730 pc=0x771a210a58
07-02 17:32:37.522 28901     0 E Go      : runtime.goexit()
07-02 17:32:37.522 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:1037 +0x4 fp=0x44200637d0 sp=0x44200637d0 pc=0x771a251354
07-02 17:32:37.522 28901     0 E Go      : created by runtime.createfing
07-02 17:32:37.523 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/mfinal.go:156 +0x64
07-02 17:32:37.523 28901     0 E Go      : 
07-02 17:32:37.523 28901     0 E Go      : goroutine 5 [runnable]:
07-02 17:32:37.523 28901     0 E Go      : runtime.cgocall(0x771abc59dc, 0x4420063e48, 0x29)
07-02 17:32:37.523 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/cgocall.go:128 +0x4c fp=0x4420063e10 sp=0x4420063de0 pc=0x771a1fd59c
07-02 17:32:37.523 28901     0 E Go      : golang.org/x/mobile/internal/mobileinit._Cfunc_free(0x773c80e4e0)
07-02 17:32:37.523 28901     0 E Go      : 	_cgo_gotypes.go:121 +0x38 fp=0x4420063e40 sp=0x4420063e10 pc=0x771a2c9358
07-02 17:32:37.523 28901     0 E Go      : golang.org/x/mobile/internal/mobileinit.lineLog.func1(0x773c80e4e0)
07-02 17:32:37.523 28901     0 E Go      : 	/Users/administrator/go/src/golang.org/x/mobile/internal/mobileinit/mobileinit_android.go:62 +0x44 fp=0x4420063e70 sp=0x4420063e40 pc=0x771a2c9944
07-02 17:32:37.523 28901     0 E Go      : golang.org/x/mobile/internal/mobileinit.lineLog(0x442000e028, 0x6)
07-02 17:32:37.523 28901     0 E Go      : 	/Users/administrator/go/src/golang.org/x/mobile/internal/mobileinit/mobileinit_android.go:62 +0x108 fp=0x4420063fc0 sp=0x4420063e70 pc=0x771a2c95c8
07-02 17:32:37.523 28901     0 E Go      : runtime.goexit()
07-02 17:32:37.523 28901     0 E Go      : 	/usr/local/Cellar/go/HEAD-896d263/libexec/src/runtime/asm_arm64.s:1037 +0x4 fp=0x4420063fc0 sp=0x4420063fc0 pc=0x771a251354
07-02 17:32:37.523 28901     0 E Go      : created by golang.org/x/mobile/internal/mobileinit.init.0
07-02 17:32:37.523 28901     0 E Go      : 	/Users/administrator/go/src/golang.org/x/mobile/internal/mobileinit/mobileinit_android.go:82 +0x100
@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

This is how jobject is defined in Android's jni.h:

#ifdef __cplusplus

class _jobject {};
typedef _jobject*       jobject;

#else /* not __cplusplus */

typedef void*           jobject;

#endif
@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

steeve added a commit to znly/go that referenced this issue Jul 4, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
	#ifdef __cplusplus
	class _jobject {};
	typedef _jobject*       jobject;
	#else /* not __cplusplus */
	typedef void*           jobject;
	#endif

This makes the jobject to uintptr check fail because it expected the
following:
	struct _jobject;
	typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>
steeve added a commit to znly/go that referenced this issue Jul 4, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
	#ifdef __cplusplus
	class _jobject {};
	typedef _jobject*       jobject;
	#else /* not __cplusplus */
	typedef void*           jobject;
	#endif

This makes the jobject to uintptr check fail because it expects the
following definition:
	struct _jobject;
	typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>
@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

We have a fix that we'll upstream today or tomorrow if you guys want: znly@6319b41

That said, I fail to understand why test22906 did not catch this. My guess is probably differences in the NDK environment.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2018

I think test22906 uses its own definition of JNI types to avoid depending on jni.h.

@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

Ah you're right.

@eliasnaur

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2018

This is a candidate for back-porting as well.

steeve added a commit to znly/go that referenced this issue Jul 4, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213
@steeve

This comment has been minimized.

Copy link
Contributor

commented Jul 4, 2018

After fighting with the auth on Gerrit, I bailed and submitted a PR at #26221

@gopherbot

This comment has been minimized.

Copy link

commented Jul 4, 2018

Change https://golang.org/cl/122217 mentions this issue: cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs

steeve added a commit to znly/go that referenced this issue Jul 11, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>
steeve added a commit to znly/go that referenced this issue Jul 11, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>
steeve added a commit to znly/go that referenced this issue Jul 11, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jul 11, 2018

steeve added a commit to znly/go that referenced this issue Jul 12, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>
steeve added a commit to znly/go that referenced this issue Jul 17, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Signed-off-by: Steeve Morin <steeve.morin@gmail.com>

@gopherbot gopherbot closed this in 5419e7a Jul 17, 2018

steeve added a commit to znly/go that referenced this issue Aug 28, 2018
cmd/cgo: update JNI's jobject to uintptr check for newer Android NDKs
In Android's NDK16, jobject is now declared as:
    #ifdef __cplusplus
    class _jobject {};
    typedef _jobject*       jobject;
    #else /* not __cplusplus */
    typedef void*           jobject;
    #endif

This makes the jobject to uintptr check fail because it expects the
following definition:
    struct _jobject;
    typedef struct _jobject *jobject;

Update the type check to handle that new type definition in both C and
C++ modes.

Fixes golang#26213

Change-Id: Ic36d4a5176526998d2d5e4e404f8943961141f7a
GitHub-Last-Rev: 42037c3
GitHub-Pull-Request: golang#26221
Reviewed-on: https://go-review.googlesource.com/122217
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

@golang golang locked and limited conversation to collaborators Jul 17, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.