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

int64 param not working for imported functions #50

Closed
noot opened this issue Jun 21, 2019 · 14 comments
Closed

int64 param not working for imported functions #50

noot opened this issue Jun 21, 2019 · 14 comments
Assignees
Labels
🐞 bug Something isn't working 📦 component-extension About the Go extension

Comments

@noot
Copy link
Contributor

noot commented Jun 21, 2019

Describe the bug

I have a wasm module that imports a function ext_print_num that takes an int64 parameter. However when I try to implement it, I get an error saying "conflicting types for ext_print_num".

Maybe this is an issue with int64? My other import functions that accept int32 all work perfectly.

Steps to reproduce

package main

// extern void ext_print_num(void *context, int64_t data);
import "C"

//export ext_print_num
func ext_print_num(context unsafe.Pointer, data int64) {
	return
}

func main() {
	bytes, err := wasm.ReadBytes("mymodule.wasm")
	if err != nil {
		return nil, err
	}
	
	imports, err := wasm.NewImports().Append("ext_print_num", ext_print_num, C.ext_print_num)
	if err != nil {
		return nil, err
	}

	instance, err := wasm.NewInstanceWithImports(bytes, imports)
	if err != nil {
		return nil, err
	}
	defer instance.Close()
}

the full code is here: https://github.com/ChainSafe/gossamer/blob/elizabeth/wasmer/runtime/wasmer.go

Expected behavior

I wouldn't expect an error since the types should match.

Actual behavior

In file included from _cgo_export.c:4:0:
cgo-gcc-export-header-prolog:58:13: error: conflicting types for ‘ext_print_num’
In file included from _cgo_export.c:4:0:
wasmer.go:9:14: note: previous declaration of ‘ext_print_num’ was here
_cgo_export.c:176:6: error: conflicting types for ‘ext_print_num’
 void ext_print_num(void* p0, GoInt64 p1)
      ^~~~~~~~~~~~~
In file included from _cgo_export.c:4:0:
wasmer.go:9:14: note: previous declaration of ‘ext_print_num’ was here
FAIL	github.com/ChainSafe/gossamer/runtime [build failed]

Additional context

Also, just wondering if you have a gitter or some other channel I can reach you on. Thanks :)

@noot noot added the 🐞 bug Something isn't working label Jun 21, 2019
@noot noot changed the title I64 param not working for imported functions int64 param not working for imported functions Jun 21, 2019
@Hywan Hywan self-assigned this Jun 21, 2019
@Hywan Hywan added the 📦 component-extension About the Go extension label Jun 21, 2019
@Hywan
Copy link
Contributor

Hywan commented Jun 24, 2019

In #51, I've added tests to cover all Wasm types combined to Wasm imported functions. i64 works as expected, so I don't understand the issue with your program.

I would recommend to debug cgo generated files with go tool cgo your_file.go, and see the _cgo_export.* files. Also check the _cgo_gotypes.go file.

By reading the message, it seems that the ext_print_num function is declared twice. It is declared in another file? Can you have cache somewhere?

@noot
Copy link
Contributor Author

noot commented Jun 24, 2019

It doesn't seem to be declared twice, I've made sure it isn't in any other files.

Looking in _cgo_export.c, I have this declaration which looks correct:

extern void _cgoexp_fb7b4fb1a470_ext_print_num(void *, int, __SIZE_TYPE__);

CGO_NO_SANITIZE_THREAD
void ext_print_num(void* p0, GoInt64 p1)
{
	__SIZE_TYPE__ _cgo_ctxt = _cgo_wait_runtime_init_done();
	struct {
		void* p0;
		GoInt64 p1;
	} __attribute__((__packed__, __gcc_struct__)) a;
	a.p0 = p0;
	a.p1 = p1;
	_cgo_tsan_release();
	crosscall2(_cgoexp_fb7b4fb1a470_ext_print_num, &a, 16, _cgo_ctxt);
	_cgo_tsan_acquire();
	_cgo_release_context(_cgo_ctxt);
}

The other declaration is in the go file. I'm not sure what's going wrong since it seems to look the same as your tests.

@Hywan
Copy link
Contributor

Hywan commented Jun 25, 2019

The error comes from cgo, not wasmer. I'm not sure how I can help you right now. Did you try to remove all the Go compiler caches?

@noot
Copy link
Contributor Author

noot commented Jun 25, 2019

@Hywan no worries, yeah I tried. I'll try to figure it out.

@noot
Copy link
Contributor Author

noot commented Jun 25, 2019

when running the tests for this repo with go test ./..., I run into a similar issue:

?   	github.com/wasmerio/go-ext-wasm/go-wasmer	[no test files]
# github.com/wasmerio/go-ext-wasm/wasmer/test
In file included from _cgo_export.c:4:0:
cgo-gcc-export-header-prolog:46:16: error: conflicting types for ‘sum_i64’
In file included from _cgo_export.c:4:0:
imports.go:6:17: note: previous declaration of ‘sum_i64’ was here
_cgo_export.c:46:9: error: conflicting types for ‘sum_i64’
 GoInt64 sum_i64(void* p0, GoInt64 p1, GoInt64 p2)
         ^~~~~~~
In file included from _cgo_export.c:4:0:
imports.go:6:17: note: previous declaration of ‘sum_i64’ was here
ok  	github.com/wasmerio/go-ext-wasm/wasmer	0.028s
FAIL	github.com/wasmerio/go-ext-wasm/wasmer/test [build failed]

Perhaps something is wrong with my environment, what does your go env look like?

@Hywan
Copy link
Contributor

Hywan commented Jun 26, 2019

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hywan/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hywan/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/m0/4f6b8rhj4gnb_29c_gz1xq0m0000gn/T/go-build398726835=/tmp/go-build -gno-record-gcc-switches -fno-common"

@Hywan
Copy link
Contributor

Hywan commented Jun 26, 2019

Do you want me to instruct the CI to print go env, so that you can compare?

@noot
Copy link
Contributor Author

noot commented Jun 26, 2019

Here's mine:

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/elizabeth/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/elizabeth/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/elizabeth/go.mod"
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-build446087408=/tmp/go-build -gno-record-gcc-switches"

Looks like the main difference is I'm using gcc and you have clang.
Sure, printing in the CI might be helpful.

Update: I changed the compiler to clang with export CC="clang" and export CXX="clang++" and still having the same issue. I got someone to try running my code on Mac OS and they didn't have this error, so it's probably Linux.

@syrusakbary
Copy link
Member

@losfair can you check why this is happening on Linux and not on Mac OS?

@noot
Copy link
Contributor Author

noot commented Jul 3, 2019

Hey, any updates? I would really like to use wasmer in one of my projects :)

@yuce
Copy link

yuce commented Jul 4, 2019

Using C.int64_t instead of int64 solves the issue (gcc 7.4, Go 1.12.6 ubuntu 18.04).

@noot
Copy link
Contributor Author

noot commented Jul 4, 2019

@yuce that resolved my issue, thanks so much!

@yuce
Copy link

yuce commented Jul 4, 2019

@noot happy to help!

@Hywan Hywan closed this as completed Jul 5, 2019
@losfair
Copy link

losfair commented Jul 14, 2019

After changing int64_t to long long in cgo declaration, the provided example code works for me without the int64 -> C.int64_t modification.

I think it's either because the cgo type checker does not handle C typedefs properly, or int64_t is not defined as long long in your compilation environment.

@Hywan Hywan mentioned this issue Aug 29, 2019
bors bot added a commit that referenced this issue Aug 30, 2019
67: Changed int64_t to long long as suggested by losfair / hywan r=Hywan a=ethanfrey

Closes #65

Based on this suggestion: #50 (comment)

Co-authored-by: Ethan Frey <ethanfrey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 component-extension About the Go extension
Projects
None yet
Development

No branches or pull requests

5 participants