An exported Go function like
//export F
func F() {}
gets declared in _cgo_export.h as something like
extern void F(void);
The exact declaration varies by operating system.
In particular, Windows adds __declspec(dllimport).
Clang on Windows/ARM64 rejects code that contains
conflicting declarations for F, like:
extern void F(void);
extern void __declspec(dllimport) F(void);
This means that F must not be declared separately from _cgo_export.h:
any code that wants to refer to F must use #include "_cgo_export.h".
Unfortunately, the cgo prologue itself (the commented code before import "C")
cannot include "_cgo_export.h", because that file is itself produced from the
cgo Go sources and therefore cannot be a dependency of the cgo Go sources.
This CL rewrites misc/cgo/test to avoid redeclaring exported functions.
Most of the time, this is not a significant problem: just move the code
that needs the header into a .c file, perhaps with a wrapper exposed
to the cgo Go sources.
The one case that is potentially problematic is f7665, which is part of
the test for golang.org/issue/7665. That bug report explicitly identified
a bug in referring to the C name for an exported function in the same
Go source file as it was exported function. That is now impossible,
at least on Windows/ARM64, so the test is modified a bit and possibly
does not test what the original bug was. But the original bug should
be long gone: that part of the compiler has been rewritten.
Change-Id: I0d14d9336632f0e5e3db4273d9d32ef2cca0298d
Reviewed-on: https://go-review.googlesource.com/c/go/+/312029
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
tests that run commands should log their actions in a
shell-pasteable way.
Change-Id: Ifeee88397047ef5a76925c5f30c213e83e535038
Reviewed-on: https://go-review.googlesource.com/c/go/+/309770
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This follows the spelling choices that the Go project has made for English words.
https://github.com/golang/go/wiki/Spelling
Change-Id: Ie7c586d2cf23020cb492cfff58c0831d2d8d3a78
GitHub-Last-Rev: e16a32cd22
GitHub-Pull-Request: golang/go#45442
Reviewed-on: https://go-review.googlesource.com/c/go/+/308291
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
For #42076Fixes#45451
Change-Id: I69646226d3480d5403205412ddd13c0cfc2c8a53
Reviewed-on: https://go-review.googlesource.com/c/go/+/308970
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
A non-trivial Cgo program may need to use callbacks and interact with
go objects per goroutine. Because of the rules for passing pointers
between Go and C, such a program needs to store handles to associated
Go values. This often causes much extra effort to figure out a way to
correctly deal with: 1) map collision; 2) identifying leaks and 3)
concurrency.
This CL implements a Handle representation in runtime/cgo package, and
related methods such as Value, Delete, etc. which allows Go users can
use a standard way to handle the above difficulties.
In addition, the CL allows a Go value to have multiple handles, and the
NewHandle always returns a different handle compare to the previously
returned handles. In comparison, CL 294670 implements a different
behavior of NewHandle that returns a unique handle when the Go value is
referring to the same object.
Benchmark:
name time/op
Handle/non-concurrent-16 487ns ± 1%
Handle/concurrent-16 674ns ± 1%
Fixes#37033
Change-Id: I0eadb9d44332fffef8fb567c745246a49dd6d4c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/295369
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Zhang <cherryyz@google.com>
Pass information about original end position for c function arguments
processed in pointer checking generated code.
Fixes#42580
Change-Id: Ic8a578168362f0ca6055064dbbea092ad37477a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/269760
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Update references missed in CL 263142.
For #41190
Change-Id: I778760a6a69bd0440fec0848bdef539c9ccb4ee1
GitHub-Last-Rev: dda42b09ff
GitHub-Pull-Request: golang/go#42874
Reviewed-on: https://go-review.googlesource.com/c/go/+/273946
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Zhang <cherryyz@google.com>
Static tmps are private to a package, but with plugins a package
can be shared among multiple DSOs. They need to have a consistent
view of the static tmps, especially for writable ones. So export
them. (Read-only static tmps have the same values anyway, so it
doesn't matter. Also Mach-O doesn't support dynamically exporting
read-only symbols anyway.)
Fixes#44956.
Change-Id: I921e25b7ab73cd5d5347800eccdb7931e3448779
Reviewed-on: https://go-review.googlesource.com/c/go/+/301793
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
For https://gcc.gnu.org/PR99553
Change-Id: I29a7fbfd89963d4139bc19af99330d70567938ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/300993
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Currently, the cmd/dist runs test cases in misc/cgo/testsantizers only
when memeory sanitizer is supported, but the tsan tests in
misc/cgo/testsanitizers do not require support for -msan option, which
makes tsan tests can not be run on some unsupported -msan option platforms.
Therefore, this patch moves the test constraints from cmd/dist to
msan_test.go, so that the tsan tests in misc/cgo/testsanitizers
can be run on any system where the C compiler supports -fsanitize=thread
option.
Change-Id: I779c92eedd0270050f1a0b1a69ecce50c3712bc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/297774
Trust: fannie zhang <Fannie.Zhang@arm.com>
Run-TryBot: fannie zhang <Fannie.Zhang@arm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When using plugins, a type (whose value) may be pass to a plugin
and get converted to interface there, or vice versa. We need to
treat the type as potentially converted to interface, and retain
its methods.
Should fix#44586.
Change-Id: I80dd35e68baedaa852a317543ccd78d94628d13b
Reviewed-on: https://go-review.googlesource.com/c/go/+/296709
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
s/!gccgo/gc/ in files which use gc-syntax assembly.
Change-Id: Ifdadb62edd1210ebc70e7cd415648b752afaf067
Reviewed-on: https://go-review.googlesource.com/c/go/+/269957
Reviewed-by: Than McIntosh <thanm@google.com>
Trust: David Chase <drchase@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Both asmcgocall and systemstack need to save the calling Go code's
context for use by traceback, but they do it differently.
Systemstack's appraoch is better, because it doesn't require a
special case in traceback.
So make them both use that.
While we are here, the fake mstart caller in systemstack is
no longer needed and can be removed.
(traceback knows to stop in systemstack because of the writes to SP.)
Also remove the fake mstarts in sys_windows_*.s.
And while we are there, fix the control flow guard code in sys_windows_arm.s.
The current code is using pointers to a stack frame that technically is gone
once we hit the RET instruction. Clearly it's working OK, but better not to depend
on data below SP being preserved, even for just a few instructions.
Store the value we need in other registers instead.
(This code is only used for pushing a sigpanic call, which does not
actually return to the site of the fault and therefore doesn't need to
preserve any of the registers.)
This CL is part of a stack adding windows/arm64
support (#36439), intended to land in the Go 1.17 cycle.
This CL is, however, not windows/arm64-specific.
It is cleanup meant to make the port (and future ports) easier.
Change-Id: Id1e3ef5e54f7ad786e4b87043f2626eba7c3bbd9
Reviewed-on: https://go-review.googlesource.com/c/go/+/288799
Trust: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In the linker's deadcode pass we decode type symbols for
interface satisfaction analysis. When linking against Go shared
libraries, the type symbol may come from a shared library, so it
doesn't have data in the current module being linked, so we cannot
decode it. We already have code to skip DYNIMPORT symbols. However,
this doesn't actually work, because at that point the type symbols'
names haven't been mangled, whereas they may be mangled in the
shared library. So the symbol definition (in shared library) and
reference (in current module) haven't been connected.
Skip decoding type symbols of type Sxxx (along with DYNIMPORT)
when linkShared.
Note: we cannot skip all type symbols, as we still need to mark
unexported methods defined in the current module.
Fixes#44031.
Change-Id: I833d19a060c94edbd6fc448172358f9a7d760657
Reviewed-on: https://go-review.googlesource.com/c/go/+/288496
Trust: Cherry Zhang <cherryyz@google.com>
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
The original Darwin/ARM port is gone. For ARM64, it works fine
without the flags on macOS/ARM64. Remove the flags.
Change-Id: I9cc00c49dd71376dd9c52abb78c2d8cec656b3db
Reviewed-on: https://go-review.googlesource.com/c/go/+/280157
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Somehow I missed that one. It works fine.
Change-Id: I0b1286bf1e6a8f40b9f3f114f49b3034079e0b85
Reviewed-on: https://go-review.googlesource.com/c/go/+/280156
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, for data moving, we generate an msanread of the source,
followed by an msanwrite of the destination. msanread checks
the source is initialized.
This has a problem: if the source is an aggregate type containing
alignment paddings, the padding bytes may not be thought as
initialized by MSAN. If we copy the aggregate type by value, if
it counts as a read, MSAN reports using uninitialized data. This
CL changes it to use __msan_memmove for data copying, which tells
MSAN to propagate initialized-ness but not check for it.
Caveat: technically __msan_memmove is not a public API of MSAN,
although the C compiler does generate direct calls to it.
Also, when instrumenting a load of a struct, split the
instrumentation to fields, instead of generating an msanread for
the whole struct. This skips padding bytes, which may not be
considered initialized in MSAN.
Fixes#42820.
Change-Id: Id861c8bbfd94cfcccefcc58eaf9e4eb43b4d85c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/270859
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Plugin exports symbols as interfaces. Mark their types as used in
interfaces, so their methods will be kept alive by the linker.
Fixes#42579.
Change-Id: If1b5aacc21510c20c25f88bb131bca61db6f1d56
Reviewed-on: https://go-review.googlesource.com/c/go/+/269819
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
CL 258938 changed the way C to Go calls work such that they now
construct a C struct on the C side for the arguments and space for the
results. Any pointers in the result space must be zeroed, so we just
zero the whole struct.
However, C makes it surprisingly hard to robustly zero any struct
type. We had used a "{0}" initializer, which works in the vast
majority of cases, but fails if the type is empty or effectively
empty.
This CL fixes this by changing how the cgo tool zero-initializes the
argument struct to be more robust.
Fixes#42495.
Change-Id: Id1749b9d751e59eb7a02a9d44fec0698a2bf63cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/269337
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
A hand-edited object file can have a symbol name that uses newline and
other normally invalid characters. The cgo tool will generate Go files
containing symbol names, unquoted. That can permit those symbol names
to inject Go code into a cgo-generated file. If that Go code uses the
//go:cgo_ldflag pragma, it can cause the C linker to run arbitrary
code when building a package. If you build an imported package we
permit arbitrary code at run time, but we don't want to permit it at
package build time. This CL prevents this in two ways.
In cgo, reject invalid symbols that contain non-printable or space
characters, or that contain anything that looks like a Go comment.
In the go tool, double check all //go:cgo_ldflag directives in
generated code, to make sure they follow the existing LDFLAG restrictions.
Thanks to Imre Rad / https://www.linkedin.com/in/imre-rad-2358749b for
reporting this.
Fixes CVE-2020-28367
Change-Id: Ia1ad8f3791ea79612690fa7d26ac451d0f6df7c1
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/895832
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/269658
Trust: Katie Hockman <katie@golang.org>
Trust: Roland Shoemaker <roland@golang.org>
Run-TryBot: Katie Hockman <katie@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
TestSetuidEtc() was failing sporadically on linux-ppc64. From the
three https://build.golang.org/ logs, it looked like the logged
errors could be associated with threads dying, but proc reads
were, in some way, racing with their demise.
Exploring ways to increase thread demise, revealed that races
of this type can happen on non-ppc64 systems, and that
os.IsNotExist(err) was not a sufficient error condition test
for a thread's status file disappearing. This change includes a
fix for that to.
The actual issue on linux-ppc64 appears to be tied to PID reaping
and reuse latency on whatever the build test environment is for
linux-ppc64-buildlet. I suspect this can happen on any linux
system, however, especially where the container has a limited PID
range.
The fix for this, limited to the test (the runtime syscall support
is unchanged), is to confirm that the Pid for the interrogated
thread's /proc/<TID>/status file confirms that it is still
associated with the test-process' PID.
linux-ppc64-buildlet:
go/bin/go test syscall -run=TestSetuidEtc -count=10000
ok syscall 104.285s
Fixes#42462
Change-Id: I55c84ab8361003570a405fa52ffec4949bf91113
Reviewed-on: https://go-review.googlesource.com/c/go/+/268717
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Tobias Klauser <tobias.klauser@gmail.com>
Fixes#36641
Change-Id: I51868d83ce341d78d33b221d184c5a5110c60d14
Reviewed-on: https://go-review.googlesource.com/c/go/+/263598
Trust: Joel Sing <joel@sing.id.au>
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Allocate a C enum object, and test if it can be assigned a value
successfully.
For #39537
Change-Id: I7b5482112486440b9d99f2ee4051328d87f45dca
GitHub-Last-Rev: 81890f40ac
GitHub-Pull-Request: golang/go#39977
Reviewed-on: https://go-review.googlesource.com/c/go/+/240697
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Emmanuel Odeke <emmanuel@orijtech.com>
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 <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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 <emm.odeke@gmail.com>
Trust: Ian Lance Taylor <iant@golang.org>
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
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 #6853Fixes#30674
Change-Id: I9c7fb09c677cc760f24d0f7d199740ae73981413
Reviewed-on: https://go-review.googlesource.com/c/go/+/262797
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Trust: Alex Brainman <alex.brainman@gmail.com>
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 <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
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 <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
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 <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
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 <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Introduce GOOS=ios for iOS systems. GOOS=ios matches "darwin"
build tag, like GOOS=android matches "linux" and GOOS=illumos
matches "solaris". Only ios/arm64 is supported (ios/amd64 is
not).
GOOS=ios and GOOS=darwin remain essentially the same at this
point. They will diverge at later time, to differentiate macOS
and iOS.
Uses of GOOS=="darwin" are changed to (GOOS=="darwin" || GOOS=="ios"),
except if it clearly means macOS (e.g. GOOS=="darwin" && GOARCH=="amd64"),
it remains GOOS=="darwin".
Updates #38485.
Change-Id: I4faacdc1008f42434599efb3c3ad90763a83b67c
Reviewed-on: https://go-review.googlesource.com/c/go/+/254740
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The cgo tool would sometimes emit a bitfield at an offset that did not
correspond to the C offset, such as for the example in the new test.
Change-Id: I61b2ca10ee44a42f81c13ed12865f2060168fed5
Reviewed-on: https://go-review.googlesource.com/c/go/+/252378
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
cgo effectively prepends -I${SRCDIR} to the header include path of all
preambles it processes, so when an #include <> matches a header file
both in the source directory and also another include directory, the
local copy will be used in preference.
This behaviour is surprising but unfortunately also longstanding and
relied upon by packages in the wild, so the best we can do is to
document it.
Fixes#41059
Change-Id: If6d2818294b2bd94ea0fe5fd6ce77e54b3e167a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/251758
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Consider this test package:
package p
// enum E { E0 };
// union U { long x; };
// void f(enum E e, union U* up) {}
import "C"
func f() {
C.f(C.enum_E(C.E0), (*C.union_U)(nil))
}
In Go 1.14, cgo translated this to (omitting irrelevant details):
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(uint32(_Ciconst_E0), (*[8]byte)(nil))
}
func _Cfunc_f(p0 uint32, p1 *[8]byte) (r1 _Ctype_void) { ... }
Notably, _Ctype_union_U was declared as a defined type, but uses were
being rewritten into uses of the underlying type, which matched how
_Cfunc_f was declared.
After CL 230037, cgo started consistently rewriting "C.foo" type
expressions as "_Ctype_foo", which caused it to start emitting:
type _Ctype_enum_E uint32
type _Ctype_union_U [8]byte
func f() {
_Cfunc_f(_Ctype_enum_E(_Ciconst_E0), (*_Ctype_union_U)(nil))
}
// _Cfunc_f unchanged
Of course, this fails to type-check because _Ctype_enum_E and
_Ctype_union_U are defined types.
This CL changes cgo to emit:
type _Ctype_enum_E = uint32
type _Ctype_union_U = [8]byte
// f unchanged since CL 230037
// _Cfunc_f still unchanged
It would probably be better to fix this in (*typeConv).loadType so
that cgo generated code uses the _Ctype_foo aliases too. But as it
wouldn't have any effect on actual compilation, it's not worth the
risk of touching it at this point in the release cycle.
Updates #39537.
Fixes#40494.
Change-Id: I88269660b40aeda80a9a9433777601a781b48ac0
Reviewed-on: https://go-review.googlesource.com/c/go/+/246057
Reviewed-by: Ian Lance Taylor <iant@golang.org>
During the transitioning period, we mark symbols from Go shared
libraries reachable unconditionally. That might be useful when
there was still a large portion of the linker using sym.Symbols,
and only reachable symbols were converted to sym.Symbols. Marking
them reachable brings them to the dynamic symbol table, even if
they are not needed, increased the binary size unexpectedly.
That time has passed. Now we largely operate on loader symbols,
and it is not needed to mark them reachable anymore.
Fixes#40416.
Change-Id: I1e2bdb93a960ba7dc96575fabe15af93d8e95329
Reviewed-on: https://go-review.googlesource.com/c/go/+/244839
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When linking against a Go shared library, when a global variable
in the main module has a type defined in the shared library, the
linker needs to pull the GC data from the shared library to build
the GC program for the global variable. Currently, this fails
silently, as the shared library file is closed too early and the
read failed (with no error check), causing a zero GC map emitted
for the variable, which in turn causes the runtime to treat the
variable as pointerless.
For now, fix this by keeping the file open. In the future we may
want to use mmap to read from the shared library instead.
Also add error checking. And fix a (mostly harmless) mistake in
size caluculation.
Also remove an erroneous condition for ARM64. ARM64 used to have
a special case to get the addend from the relocation on the
gcdata field. That was removed, but the new code accidentally
returned 0 unconditionally. It's no longer necessary to have any
special case, since the addend is now applied directly to the
gcdata field on ARM64, like on all the other platforms.
Fixes#39927.
This is the second attempt of CL 240462. And this reverts
CL 240616.
Change-Id: I01c82422b9f67e872d833336885935bc509bc91b
Reviewed-on: https://go-review.googlesource.com/c/go/+/240621
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This reverts CL 240462.
Reason for revert: test fails on PPC64LE.
Updates #39927.
Change-Id: I4f14fd0c36e604a80ae9f2f86d1e643e28945e93
Reviewed-on: https://go-review.googlesource.com/c/go/+/240616
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
When linking against a Go shared library, when a global variable
in the main module has a type defined in the shared library, the
linker needs to pull the GC data from the shared library to build
the GC program for the global variable. Currently, this fails
silently, as the shared library file is closed too early and the
read failed (with no error check), causing a zero GC map emitted
for the variable, which in turn causes the runtime to treat the
variable as pointerless.
For now, fix this by keeping the file open. In the future we may
want to use mmap to read from the shared library instead.
Also add error checking. And fix a (mostly harmless) mistake in
size caluculation.
Also remove an erroneous condition for ARM64. ARM64 used to have
a special case to get the addend from the relocation on the
gcdata field. That was removed, but the new code accidentally
returned 0 unconditionally. It's no longer necessary to have any
special case, since the addend is now applied directly to the
gcdata field on ARM64, like on all the other platforms.
Fixes#39927.
Change-Id: Iecd32315b326c7059587fdc190e2fa99426e497e
Reviewed-on: https://go-review.googlesource.com/c/go/+/240462
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Austin Clements <austin@google.com>
CL 230037 changed cmd/cgo to emit "type _Ctype_foo = bar" aliases for
all C.foo types mentioned in the original Go source files. However,
cmd/cgo already emits an appropriate type definition for _Ctype_void.
So if a source file explicitly mentions C.void, this resulted in
_Ctype_void being declared multiple times.
This CL fixes the issue by suppressing the "type _Ctype_void =
_Ctype_void" alias before printing it. This should be safe because
_Ctype_void is the only type that's specially emitted in out.go at the
moment.
A somewhat better fix might be to fix how _Ctype_void is declared in
the cmd/cgo "frontend", but this is a less invasive fix.
Fixes#39877.
Change-Id: Ief264b3847c8ef8df1478a6333647ff2cf09b63d
Reviewed-on: https://go-review.googlesource.com/c/go/+/240180
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Normally, packages are loaded in dependency order, and if a
Library object is not nil, it is already loaded with the actual
fingerprint. In shared build mode, however, packages may be added
not in dependency order (e.g. go install -buildmode=shared std
adds all std packages before loading them), and it is possible
that a Library's fingerprint is not yet loaded. Skip the check
in this case (when the fingerprint is the zero value).
Fixes#39777.
Change-Id: I66208e92bf687c8778963ba8e33e9bd948f82f3a
Reviewed-on: https://go-review.googlesource.com/c/go/+/239517
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Similarly to EGLDisplay, EGLConfig is declared as a pointer but may
contain non-pointer values.
I believe this is the root cause of https://todo.sr.ht/~eliasnaur/gio/121.
Change-Id: I412c4fbc2eef4aa028534d68bda95db98e3a365d
Reviewed-on: https://go-review.googlesource.com/c/go/+/235817
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Cgo's initial design for handling "#define foo int*" involved
rewriting "C.foo" to "*_Ctype_int" everywhere. But now that we have
type aliases, we can declare "type _Ctype_foo = *_Ctype_int" once, and
then rewrite "C.foo" to just "_Ctype_foo".
This is important for go/types's UsesCgo mode, where go/types needs to
be able to figure out a type for each C.foo identifier using only the
information written into _cgo_gotypes.go.
Fixes#38649.
Change-Id: Ia0f8c2d82df81efb1be5bc26195ea9154c0af871
Reviewed-on: https://go-review.googlesource.com/c/go/+/230037
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This removes all conditions and conditional code (that I could find)
that depended on darwin/386.
Fixes#37610.
Change-Id: I630d9ea13613fb7c0bcdb981e8367facff250ba0
Reviewed-on: https://go-review.googlesource.com/c/go/+/227582
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This removes all conditions and conditional code (that I could find)
that depended on darwin/arm.
Fixes#35439 (since that only happened on darwin/arm)
Fixes#37611.
Change-Id: Ia4c32a5a4368ed75231075832b0b5bfb1ad11986
Reviewed-on: https://go-review.googlesource.com/c/go/+/227198
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 181857 broke the translation of certain C types using cmd/cgo -godefs
because it stores each typedef, array and qualified type with their
parent type name in the translation cache.
Fix this by only considering the parent type for typedefs of anonymous
structs which is the only case where types might become ambiguous.
Updates #31891Fixes#37479Fixes#37621
Change-Id: I301a749ec89585789cb0d213593bb8b7341beb88
Reviewed-on: https://go-review.googlesource.com/c/go/+/226341
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trace output showing how dummy GOROOT was being set up was incorrect
(sense of the "cp -r" trace messages was inverted). This patch fixes
the problem.
Change-Id: Ib0ee649e305bfa1bc0c49e0d5ba2ea31e0a4f67e
Reviewed-on: https://go-review.googlesource.com/c/go/+/224377
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Increase the size of the signal stack as the value given by SIGSTKSZ
is too small for the Go signal handler.
Fixes#37609
Change-Id: I56f1006bc69a2a9fb43f9e0da00061964290a690
Reviewed-on: https://go-review.googlesource.com/c/go/+/221804
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If GOBIN is set in the GOENV file, then merely unsetting it in the
process environment is not sufficient. We can instead either set GOBIN
explicitly, or disable GOENV explicitly. For now, we (semi-arbitrary)
choose the former.
Fixes#37390
Change-Id: Iec54532c804b70546d695105cd89e9169eac5dbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/220652
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tests are skipped on linux/ppc64, not aix/ppc64.
Change-Id: I6b91b89f24d76b0f9be3eaf816f81ad4246e418f
Reviewed-on: https://go-review.googlesource.com/c/go/+/220423
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Regression tests for #24161 use a macro to conditionally compile some
stub definitions. The macro tests that the minimum macOS version is
less than 10.12.
We get duplicate definitions when building this test with
CGO_CFLAGS=-mmacosx-version-min=10.x where 10.x < 10.12. With this
change, we use a different macro, __MAC_OS_X_VERSION_MAX_ALLOWED__,
which tests the SDK version instead of the minimum macOS version. This
checks whether these definitions are present in headers.
After this change, 'go tool dist test cgo_test' should pass with
CGO_FLAGS=-mmacosx-version-min=10.10.
Updates #35459
Change-Id: I88d63601c94b0369c73c38d216a2d41ba7d4e579
Reviewed-on: https://go-review.googlesource.com/c/go/+/216243
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Go 1.14 will drop support for macOS 10.10, see #23011
This reverts CL 125304
Updates #26475
Updates #26513
Change-Id: Ia13eef30f22d67103f7ae45424124fbb116e1261
Reviewed-on: https://go-review.googlesource.com/c/go/+/214057
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The test for issue 8945 was marked to only run on gccgo, but there was
no reason for that. It broke for gccgo using GCC 10, because GCC 10
defaults to -fno-common. Make the test run on gc, and split it into
test.go and testx.go to make it work with GCC 10.
The test for issue 9026 used two identical structs which GCC 10 turns
into the same type. The point of the test is not that the structs are
identical, but that they are handled in a particular order. So make
them different.
Updates #8945
Updates #9026
Change-Id: I000fb02f88f346cfbbe5dbefedd944a2c64e8d8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/211217
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Instead of installing shared libraries to GOROOT/pkg, clone the
necessary files into a new GOROOT and run there.
Given that we now have a build cache, ideally we should not need to
install into GOROOT/pkg at all, but we can't fix that during the 1.14
code freeze.
Updates #28387
Updates #28553
Updates #30316
Change-Id: I83084a8ca29a5dffcd586c7fccc3f172cac57cc6
Reviewed-on: https://go-review.googlesource.com/c/go/+/208482
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Otherwise, these tests produce no output, which can make the overall
output of all.bash a bit tricky to decipher.
Updates #30316
Updates #29062
Change-Id: I33b9e070fd28b9f21ece128e9e603a982c08b7cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/208483
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
It turns out that the relative-path support never worked in the first
place.
It had been masked by the fact that we ~never invoke overlayDir with
an absolute path, which caused filepath.Rel to always return an error,
and overlayDir to always fall back to absolute paths.
Since the absolute paths seem to be working fine (and are simpler),
let's stick with those. As far as I can recall, the relative paths
were only a space optimization anyway.
Updates #28387
Updates #30316
Change-Id: Ie8cd28f3c41ca6497ace2799f4193d7f5dde7a37
Reviewed-on: https://go-review.googlesource.com/c/go/+/208481
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Previously, 'go test -v' in this directory would result in a massive
dump of go command output, because the test plumbed -v to 'build -x'.
This change separates them into distinct flags, so that '-v' only
implies the display of default 'go' command output.
Updates #30316
Change-Id: Ifb125f35ec6a0bebe7e8286e7c546d132fb213df
Reviewed-on: https://go-review.googlesource.com/c/go/+/208232
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The tests in this package invoked 'go install -i -buildmode=c-shared'
in order to generate an archive as well as multiple C header files.
Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_testcshared_shared, which may not be writable
— for example, if GOROOT is owned by the root user but the test is
being run by a non-root user.
Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.
In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.
Updates #28387
Updates #30316
Updates #35715
Change-Id: I622426a860828020d98f7040636f374e5c766d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/208119
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also add a -testwork flag to facilitate debugging the test itself.
Three of the tests of this package invoked 'go install -i
-buildmode=c-archive' in order to generate an archive as well as
multiple C header files.
Unfortunately, the behavior of the '-i' flag is inappropriately broad
for this use-case: it not only generates the library and header files
(as desired), but also attempts to install a number of (unnecessary)
archive files for transitive dependencies to
GOROOT/pkg/$GOOS_$GOARCH_shared, which may not be writable — for
example, if GOROOT is owned by the root user but the test is being run
by a non-root user.
Instead, for now we generate the header files for transitive dependencies
separately by running 'go tool cgo -exportheader'.
In the future, we should consider how to improve the ergonomics for
generating transitive header files without coupling that to
unnecessary library installation.
Updates #28387
Updates #30316
Updates #35715
Change-Id: I3d483f84e22058561efe740aa4885fc3f26137b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/208117
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
One of the 'go build' commands executed by this test passed the '-i'
flag, which caused the 'go' command to attempt to install transitive
standard-library dependencies to GOROOT/pkg/$GOOS_$GOARCH_dynlink.
That failed if GOROOT/pkg was not writable (for example, if GOROOT was
owned by the root user, but the user running the test was not root).
As far as I can tell the '-i' flag is not necessary in this test.
Prior to the introduction of the build cache it may have been an
optimization, but now that the build cache is required the '-i' flag
only adds extra work.
Updates #30316
Change-Id: Ib60080a008c1941aa92b5bdd5a194d89fd6202aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/208120
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The bash script that drives this test needs to know whether the
fortran compiler works, but it doesn't actually care about the
generated binary. Write that binary to /dev/null.
Updates #28387
Updates #30316
Change-Id: I4f86da1aeb939fc205f467511fc69235a6a9af26
Reviewed-on: https://go-review.googlesource.com/c/go/+/208124
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The NDK is switching to ldd, and will stop including the gold linker.
Change-Id: If74168017c9874134b34010906ab1d94001528b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/206840
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The bash version of the test wrote intermediate files to its testdata directory.
Updates #28387
Updates #30316Fixes#35536
Change-Id: Ib81b547d3c43e90df713a2172c8f399fefb53c68
Reviewed-on: https://go-review.googlesource.com/c/go/+/206901
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The problem causing the assert in #21647 are fixed at this point,
along with various other linker issues with plugin + Darwin. With
this in mind, remove the "-ldflags=-w" workaround for plugin mode on
Darwin and re-enable the appropriate tests misc/cgo/testplugin
Fixes#21647.
Fixes#27502.
Change-Id: I5b662987b138b06cfc9e1f9f6d804cf682bd501a
Reviewed-on: https://go-review.googlesource.com/c/go/+/206198
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
When translating C types, cache the in-progress type under its parent
names, so that anonymous structs can also be translated for multiple
typedefs, without clashing.
Standalone types are not affected by this change.
Also updated the test for issue 9026 because the C struct name
generation algorithm has changed.
Fixes#31891
Change-Id: I00cc64852a2617ce33da13f74caec886af05b9f2
Reviewed-on: https://go-review.googlesource.com/c/go/+/181857
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently cgoCheckPointer is only used with one optional argument.
Using a slice for the optional arguments is quite expensive, hence
replace it with a single interface{}. This results in ~30% improvement.
When checking struct fields, they quite often end up being without
pointers. Check this before calling cgoCheckPointer, which results in
additional ~20% improvement.
Inline some p == nil checks from cgoIsGoPointer which gives
additional ~15% improvement.
All of this translates to:
name old time/op new time/op delta
CgoCall/add-int-32 46.9ns ± 1% 46.6ns ± 1% -0.75% (p=0.000 n=18+20)
CgoCall/one-pointer-32 143ns ± 1% 87ns ± 1% -38.96% (p=0.000 n=20+20)
CgoCall/eight-pointers-32 767ns ± 0% 327ns ± 1% -57.30% (p=0.000 n=18+16)
CgoCall/eight-pointers-nil-32 110ns ± 1% 89ns ± 2% -19.10% (p=0.000 n=19+19)
CgoCall/eight-pointers-array-32 5.09µs ± 1% 3.56µs ± 2% -30.09% (p=0.000 n=19+19)
CgoCall/eight-pointers-slice-32 3.92µs ± 0% 2.57µs ± 2% -34.48% (p=0.000 n=20+20)
Change-Id: I2aa9f5ae8962a9a41a7fb1db0c300893109d0d75
Reviewed-on: https://go-review.googlesource.com/c/go/+/198081
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add a bunch of extra tests and benchmarks for defer, in preparation for new
low-cost (open-coded) implementation of defers (see #34481),
- New file defer_test.go that tests a bunch more unusual defer scenarios,
including things that might have problems for open-coded defers.
- Additions to callers_test.go actually verifying what the stack trace looks like
for various panic or panic-recover scenarios.
- Additions to crash_test.go testing several more crash scenarios involving
recursive panics.
- New benchmark in runtime_test.go measuring speed of panic-recover
- New CGo benchmark in cgo_test.go calling from Go to C back to Go that
shows defer overhead
Updates #34481
Change-Id: I423523f3e05fc0229d4277dd00073289a5526188
Reviewed-on: https://go-review.googlesource.com/c/go/+/197017
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
GCC has supported the __atomic intrinsics since 4.7, and clang
supports them as well. They are better than the __sync intrinsics in
that they specify a memory model and, more importantly for our purposes,
they are reliably implemented either in the compiler or in libatomic.
Change-Id: I5e0036ea3300f65c28b1c3d1f3b93fb61c1cd646
Reviewed-on: https://go-review.googlesource.com/c/go/+/193603
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
This test contains a very tight loop with locking/unlocking that can
wind up as an unpreemptible when compiled with gccgo, depending on
inlining. Tweak the test slightly to avoid this problem.
Change-Id: I155fd2b4bfea961244eb6c6594c24ab03d32d41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/193619
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
macOS and iOS deliver SIGPIPE signals to the main thread and not
the thread that raised it by writing to a closed socket or pipe.
SIGPIPE signals can be suppressed for sockets with the SO_NOSIGPIPE
option, but there is no similar option for pipes. We have no other
choice but to never forward SIGPIPE on macOS.
This is a fixup of reverted CL 188297.
Fixes#33384
Change-Id: I09b258b078857ad3b22025bc2902d1b12d2afd92
Reviewed-on: https://go-review.googlesource.com/c/go/+/191785
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts CL 188297.
Reason for revert: broke multiple of the darwin builders.
Fixes#33943.
Change-Id: Iacff98d1450edc70402dc7a220d16fcd73337c9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/191784
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
macOS and iOS deliver SIGPIPE signals to the main thread and not
the thread that raised it by writing to a closed socket or pipe.
SIGPIPE signals can be suppressed for sockets with the SO_NOSIGPIPE
option, but there is no similar option for pipes. We have no other
choice but to never forward SIGPIPE on macOS.
Fixes#33384
Change-Id: Ice3de75b121f00006ee11c26d560e619536460be
Reviewed-on: https://go-review.googlesource.com/c/go/+/188297
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These functions are compiler generated, and as such are only available
in the internal ABI. Doing this avoids generating an alias symbol.
Doing that avoids confusion between unmangled and mangled type symbols.
Fixes#30768
Change-Id: I197a5ba6403aac11989ffa951dbe35bd0506de91
Reviewed-on: https://go-review.googlesource.com/c/go/+/186077
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
We already skipped rewriting the call if there were fewer args than
parameters. But we can also get a cgo crash if there are more args,
if at least one of the extra args uses a name qualified with "C.".
Skip the rewrite, since the build will fail later anyhow.
Fixes#33061
Change-Id: I62ff3518b775b502ad10c2bacf9102db4c9a531c
Reviewed-on: https://go-review.googlesource.com/c/go/+/185797
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
According to the documentation "When passing a pointer to a field in a
struct, the Go memory in question is the memory occupied by the field,
not the entire struct.". checkAddr states that this should also work
with type conversions, which is implemented in isType. However,
ast.StarExpr must be enclosed in ast.ParenExpr according to the go spec
(see example below), which is not considered in the checks.
Example:
// struct Si { int i; int *p; }; void f(struct I *x) {}
import "C"
type S {
p *int
i C.struct_Si
}
func main() {
v := &S{new(int)}
C.f((*C.struct_I)(&v.i)) // <- panic
}
This example will cause cgo to emit a cgoCheck that checks the whole
struct S instead of just S.i causing the panic "cgo argument has Go
pointer to Go pointer".
This patch fixes this situation by adding support for ast.ParenExpr to
isType and adds a test, that fails without the fix.
Fixes#32970.
Change-Id: I15ea28c98f839e9fa708859ed107a2e5f1483133
Reviewed-on: https://go-review.googlesource.com/c/go/+/185098
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ensure that during rewriting of expressions that take the address of
an array, that we properly recognize *ast.IndexExpr as an operation
to create a pointer variable and thus assign the proper addressOf
and deference operators as "&" and "*" respectively.
This fixes a regression from CL 142884.
Fixed#32579
Change-Id: I3cb78becff4f8035d66fc5536e5b52857eacaa3d
Reviewed-on: https://go-review.googlesource.com/c/go/+/183458
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Roll back CL 159258 and CL 168337. Those changes broke existing
code. I can't see any way to keep existing code working while also
producing good error messages for types like C.ulong (such as the ones
already tested for in misc/cgo/errors).
This is not an exact roll back because parts of the code have changed
since those CLs.
Updates #29878Fixes#31093
Change-Id: I56fe76c167ff0ab381ed273b9ca4b952402e1434
Reviewed-on: https://go-review.googlesource.com/c/go/+/180357
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
ARM64's R19-R29 and F8-F15 are callee saved registers, which
should be saved in the beginning of sigtramp, and restored at
the end.
fixes#31827
Change-Id: I622e03f1a13fec969d3a11b6a303a8a492e02bcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/177045
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Each different file that does import "C" must be compiled
and analyzed separately by cgo. Having fewer files import "C"
reduces the cost of building the test. This is especially important
because this test is built and run four different times (with different
settings) during all.bash.
go test -c in this directory used to take over 20 seconds on my laptop.
Now it takes under 5 seconds.
Removes 23.4r 29.0u 21.5s from all.bash.
For #26473.
Change-Id: Ie7cb7b0d9d6138ebd2eb548d0d8ea6e409ae10b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/177558
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Build a single binary containing all the TestPointerChecks
instead of building many small binaries,
each with its own cgo+compile+link invocation.
This cuts 'go test -run=TestPointerChecks'
from 6.7r 35.5u 26.1s to 2.1r 2.1u 1.4s.
Move as many cgo checks as possible into fewer test files
for TestReportsTypeErrors too.
This cuts 'go test -run=TestReportsTypeErrors'
from 2.1r 6.7u 6.7s to 1.5r 2.5u 2.5s.
After this change, all.bash runs in ~4:30 on my laptop.
For #26473.
Change-Id: I3787448b03689a1f62dd810957ab6013bb75582f
Reviewed-on: https://go-review.googlesource.com/c/go/+/177599
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These tests take 20 seconds each to rebuild the entire world
in their respective modes. That's too much for short mode,
including all.bash on developer machines.
Keep doing it on builders and if someone runs 'go test' by hand
in that directory.
For #26473.
Change-Id: I3dc6955bc3aa7a20fd170efcde72a7d19b37bdbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/177417
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Followup to CL 157961 and CL 158457.
Finish the list of operators and punctuation
that disable semicolon insertion at end-of-line
The reported case was "(" but "." was also missing.
Fixes#31017.
Change-Id: I0c06443f38dc8250c62e3aadd104abfa0e3be074
Reviewed-on: https://go-review.googlesource.com/c/go/+/174524
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Like GOOS=android which implies the "linux" build tag, GOOS=illumos
implies the "solaris" build tag. This lets the existing ecosystem of
packages still work on illumos, but still permits packages to start
differentiating between solaris and illumos.
Fixes#20603
Change-Id: I8f4eabf1a66060538dca15d7658c1fbc6c826622
Reviewed-on: https://go-review.googlesource.com/c/go/+/174457
Run-TryBot: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
