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>
A typo was made, which I noticed while looking through the recent master
commits.
Change-Id: Ieed5d6664a1f3ff5892d59abf194963b44ef0e55
Reviewed-on: https://go-review.googlesource.com/c/go/+/280454
Trust: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
This results in a nicer error message if the argument to Go.run is
omitted or of the wrong type.
Fixes#37000
Change-Id: I7f36d007f41a79b2cea1cebf5cce127786341202
Reviewed-on: https://go-review.googlesource.com/c/go/+/266117
Trust: Richard Musiol <neelance@gmail.com>
Run-TryBot: Richard Musiol <neelance@gmail.com>
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>
wasm_exec.js expects that either "require" is available or that the
globals "crypto", "TextEncoder" and "TextDecoder" are already defined.
Report a better error message if this is not the case, suggesting the
use of a polyfill.
Updates #41482
Change-Id: I5473cae15c98ae42e39f5928245b7762e7a5a8bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/261357
Trust: Richard Musiol <neelance@gmail.com>
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
An i32 passed from WebAssembly to JavaScript is always read as a signed
integer. Use the bitshift operator to turn it into an unsigned integer.
Fixes#40923
Change-Id: Ia91ed2145dd2fc3071e2fc22b86ebfcb3c1e9f4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/261358
Trust: Richard Musiol <neelance@gmail.com>
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.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>
Change-Id: I247fc9e0e26e706e6af07367f953eaa1b7e544c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/259577
Trust: Elias Naur <mail@eliasnaur.com>
Run-TryBot: Elias Naur <mail@eliasnaur.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Update the README to mention the emulator. Remove reference to gomobile
while here; there are multiple ways to develop for iOS today, including
using the c-archive buildmode directly.
Updates #38485
Change-Id: Iccef75e646ea8e1b9bc3fc37419cc2d6bf3dfdf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/255257
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Elias Naur <mail@eliasnaur.com>
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>
The paths may contain spaces. Quote them.
Change-Id: I1f67085a1e7c40f60282c2fea7104fb44a01e310
Reviewed-on: https://go-review.googlesource.com/c/go/+/254739
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@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>
JavaScript environments are quite unpredictable because bundlers add
mocks for compatibility and libraries can polute the global namespace.
Detect more of such situations:
- Add check that require("fs") returns an object.
- Fix check that require("fs") returns an non-empty object.
- Add check that "module" is defined.
Fixes#40730
Change-Id: I2ce65fc7db64bbbb0b60eec79a4cfe5c3fec99c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/248758
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
