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>
Old trace viewer stopped working with Chrome M80+ because the
old trace viewer heavily depended on WebComponents V0 which are deprecated.
Trace viewer recently migrated to use WebComponents V0 polyfill
(crbug.com/1036492). This CL brings in the newly updated trace_viewer_full.html
(sync'd @ 9508452e)
and updates the javascript snippet included in the /trace endpoint
to use the polyfill.
This brings in webcomponents.min.js copied from
https://chromium.googlesource.com/catapult/+/9508452e18f130c98499cb4c4f1e1efaedee8962/third_party/polymer/components/webcomponentsjs/webcomponents.min.js
That is necessary because the /trace endpoint needs to import
the vulcanized trace_viewer_full.html.
It's possible that some features are not working correctly with
this polyfill. In that case, report the issue to crbug.com/1036492.
There will be a warning message in the UI (yellow banner above the timeline)
which can be hidden by clicking the 'hide' button.
This allows to render the trace in browsers other than chrome in theory,
but I observed some buttons and functions still don't work outside
chrome.
Fixes#34374.
Change-Id: Ib575f756f5e6b22ad904ede6e4d224a995ebe259
Reviewed-on: https://go-review.googlesource.com/c/go/+/219997
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
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>
tabs.getSelected has been deprecated since Chrome 33. Instead,
use tabs.query.
Fixes#35663
Change-Id: I4f7f17f948987aff8409ac8210f04eb1f7ebf908
Reviewed-on: https://go-review.googlesource.com/c/go/+/207402
Reviewed-by: Andrew Bonventre <andybons@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>
Recent Xcode versions started to complain about the current min
version:
ld: warning: OS version (6.0.0) too small, changing to 7.0.0
Change-Id: Ieb525dd3e57429fe226b9d30d584b073c5e4768c
Reviewed-on: https://go-review.googlesource.com/c/go/+/204663
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Run-TryBot: Elias Naur <mail@eliasnaur.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The js.Value struct now contains a pointer, so a finalizer can
determine if the value is not referenced by Go any more.
Unfortunately this breaks Go's == operator with js.Value. This change
adds a new Equal method to check for the equality of two Values.
This is a breaking change. The == operator is now disallowed to
not silently break code.
Additionally the helper methods IsUndefined, IsNull and IsNaN got added.
Fixes#35111
Change-Id: I58a50ca18f477bf51a259c668a8ba15bfa76c955
Reviewed-on: https://go-review.googlesource.com/c/go/+/203600
Run-TryBot: Richard Musiol <neelance@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
benchcmp was moved out of misc into x/tools in CL 60100043 in 2014,
and then replaced by a forwarding script in CL 82710043.
Five years have since passed, and the forwarding script has outlived
its usefulness. It's now more confusing than helpful. Delete it.
Change-Id: I8c7d65b97e0b3fe367df69a86ae10c7960c05be3
Reviewed-on: https://go-review.googlesource.com/c/go/+/202762
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
You were a useful port and you've served your purpose.
Thanks for all the play.
A subsequent CL will remove amd64p32 (including assembly files and
toolchain bits) and remaining bits. The amd64p32 removal will be
separated into its own CL in case we want to support the Linux x32 ABI
in the future and want our old amd64p32 support as a starting point.
Updates #30439
Change-Id: Ia3a0c7d49804adc87bf52a4dea7e3d3007f2b1cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/199499
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When running wasm in the browser, the "process" global is not defined.
This causes functions like os.Getpid() to panic, which is unusual.
For example on Windows os.Getpid() returns -1 and does not panic.
This change adds a dummy polyfill for "process" which returns -1 or an
error. It also extends the polyfill for "fs".
Fixes#34627
Replaces CL 199357
Change-Id: Ifeb12fe7e152c517848933a9ab5f6f749896dcef
Reviewed-on: https://go-review.googlesource.com/c/go/+/199698
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
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>
I noticed lots of trailing whitespace in one of cmd/trace's HTML files.
While at it, remove a few others from still-maintained files. Leave old
documents alone, such as doc/devel/weekly.html.
Change-Id: I7de7bbb6dd3fe6403bbb1f1178a8d3640c1e537b
Reviewed-on: https://go-review.googlesource.com/c/go/+/196178
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The wasm_exec.js wrapper tries to set up the argv and envp following
the UNIX conventions, but doesn't get it quite right, which can cause
runtime.goenv to crash if you get unlucky.
The main problem was that the envp array wasn't terminated with a nil
pointer, so the runtime didn't know when to stop reading the array.
This CL adds that nil pointer to the end of the envp array.
The other problem was harmless, but confusing. In the UNIX convention,
the argv array consists of argc pointers followed by a nil pointer,
followed by the envp array. However, wasm_exec.js put the environment
variable count between the two pointer arrays rather than a nil
pointer. The runtime never looks at this slot, so it didn't matter,
but the break from convention left Cherry and I trying to debug why it
*wasn't* losing any environment variables before we realized that that
layouts happened to be close enough to work. This CL switches to the
UNIX convention of simply terminating the argv array with a nil
pointer.
Change-Id: Ic9a4cd9eabb5dfa599a809b960f9e579b9f1f4db
Reviewed-on: https://go-review.googlesource.com/c/go/+/193417
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Richard Musiol <neelance@gmail.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>
In preparation for general faketime support, this renames the existing
nanotime, walltime, and write functions to nanotime1, walltime1, and
write1 and wraps them with trivial Go functions. This will let us
inject different implementations on all platforms when faketime is
enabled.
Updates #30439.
Change-Id: Ice5ccc513a32a6d89ea051638676d3ee05b00418
Reviewed-on: https://go-review.googlesource.com/c/go/+/192738
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@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>
The go command invokes each tool with -V=full to discover its version
to compute a tool id. For release versions (that don't include the
word "devel"), the go command only used the third word in
the output (e.g., "go1.13"), ignoring any toolchain experiments that
followed. With this change, the go command will use whole version line
in the tool id for release versions.
Also, when -V=full is set and there are non-default experiments,
experiments are no longer printed twice.
Fixes#33091
Change-Id: I19b96f939c7e2fbc5d8befe3659156ee4b58daef
Reviewed-on: https://go-review.googlesource.com/c/go/+/186200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
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>
