From a9c75ecd3da2d87ce08b2e75bd4f332185cd7fc8 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 2 Oct 2020 15:48:50 -0700 Subject: cmd/compile: export notinheap annotation to object file In the rare case when a cgo type makes it into an object file, we need the go:notinheap annotation to go with it. Fixes #41761 Change-Id: I541500cb1a03de954881aef659f96fc0b7738848 Reviewed-on: https://go-review.googlesource.com/c/go/+/259297 Trust: Keith Randall Run-TryBot: Keith Randall TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- misc/cgo/test/testdata/issue41761.go | 20 ++++++++++++++++++++ misc/cgo/test/testdata/issue41761a/a.go | 14 ++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 misc/cgo/test/testdata/issue41761.go create mode 100644 misc/cgo/test/testdata/issue41761a/a.go (limited to 'misc/cgo') diff --git a/misc/cgo/test/testdata/issue41761.go b/misc/cgo/test/testdata/issue41761.go new file mode 100644 index 0000000000..919c749251 --- /dev/null +++ b/misc/cgo/test/testdata/issue41761.go @@ -0,0 +1,20 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package cgotest + +/* + typedef struct S S; +*/ +import "C" + +import ( + "cgotest/issue41761a" + "testing" +) + +func test41761(t *testing.T) { + var x issue41761a.T + _ = (*C.struct_S)(x.X) +} diff --git a/misc/cgo/test/testdata/issue41761a/a.go b/misc/cgo/test/testdata/issue41761a/a.go new file mode 100644 index 0000000000..ca5c18191e --- /dev/null +++ b/misc/cgo/test/testdata/issue41761a/a.go @@ -0,0 +1,14 @@ +// Copyright 2020 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package issue41761a + +/* + typedef struct S S; +*/ +import "C" + +type T struct { + X *C.S +} -- cgit v1.2.1 From 28e549dec3954b36d0c83442be913d8709d7e5ae Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Sat, 12 Sep 2020 12:33:24 -0400 Subject: runtime: use sigaltstack on macOS/ARM64 Currently we don't use sigaltstack on darwin/arm64, as is not supported on iOS. However, it is supported on macOS. Use it. (iOS remains unchanged.) Change-Id: Icc154c5e2edf2dbdc8ca68741ad9157fc15a72ee Reviewed-on: https://go-review.googlesource.com/c/go/+/256917 Trust: Cherry Zhang Reviewed-by: Ian Lance Taylor --- misc/cgo/test/sigaltstack.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'misc/cgo') diff --git a/misc/cgo/test/sigaltstack.go b/misc/cgo/test/sigaltstack.go index 27b753a147..034cc4b371 100644 --- a/misc/cgo/test/sigaltstack.go +++ b/misc/cgo/test/sigaltstack.go @@ -62,7 +62,7 @@ import ( func testSigaltstack(t *testing.T) { switch { - case runtime.GOOS == "solaris", runtime.GOOS == "illumos", (runtime.GOOS == "darwin" || runtime.GOOS == "ios") && runtime.GOARCH == "arm64": + case runtime.GOOS == "solaris", runtime.GOOS == "illumos", runtime.GOOS == "ios" && runtime.GOARCH == "arm64": t.Skipf("switching signal stack not implemented on %s/%s", runtime.GOOS, runtime.GOARCH) } -- cgit v1.2.1 From db428ad7b61ed757671162054252b4326045e96c Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Thu, 17 Sep 2020 15:02:26 -0400 Subject: all: enable more tests on macOS/ARM64 Updates #38485. Change-Id: Iac96f5ffe88521fcb11eab306d0df6463bdce046 Reviewed-on: https://go-review.googlesource.com/c/go/+/256920 Trust: Cherry Zhang Reviewed-by: Dmitri Shuralyov Reviewed-by: Ian Lance Taylor --- misc/cgo/testcarchive/carchive_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'misc/cgo') diff --git a/misc/cgo/testcarchive/carchive_test.go b/misc/cgo/testcarchive/carchive_test.go index 2e223ea369..6ed25d8948 100644 --- a/misc/cgo/testcarchive/carchive_test.go +++ b/misc/cgo/testcarchive/carchive_test.go @@ -603,7 +603,7 @@ func TestExtar(t *testing.T) { if runtime.Compiler == "gccgo" { t.Skip("skipping -extar test when using gccgo") } - if (runtime.GOOS == "darwin" || runtime.GOOS == "ios") && runtime.GOARCH == "arm64" { + if runtime.GOOS == "ios" { t.Skip("shell scripts are not executable on iOS hosts") } -- cgit v1.2.1 From f46a5b1e4559191363dbd4f510105dd31ae97aaa Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Sun, 11 Oct 2020 15:48:22 -0400 Subject: cmd/link: support PIE internal linking on darwin/amd64 This CL adds support of PIE internal linking on darwin/amd64. This is also preparation for supporting internal linking on darwin/arm64 (macOS), which requires PIE for everything. Updates #38485. Change-Id: I2ed58583dcc102f5e0521982491fc7ba6f2754ed Reviewed-on: https://go-review.googlesource.com/c/go/+/261642 Trust: Cherry Zhang Reviewed-by: Than McIntosh --- misc/cgo/test/issue4029.c | 1 + misc/cgo/test/issue4029.go | 4 ++++ misc/cgo/test/issue4029w.go | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) (limited to 'misc/cgo') diff --git a/misc/cgo/test/issue4029.c b/misc/cgo/test/issue4029.c index 30646ade02..e6a777fe64 100644 --- a/misc/cgo/test/issue4029.c +++ b/misc/cgo/test/issue4029.c @@ -3,6 +3,7 @@ // license that can be found in the LICENSE file. // +build !windows,!static +// +build !darwin !internal_pie #include #include diff --git a/misc/cgo/test/issue4029.go b/misc/cgo/test/issue4029.go index 1bf029d760..8602ce19e2 100644 --- a/misc/cgo/test/issue4029.go +++ b/misc/cgo/test/issue4029.go @@ -3,6 +3,10 @@ // license that can be found in the LICENSE file. // +build !windows,!static +// +build !darwin !internal_pie + +// Excluded in darwin internal linking PIE mode, as dynamic export is not +// supported. package cgotest diff --git a/misc/cgo/test/issue4029w.go b/misc/cgo/test/issue4029w.go index eee33f7010..de0cf2138a 100644 --- a/misc/cgo/test/issue4029w.go +++ b/misc/cgo/test/issue4029w.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build windows static +// +build windows static darwin,internal_pie package cgotest -- cgit v1.2.1 From 627959eb04ee0edc4a985a7526ed7fe838ad2573 Mon Sep 17 00:00:00 2001 From: Cherry Zhang Date: Wed, 14 Oct 2020 21:15:37 -0400 Subject: cmd/link: support cgo internal/linking on darwin/arm64 Cgo programs work as well. Still not enabled by default for now. Enable internal linking tests. Updates #38485. Change-Id: I8324a5c263fba221eb4e67d71207ca84fa241e6c Reviewed-on: https://go-review.googlesource.com/c/go/+/263637 Trust: Cherry Zhang Run-TryBot: Cherry Zhang TryBot-Result: Go Bot Reviewed-by: Than McIntosh --- misc/cgo/test/issue4029.c | 2 +- misc/cgo/test/issue4029.go | 3 ++- misc/cgo/test/issue4029w.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) (limited to 'misc/cgo') diff --git a/misc/cgo/test/issue4029.c b/misc/cgo/test/issue4029.c index e6a777fe64..e79c5a709c 100644 --- a/misc/cgo/test/issue4029.c +++ b/misc/cgo/test/issue4029.c @@ -3,7 +3,7 @@ // license that can be found in the LICENSE file. // +build !windows,!static -// +build !darwin !internal_pie +// +build !darwin !internal_pie,!arm64 #include #include diff --git a/misc/cgo/test/issue4029.go b/misc/cgo/test/issue4029.go index 8602ce19e2..b2d131833a 100644 --- a/misc/cgo/test/issue4029.go +++ b/misc/cgo/test/issue4029.go @@ -3,10 +3,11 @@ // license that can be found in the LICENSE file. // +build !windows,!static -// +build !darwin !internal_pie +// +build !darwin !internal_pie,!arm64 // Excluded in darwin internal linking PIE mode, as dynamic export is not // supported. +// Excluded in internal linking mode on darwin/arm64, as it is always PIE. package cgotest diff --git a/misc/cgo/test/issue4029w.go b/misc/cgo/test/issue4029w.go index de0cf2138a..b969bdd0fe 100644 --- a/misc/cgo/test/issue4029w.go +++ b/misc/cgo/test/issue4029w.go @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -// +build windows static darwin,internal_pie +// +build windows static darwin,internal_pie darwin,arm64 package cgotest -- cgit v1.2.1 From 6f7b553c82b69b47becbe36d9115971d30fdab48 Mon Sep 17 00:00:00 2001 From: Quim Muntal Date: Thu, 15 Oct 2020 23:12:49 +0200 Subject: cmd/cgo: avoid exporting all symbols on windows buildmode=c-shared Disable default symbol auto-export behaviour by marking exported function with the __declspec(dllexport) attribute. Old behaviour can still be used by setting -extldflags=-Wl,--export-all-symbols. See https://sourceware.org/binutils/docs/ld/WIN32.html for more info. This change cuts 50kb of a "hello world" dll. Updates #6853 Fixes #30674 Change-Id: I9c7fb09c677cc760f24d0f7d199740ae73981413 Reviewed-on: https://go-review.googlesource.com/c/go/+/262797 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Reviewed-by: Alex Brainman Trust: Alex Brainman --- misc/cgo/testcshared/cshared_test.go | 96 ++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) (limited to 'misc/cgo') diff --git a/misc/cgo/testcshared/cshared_test.go b/misc/cgo/testcshared/cshared_test.go index d557f34b0f..e1835afa51 100644 --- a/misc/cgo/testcshared/cshared_test.go +++ b/misc/cgo/testcshared/cshared_test.go @@ -7,6 +7,8 @@ package cshared_test import ( "bytes" "debug/elf" + "debug/pe" + "encoding/binary" "flag" "fmt" "io/ioutil" @@ -355,6 +357,100 @@ func TestExportedSymbols(t *testing.T) { } } +func checkNumberOfExportedFunctionsWindows(t *testing.T, exportAllSymbols bool) { + const prog = ` +package main + +import "C" + +//export GoFunc +func GoFunc() { + println(42) +} + +//export GoFunc2 +func GoFunc2() { + println(24) +} + +func main() { +} +` + + tmpdir := t.TempDir() + + srcfile := filepath.Join(tmpdir, "test.go") + objfile := filepath.Join(tmpdir, "test.dll") + if err := ioutil.WriteFile(srcfile, []byte(prog), 0666); err != nil { + t.Fatal(err) + } + argv := []string{"build", "-buildmode=c-shared"} + if exportAllSymbols { + argv = append(argv, "-ldflags", "-extldflags=-Wl,--export-all-symbols") + } + argv = append(argv, "-o", objfile, srcfile) + out, err := exec.Command("go", argv...).CombinedOutput() + if err != nil { + t.Fatalf("build failure: %s\n%s\n", err, string(out)) + } + + f, err := pe.Open(objfile) + if err != nil { + t.Fatalf("pe.Open failed: %v", err) + } + defer f.Close() + section := f.Section(".edata") + if section == nil { + t.Error(".edata section is not present") + } + + // TODO: deduplicate this struct from cmd/link/internal/ld/pe.go + type IMAGE_EXPORT_DIRECTORY struct { + _ [2]uint32 + _ [2]uint16 + _ [2]uint32 + NumberOfFunctions uint32 + NumberOfNames uint32 + _ [3]uint32 + } + var e IMAGE_EXPORT_DIRECTORY + if err := binary.Read(section.Open(), binary.LittleEndian, &e); err != nil { + t.Fatalf("binary.Read failed: %v", err) + } + + expectedNumber := uint32(2) + + if exportAllSymbols { + if e.NumberOfFunctions <= expectedNumber { + t.Fatalf("missing exported functions: %v", e.NumberOfFunctions) + } + if e.NumberOfNames <= expectedNumber { + t.Fatalf("missing exported names: %v", e.NumberOfNames) + } + } else { + if e.NumberOfFunctions != expectedNumber { + t.Fatalf("too many exported functions: %v", e.NumberOfFunctions) + } + if e.NumberOfNames != expectedNumber { + t.Fatalf("too many exported names: %v", e.NumberOfNames) + } + } +} + +func TestNumberOfExportedFunctions(t *testing.T) { + if GOOS != "windows" { + t.Skip("skipping windows only test") + } + t.Parallel() + + t.Run("OnlyExported", func(t *testing.T) { + checkNumberOfExportedFunctionsWindows(t, false) + }) + t.Run("All", func(t *testing.T) { + checkNumberOfExportedFunctionsWindows(t, true) + }) +} + // test1: shared library can be dynamically loaded and exported symbols are accessible. func TestExportedSymbolsWithDynamicLoad(t *testing.T) { t.Parallel() -- cgit v1.2.1 From d1b1145cace8b968307f9311ff611e4bb810710c Mon Sep 17 00:00:00 2001 From: "Andrew G. Morgan" Date: Mon, 9 Dec 2019 21:50:16 -0800 Subject: syscall: support POSIX semantics for Linux syscalls This change adds two new methods for invoking system calls under Linux: syscall.AllThreadsSyscall() and syscall.AllThreadsSyscall6(). These system call wrappers ensure that all OSThreads mirror a common system call. The wrappers serialize execution of the runtime to ensure no race conditions where any Go code observes a non-atomic OS state change. As such, the syscalls have higher runtime overhead than regular system calls, and only need to be used where such thread (or 'm' in the parlance of the runtime sources) consistency is required. The new support is used to enable these functions under Linux: syscall.Setegid(), syscall.Seteuid(), syscall.Setgroups(), syscall.Setgid(), syscall.Setregid(), syscall.Setreuid(), syscall.Setresgid(), syscall.Setresuid() and syscall.Setuid(). They work identically to their glibc counterparts. Extensive discussion of the background issue addressed in this patch can be found here: https://github.com/golang/go/issues/1435 In the case where cgo is used, the C runtime can launch pthreads that are not managed by the Go runtime. As such, the added syscall.AllThreadsSyscall*() return ENOTSUP when cgo is enabled. However, for the 9 syscall.Set*() functions listed above, when cgo is active, these functions redirect to invoke their C.set*() equivalents in glibc, which wraps the raw system calls with a nptl:setxid fixup mechanism. This achieves POSIX semantics for these functions in the combined Go and C runtime. As a side note, the glibc/nptl:setxid support (2019-11-30) does not extend to all security related system calls under Linux so using native Go (CGO_ENABLED=0) and these AllThreadsSyscall*()s, where needed, will yield more well defined/consistent behavior over all threads of a Go program. That is, using the syscall.AllThreadsSyscall*() wrappers for things like setting state through SYS_PRCTL and SYS_CAPSET etc. Fixes #1435 Change-Id: Ib1a3e16b9180f64223196a32fc0f9dce14d9105c Reviewed-on: https://go-review.googlesource.com/c/go/+/210639 Trust: Emmanuel Odeke Trust: Ian Lance Taylor Trust: Michael Pratt Run-TryBot: Emmanuel Odeke Reviewed-by: Michael Pratt Reviewed-by: Austin Clements --- misc/cgo/test/cgo_linux_test.go | 1 + misc/cgo/test/issue1435.go | 152 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) create mode 100644 misc/cgo/test/issue1435.go (limited to 'misc/cgo') diff --git a/misc/cgo/test/cgo_linux_test.go b/misc/cgo/test/cgo_linux_test.go index 7b56e11a27..a9746b552e 100644 --- a/misc/cgo/test/cgo_linux_test.go +++ b/misc/cgo/test/cgo_linux_test.go @@ -15,5 +15,6 @@ func TestSetgid(t *testing.T) { } testSetgid(t) } +func Test1435(t *testing.T) { test1435(t) } func Test6997(t *testing.T) { test6997(t) } func TestBuildID(t *testing.T) { testBuildID(t) } diff --git a/misc/cgo/test/issue1435.go b/misc/cgo/test/issue1435.go new file mode 100644 index 0000000000..155d33baff --- /dev/null +++ b/misc/cgo/test/issue1435.go @@ -0,0 +1,152 @@ +// Copyright 2019 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// +build linux,cgo + +package cgotest + +import ( + "fmt" + "io/ioutil" + "strings" + "syscall" + "testing" +) + +// #include +// #include +// #include +// #include +// #include +// +// pthread_t *t = NULL; +// pthread_mutex_t mu; +// int nts = 0; +// int all_done = 0; +// +// static void *aFn(void *vargp) { +// int done = 0; +// while (!done) { +// usleep(100); +// pthread_mutex_lock(&mu); +// done = all_done; +// pthread_mutex_unlock(&mu); +// } +// return NULL; +// } +// +// void trial(int argc) { +// int i; +// nts = argc; +// t = calloc(nts, sizeof(pthread_t)); +// pthread_mutex_init(&mu, NULL); +// for (i = 0; i < nts; i++) { +// pthread_create(&t[i], NULL, aFn, NULL); +// } +// } +// +// void cleanup(void) { +// int i; +// pthread_mutex_lock(&mu); +// all_done = 1; +// pthread_mutex_unlock(&mu); +// for (i = 0; i < nts; i++) { +// pthread_join(t[i], NULL); +// } +// pthread_mutex_destroy(&mu); +// free(t); +// } +import "C" + +// compareStatus is used to confirm the contents of the thread +// specific status files match expectations. +func compareStatus(filter, expect string) error { + expected := filter + "\t" + expect + pid := syscall.Getpid() + fs, err := ioutil.ReadDir(fmt.Sprintf("/proc/%d/task", pid)) + if err != nil { + return fmt.Errorf("unable to find %d tasks: %v", pid, err) + } + for _, f := range fs { + tf := fmt.Sprintf("/proc/%s/status", f.Name()) + d, err := ioutil.ReadFile(tf) + if err != nil { + return fmt.Errorf("unable to read %q: %v", tf, err) + } + lines := strings.Split(string(d), "\n") + for _, line := range lines { + if strings.HasPrefix(line, filter) { + if line != expected { + return fmt.Errorf("%s %s (bad)\n", tf, line) + } + break + } + } + } + return nil +} + +// test1435 test 9 glibc implemented setuid/gid syscall functions are +// mapped. This test is a slightly more expansive test than that of +// src/syscall/syscall_linux_test.go:TestSetuidEtc() insofar as it +// launches concurrent threads from C code via CGo and validates that +// they are subject to the system calls being tested. For the actual +// Go functionality being tested here, the syscall_linux_test version +// is considered authoritative, but non-trivial improvements to that +// should be mirrored here. +func test1435(t *testing.T) { + if syscall.Getuid() != 0 { + t.Skip("skipping root only test") + } + + // Launch some threads in C. + const cts = 5 + C.trial(cts) + defer C.cleanup() + + vs := []struct { + call string + fn func() error + filter, expect string + }{ + {call: "Setegid(1)", fn: func() error { return syscall.Setegid(1) }, filter: "Gid:", expect: "0\t1\t0\t1"}, + {call: "Setegid(0)", fn: func() error { return syscall.Setegid(0) }, filter: "Gid:", expect: "0\t0\t0\t0"}, + + {call: "Seteuid(1)", fn: func() error { return syscall.Seteuid(1) }, filter: "Uid:", expect: "0\t1\t0\t1"}, + {call: "Setuid(0)", fn: func() error { return syscall.Setuid(0) }, filter: "Uid:", expect: "0\t0\t0\t0"}, + + {call: "Setgid(1)", fn: func() error { return syscall.Setgid(1) }, filter: "Gid:", expect: "1\t1\t1\t1"}, + {call: "Setgid(0)", fn: func() error { return syscall.Setgid(0) }, filter: "Gid:", expect: "0\t0\t0\t0"}, + + {call: "Setgroups([]int{0,1,2,3})", fn: func() error { return syscall.Setgroups([]int{0, 1, 2, 3}) }, filter: "Groups:", expect: "0 1 2 3 "}, + {call: "Setgroups(nil)", fn: func() error { return syscall.Setgroups(nil) }, filter: "Groups:", expect: " "}, + {call: "Setgroups([]int{0})", fn: func() error { return syscall.Setgroups([]int{0}) }, filter: "Groups:", expect: "0 "}, + + {call: "Setregid(101,0)", fn: func() error { return syscall.Setregid(101, 0) }, filter: "Gid:", expect: "101\t0\t0\t0"}, + {call: "Setregid(0,102)", fn: func() error { return syscall.Setregid(0, 102) }, filter: "Gid:", expect: "0\t102\t102\t102"}, + {call: "Setregid(0,0)", fn: func() error { return syscall.Setregid(0, 0) }, filter: "Gid:", expect: "0\t0\t0\t0"}, + + {call: "Setreuid(1,0)", fn: func() error { return syscall.Setreuid(1, 0) }, filter: "Uid:", expect: "1\t0\t0\t0"}, + {call: "Setreuid(0,2)", fn: func() error { return syscall.Setreuid(0, 2) }, filter: "Uid:", expect: "0\t2\t2\t2"}, + {call: "Setreuid(0,0)", fn: func() error { return syscall.Setreuid(0, 0) }, filter: "Uid:", expect: "0\t0\t0\t0"}, + + {call: "Setresgid(101,0,102)", fn: func() error { return syscall.Setresgid(101, 0, 102) }, filter: "Gid:", expect: "101\t0\t102\t0"}, + {call: "Setresgid(0,102,101)", fn: func() error { return syscall.Setresgid(0, 102, 101) }, filter: "Gid:", expect: "0\t102\t101\t102"}, + {call: "Setresgid(0,0,0)", fn: func() error { return syscall.Setresgid(0, 0, 0) }, filter: "Gid:", expect: "0\t0\t0\t0"}, + + {call: "Setresuid(1,0,2)", fn: func() error { return syscall.Setresuid(1, 0, 2) }, filter: "Uid:", expect: "1\t0\t2\t0"}, + {call: "Setresuid(0,2,1)", fn: func() error { return syscall.Setresuid(0, 2, 1) }, filter: "Uid:", expect: "0\t2\t1\t2"}, + {call: "Setresuid(0,0,0)", fn: func() error { return syscall.Setresuid(0, 0, 0) }, filter: "Uid:", expect: "0\t0\t0\t0"}, + } + + for i, v := range vs { + if err := v.fn(); err != nil { + t.Errorf("[%d] %q failed: %v", i, v.call, err) + continue + } + if err := compareStatus(v.filter, v.expect); err != nil { + t.Errorf("[%d] %q comparison: %v", i, v.call, err) + } + } +} -- cgit v1.2.1 From 30c18878730434027dbefd343aad74963a1fdc48 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Thu, 1 Oct 2020 17:22:38 -0400 Subject: runtime,cmd/cgo: simplify C -> Go call path This redesigns the way calls work from C to exported Go functions. It removes several steps from the call path, makes cmd/cgo no longer sensitive to the Go calling convention, and eliminates the use of reflectcall from cgo. In order to avoid generating a large amount of FFI glue between the C and Go ABIs, the cgo tool has long depended on generating a C function that marshals the arguments into a struct, and then the actual ABI switch happens in functions with fixed signatures that simply take a pointer to this struct. In a way, this CL simply pushes this idea further. Currently, the cgo tool generates this argument struct in the exact layout of the Go stack frame and depends on reflectcall to unpack it into the appropriate Go call (even though it's actually reflectcall'ing a function generated by cgo). In this CL, we decouple this struct from the Go stack layout. Instead, cgo generates a Go function that takes the struct, unpacks it, and calls the exported function. Since this generated function has a generic signature (like the rest of the call path), we don't need reflectcall and can instead depend on the Go compiler itself to implement the call to the exported Go function. One complication is that syscall.NewCallback on Windows, which converts a Go function into a C function pointer, depends on cgocallback's current dynamic calling approach since the signatures of the callbacks aren't known statically. For this specific case, we continue to depend on reflectcall. Really, the current approach makes some overly simplistic assumptions about translating the C ABI to the Go ABI. Now we're at least in a much better position to do a proper ABI translation. For comparison, the current cgo call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> _cgoexp_GoF (generated Go function) -> cgocallback (in asm_*.s) -> cgocallback_gofunc (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> reflectcall (in asm_*.s) -> _cgoexpwrap_GoF (generated Go function) -> p.GoF Now the call path looks like: GoF (generated C function) -> crosscall2 (in cgo/asm_*.s) -> cgocallback (in asm_*.s) -> cgocallbackg (in cgocall.go) -> cgocallbackg1 (in cgocall.go) -> _cgoexp_GoF (generated Go function) -> p.GoF Notably: 1. We combine _cgoexp_GoF and _cgoexpwrap_GoF and move the combined operation to the end of the sequence. This combined function also handles reflectcall's previous role. 2. We combined cgocallback and cgocallback_gofunc since the only purpose of having both was to convert a raw PC into a Go function value. We instead construct the Go function value in cgocallbackg1. 3. cgocallbackg1 no longer reaches backwards through the stack to get the arguments to cgocallback_gofunc. Instead, we just pass the arguments down. 4. Currently, we need an explicit msanwrite to mark the results struct as written because reflectcall doesn't do this. Now, the results are written by regular Go assignments, so the Go compiler generates the necessary MSAN annotations. This also means we no longer need to track the size of the arguments frame. Updates #40724, since now we don't need to teach cgo about the register ABI or change how it uses reflectcall. Change-Id: I7840489a2597962aeb670e0c1798a16a7359c94f Reviewed-on: https://go-review.googlesource.com/c/go/+/258938 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- misc/cgo/test/callback.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'misc/cgo') diff --git a/misc/cgo/test/callback.go b/misc/cgo/test/callback.go index e749650293..814888e3ac 100644 --- a/misc/cgo/test/callback.go +++ b/misc/cgo/test/callback.go @@ -181,7 +181,7 @@ func testCallbackCallers(t *testing.T) { name := []string{ "runtime.cgocallbackg1", "runtime.cgocallbackg", - "runtime.cgocallback_gofunc", + "runtime.cgocallback", "runtime.asmcgocall", "runtime.cgocall", "test._Cfunc_callback", -- cgit v1.2.1 From 3f6b1a0d5eea4756b905db1c2b2c03e8594850d3 Mon Sep 17 00:00:00 2001 From: HowJMay Date: Tue, 27 Oct 2020 17:03:48 +0000 Subject: misc/cgo/test: test C.enum_* Allocate a C enum object, and test if it can be assigned a value successfully. For #39537 Change-Id: I7b5482112486440b9d99f2ee4051328d87f45dca GitHub-Last-Rev: 81890f40acc5589563ec1206fe119873fb46dc1b GitHub-Pull-Request: golang/go#39977 Reviewed-on: https://go-review.googlesource.com/c/go/+/240697 Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor Trust: Emmanuel Odeke --- misc/cgo/test/cgo_test.go | 2 ++ misc/cgo/test/test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+) (limited to 'misc/cgo') diff --git a/misc/cgo/test/cgo_test.go b/misc/cgo/test/cgo_test.go index b745a4417f..f7a76d047b 100644 --- a/misc/cgo/test/cgo_test.go +++ b/misc/cgo/test/cgo_test.go @@ -76,6 +76,8 @@ func TestCheckConst(t *testing.T) { testCheckConst(t) } func TestConst(t *testing.T) { testConst(t) } func TestCthread(t *testing.T) { testCthread(t) } func TestEnum(t *testing.T) { testEnum(t) } +func TestNamedEnum(t *testing.T) { testNamedEnum(t) } +func TestCastToEnum(t *testing.T) { testCastToEnum(t) } func TestErrno(t *testing.T) { testErrno(t) } func TestFpVar(t *testing.T) { testFpVar(t) } func TestHelpers(t *testing.T) { testHelpers(t) } diff --git a/misc/cgo/test/test.go b/misc/cgo/test/test.go index a78f88499b..65823b1ca0 100644 --- a/misc/cgo/test/test.go +++ b/misc/cgo/test/test.go @@ -1000,6 +1000,32 @@ func testEnum(t *testing.T) { } } +func testNamedEnum(t *testing.T) { + e := new(C.enum_E) + + *e = C.Enum1 + if *e != 1 { + t.Error("bad enum", C.Enum1) + } + + *e = C.Enum2 + if *e != 2 { + t.Error("bad enum", C.Enum2) + } +} + +func testCastToEnum(t *testing.T) { + e := C.enum_E(C.Enum1) + if e != 1 { + t.Error("bad enum", C.Enum1) + } + + e = C.enum_E(C.Enum2) + if e != 2 { + t.Error("bad enum", C.Enum2) + } +} + func testAtol(t *testing.T) { l := Atol("123") if l != 123 { -- cgit v1.2.1