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>
Only invoke adb for android if we're not running on android already.
Change-Id: I4eb94286a5bf09b382716a0474f3aebec40f5d74
Reviewed-on: https://go-review.googlesource.com/c/go/+/170953
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On darwin/amd64 the runtime method sigctxt.fixsigcode changes SIGSEGV
signals so that they are never marked SI_USER. CL 169120 changed the
signal handler to call fixsigcode even when the signal is delivered to
a non-Go thread. This breaks TestSignalForwardingExternal, so skip it.
Change-Id: I6740fb5a8f4f854ca69793537a983a696da3b495
Reviewed-on: https://go-review.googlesource.com/c/go/+/170446
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Permit weak symbols to be duplicates - most external linkers allow
this and there are various situations where they can occur (including
retpoline and retguard).
Fixes#29563
Change-Id: I355493c847fbc8f670a85a643db65a4cf8f9883d
Reviewed-on: https://go-review.googlesource.com/c/go/+/169658
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
On AIX, shared objects must be wrapped under an archive file.
For testso, creating libcgosotest with an extern symbol isn't
AIX-friendly. By default, ld will block such behavior. Rather than
forcing ld to work as on Linux and using the run-time linking,
goCallback became a function pointer which is set by setCallback().
Updates #30565
Change-Id: I455ab32faddd41f1b0c84cc9e503788044ad49b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/169020
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Instead of always inserting a nop to use as the target of an inline
mark, see if we can instead find an instruction we're issuing anyway
with the correct line number, and use that instruction. That way, we
don't need to issue a nop.
Makes cmd/go 0.3% smaller.
Update #29571
Change-Id: If6cfc93ab3352ec2c6e0878f8074a3bf0786b2f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/158021
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Enable pattern lib.a/shared.so.X in cgo_import_dynamic as on AIX,
archive files (.a) often have shared objects (.so) inside them.
Change-Id: I21096c75eb7fbcc7064b0b832bfa8ed862142051
Reviewed-on: https://go-review.googlesource.com/c/go/+/168877
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Some cgo tests aren't yet available on aix/ppc64.
-shared and -static don't work as expected and will be fixed latter.
Updates #30565
Change-Id: Ic59cabe685cb1cbdf89a8d1d1a1d2c4b0e8ef442
Reviewed-on: https://go-review.googlesource.com/c/go/+/164018
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The exact-width integer types are required to use two’s complement
representation and may not have padding bits, cf. §7.20.1.1/1 in the C11
standard or https://en.cppreference.com/w/c/types/integer. This ensures that
they have the same domain and representation as the corresponding Go types.
Fixes#29878
Change-Id: Ie8a51e91666dfd89731c7859abe47356c94ca1be
GitHub-Last-Rev: 546a2cc3f1
GitHub-Pull-Request: golang/go#29907
Reviewed-on: https://go-review.googlesource.com/c/go/+/159258
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Android tests are built on the host and run on the device. To do
that, the exec wrapper copies the test binary and testdata to the device.
To enable many more tests, make the copied environment more like the host:
- Copy all of pkg from GOROOT, not just the android pkg directory.
- Copy any parent testdata directories as well as the package's own.
- Copy *.go files from the package directory. This enables misc/cgo/stdio
and misc/cgo/life tests that were invisible before so disable them explicitly.
- Always copy the GOROOT, even for tests outside GOROOT. This is expensive
but only done once per make.bash.
- Build the go tool for the device and put it in PATH. Set GOCACHE
to a writable directory and disable cgo.
While here, use a single directory for all the exec wrapper files and
delete that once per make.bash as well.
In total, this CL enables many tests in the subrepos that would need skips
without it, in particular the x/tools tests.
Fixes#11452
Updates #23824
Updates #11811
Change-Id: I2e50d8b57db9bc4637f25272a5360c8b2cf4e627
Reviewed-on: https://go-review.googlesource.com/c/go/+/165797
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Instead of trying to guess type of constants in the AST,
which is hard, use the "var cgo%d Type = Constant"
so that typechecking is left to the Go compiler.
The previous code could still fail in some cases
for constants imported from other modules
or defined in other, non-cgo files.
Fixes#30527
Change-Id: I2120cd90e90a74b9d765eeec53f6a3d2cfc1b642
Reviewed-on: https://go-review.googlesource.com/c/go/+/164897
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Split TestMain into two functions so that we can defer cleanups.
Updates #30500
Change-Id: I4a5c7ddb8218a8bd056c8733c3cb9feb895e77a0
Reviewed-on: https://go-review.googlesource.com/c/164859
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The deferred os.RemoveAll was accidentally committed as commented out
in the original https://golang.org/cl/87158.
Updates #30500
Change-Id: Idc5195816d7978253760dbfd78fde6d22c456296
Reviewed-on: https://go-review.googlesource.com/c/164858
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
That makes the test more friendly to the Android exec script, since it
won't have to evaluate symlinks to find the directory.
Change-Id: I06aae3224d489eed6d7fac7e462361f3bf1dd3da
Reviewed-on: https://go-review.googlesource.com/c/164624
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
This test currently fails in the Android builders, with the message
pkg_test.go:64: go test -short: exec: "go": executable file not found in $PATH
(https://build.golang.org/log/39ec0da5bfb7793359e199cc8e358ca5a8257840)
I was not able to test this change, because I can't get 'gomote
create' to return an instance of anything Android. However, I will
watch the build dashboard after submitting to verify that the fix
works.
Updates #30228
Android appears to lack a 'go' command in the.
Change-Id: Ieacac7f50d19e2cfef2f5d60e79a159e55b5cfa8
Reviewed-on: https://go-review.googlesource.com/c/164097
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
I hope that this will fix the tests on iOS, but 'gomote create' isn't
giving me an instance I can test with. (Please patch and test before
approving.)
Updates #15919
Updates #30228
Change-Id: I1b7cd30d5b127a1ad3243b329fa005d229f69a24
Reviewed-on: https://go-review.googlesource.com/c/163726
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <mail@eliasnaur.com>
These source files fail to build with 'go test ./...'.
Move them into testdata so that only test.bash will see them.
Updates #30228
Change-Id: I3673f3cb64b0c128a2bca5fee7679b672fe90770
Reviewed-on: https://go-review.googlesource.com/c/163212
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Add _test.go files in the individal directories to invoke 'go build'
with appropriate arguments.
Move the test driver out of cmd/dist so that it's easier to invoke the
test separately (using 'go test .').
Updates #30228
Updates #28387
Change-Id: Ibc4a024a52c12a274058298b41cc90709f7f56c8
Reviewed-on: https://go-review.googlesource.com/c/163420
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This change preserves the ability to test misc/cgo/test in GOPATH
mode, at the cost of indirection through a 'go test' subprocess.
Updates #30228
Change-Id: I08de855e62278d30fa622b2f7478e43dd2ab0e96
Reviewed-on: https://go-review.googlesource.com/c/163418
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If we run 'go test ./...' in the misc module, we don't want to see
errors for these standalone files.
We could instead add +ignore tags to each file individually, but this
is exactly what a testdata directory is for.
Updates #30228
Change-Id: I7047ad888dd6aff701f5982d58b6a79f6a487c58
Reviewed-on: https://go-review.googlesource.com/c/163417
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Updates #30228
Change-Id: I830e3c83416b2e5744f30d1a903a74c50462716b
Reviewed-on: https://go-review.googlesource.com/c/163210
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Work involved in getting a stack trace is divided between
runtime.Callers and runtime.CallersFrames.
Before this CL, runtime.Callers returns a pc per runtime frame.
runtime.CallersFrames is responsible for expanding a runtime frame
into potentially multiple user frames.
After this CL, runtime.Callers returns a pc per user frame.
runtime.CallersFrames just maps those to user frame info.
Entries in the result of runtime.Callers are now pcs
of the calls (or of the inline marks), not of the instruction
just after the call.
Fixes#29007Fixes#28640
Update #26320
Change-Id: I1c9567596ff73dc73271311005097a9188c3406f
Reviewed-on: https://go-review.googlesource.com/c/152537
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
This was accidentally broken by CL 127755.
Fixes#29333
Change-Id: I5e92048c64a55c1699d6c38eb4dbbd51c817b820
Reviewed-on: https://go-review.googlesource.com/c/155037
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Similar to to macOS' CF* types and JNI's jobject and derived types,
the EGLDisplay type is declared as a pointer but can contain
non-pointers (see #27054).
Fix it the same way: map EGLDisplay to uintptr in Go.
Fixes#27054
RELNOTE=yes
Change-Id: I6136f8f8162687c5493b30ed324e29efe55a8fd7
Reviewed-on: https://go-review.googlesource.com/c/154417
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Take advantage of the new /*line*/ comments.
Fixes#26745
Change-Id: I8098642e0f11f7418fe81b9a08dbe07671f930fe
Reviewed-on: https://go-review.googlesource.com/c/151598
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 122575 and its successors introduced a loop calling loadDWARF,
whereas before we only called it once. Pass a single typeConv to each
call, rather than creating a new one in loadDWARF itself. Change the
maps from dwarf.Type to use string keys rather than dwarf.Type keys,
since when the DWARF is reloaded the dwarf.Type pointers will be
different. These changes permit typeConv.Type to return a consistent
value for a given DWARF type, avoiding spurious type conversion errors
due to typedefs loaded after the first loop iteration.
Fixes#27340
Change-Id: Ic33467bbfca4c54e95909621b35ba2a58216d96e
Reviewed-on: https://go-review.googlesource.com/c/152762
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fixes#28721
Change-Id: I00356f3a9b0c2fb21dc9c2237dd5296fcb3b319b
Reviewed-on: https://go-review.googlesource.com/c/152657
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Darwin doesn't support the multiple copies of the runtime package
implied by linking a c-shared library into a Go program.
Updates #29061
Change-Id: I6cf5d00babf82f1de05689c1345aaa5ae0b0659c
Reviewed-on: https://go-review.googlesource.com/c/152159
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Avoid redefinition errors when a Go file uses a cgo comment to
There is no particularly good reason to do this, but there is also no
particularly good reason that it should fail.
Fixes#27019
Change-Id: Icd6f8197a89be4ee6b03ddae675667998a8b4189
Reviewed-on: https://go-review.googlesource.com/c/152079
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The old code ignored the field alignment, and only looked at the field
offset: if the field offset required padding, cgo added padding. But
while that approach works for Go (at least with the gc toolchain) it
doesn't work for C code using packed structs. With a packed struct the
added padding may leave the struct at a misaligned position, and the
inserted alignment, which cgo is not considering, may introduce
additional, unexpected, padding. Padding that ignores alignment is not
a good idea when the struct is not packed, and Go structs are never
packed. So don't ignore alignment.
Fixes#28896
Change-Id: Ie50ea15fa6dc35557497097be9fecfecb11efd8a
Reviewed-on: https://go-review.googlesource.com/c/150602
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
An untyped constant can be defined in any input file, we shouldn't
segregate them by file.
Updates #28772
Change-Id: I0347f15236833bb511eb49f86c449ee9241b0a25
Reviewed-on: https://go-review.googlesource.com/c/151600
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
cgocall could previously invoke the race detector on an M whose P had
been retaken. The race detector would attempt to use the P-local state
from this stale P, racing with the thread that was actually wired to
that P. The result was memory corruption of ThreadSanitizer's internal
data structures that presented as hard-to-understand assertion failures
and segfaults.
Reorder cgocall so that it always acquires a P before invoking the race
detector, and add a test that stresses the interaction between cgo and
the race detector to protect against future bugs of this kind.
Fixes#27660.
Change-Id: Ide93f96a23490314d6647547140e0a412a97f0d4
Reviewed-on: https://go-review.googlesource.com/c/148717
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Updating each call in place broke when there were multiple cgo calls
used as arguments to another cgo call where some required rewriting.
Instead, rewrite calls to strings via the existing mangling mechanism,
and only substitute the top level call in place.
Fixes#28540
Change-Id: Ifd66f04c205adc4ad6dd5ee8e79e57dce17e86bb
Reviewed-on: https://go-review.googlesource.com/c/146860
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
For some reason on darwin the linker still can't add debug sections to
plugins. Executables importing "plugin" do have them, however.
Because of issue 25841, plugins on darwin would likely have bad debug
info anyway so, for now, this isn't a great loss.
This disables the check for debug sections in plugins for darwin only.
Updates #27502
Change-Id: Ib8f62dac1e485006b0c2b3ba04f86d733db5ee9a
Reviewed-on: https://go-review.googlesource.com/133435
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Splits part of dwarfgeneratedebugsyms into a new function,
dwarfGenerateDebugInfo which is called between deadcode elimination
and type name mangling.
This function takes care of collecting and processing the DIEs for
all functions and package-level variables and also generates DIEs
for all types used in the program.
Fixes#23733
Change-Id: I75ef0608fbed2dffc3be7a477f1b03e7e740ec61
Reviewed-on: https://go-review.googlesource.com/111237
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The cgo tool predefines some C types such as C.uint. Don't give an
error if the type that cgo defines does not match the type in a header file.
Fixes#26743
Change-Id: I9ed3b4c482b558d8ffa8bf61eb3209415b7a9e3c
Reviewed-on: https://go-review.googlesource.com/127356
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Ensure that we call FinishType on all the types added to the ptrs map.
We only add a key to ptrKeys once. Once we FinishType for that key,
we'll never look at that key again. But we can add a new type under that
key later, and we'll never finish it.
Make sure we add the key to the ptrKeys list every time we make the list
of types for that key non-empty.
This makes sure we FinishType each pointer type exactly once.
Fixes#26517
Change-Id: Iad86150d516fcfac167591daf5a26c38bec7d143
Reviewed-on: https://go-review.googlesource.com/126275
Reviewed-by: Ian Lance Taylor <iant@golang.org>
In -godefs mode any typedefs that appear in struct fields and the like
will presumably be defined in the input file. If we resolve to the
base type, those cross-references will not work. So for -godefs mode,
keep the Go 1.10 behavior and don't resolve the typedefs in a loop.
Fixes#26644
Change-Id: I48cf72d9eb5016353c43074e6aff6495af326f35
Reviewed-on: https://go-review.googlesource.com/125995
Reviewed-by: Keith Randall <khr@golang.org>
For unknown reasons, linking against CoreFoundation on macOS 10.10
sometimes causes mmap to ignore the hint address, which makes the Go
allocator incompatible with TSAN. Currently, the effect of this is to
run the allocator out of arena hints on the very first allocation,
causing a "too many address space collisions for -race mode" panic.
This CL skips the cgo tests that link against CoreFoundation in race
mode.
Updates #26475.
Updates #26513.
Change-Id: I52ec638c99acf5d4966e68ff0054f7679680dac6
Reviewed-on: https://go-review.googlesource.com/125304
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
In CLs 122575 and 123177 the cgo tool started explicitly looking up
typedefs. When there are two Go files using import "C", and the first
one has an incomplete typedef and the second one has a complete
version of the same typedef, then we will now record a version of the
first typedef which will not match the recorded version of the second
typedef, producing an "inconsistent definitions" error. Fix this by
silently merging incomplete typedefs with complete ones.
Fixes#26430
Change-Id: I9e629228783b866dd29b5c3a31acd48f6e410a2d
Reviewed-on: https://go-review.googlesource.com/124575
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
In Android's NDK16, jobject is now declared as:
#ifdef __cplusplus
class _jobject {};
typedef _jobject* jobject;
#else /* not __cplusplus */
typedef void* jobject;
#endif
This makes the jobject to uintptr check fail because it expects the
following definition:
struct _jobject;
typedef struct _jobject *jobject;
Update the type check to handle that new type definition in both C and
C++ modes.
Fixes#26213
Change-Id: Ic36d4a5176526998d2d5e4e404f8943961141f7a
GitHub-Last-Rev: 42037c3c58
GitHub-Pull-Request: golang/go#26221
Reviewed-on: https://go-review.googlesource.com/122217
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The vet action assumes that a.Deps[0] is the compilation action for
which vet information should be generated. However, when using
-linkshared, the action graph is built with a ModeBuggyInstall action
to install the shared library built from the compilation action.
Adjust the set up of the vet action accordingly. Also don't clean up
the working directory after completing the buggy install.
Updates #26400
Change-Id: Ia51f9f6b8cde5614a6f2e41b6207478951547770
Reviewed-on: https://go-review.googlesource.com/124275
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TARGET_OS_OSX is the right macro, but it also was only introduced
in 1.12. For 1.11 and earlier a reasonable substitution is
TARGET_OS_IPHONE == 0.
Update #24161
Update #26355
Change-Id: I5f43c463d14fada9ed1d83cc684c7ea05d94c5f3
Reviewed-on: https://go-review.googlesource.com/124075
Run-TryBot: Keith Randall <khr@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The test in CL 123715 doesn't work on iOS, it needs to use a different
version scheme to determine whether SecKeyAlgorithm and friends exist.
Restrict the old version test to OSX only.
The same problem occurs on iOS: the functions tested don't exist before
iOS 10. But we don't have builders below iOS 10, so it isn't a big issue.
If we ever get older builders, or someone wants to run all.bash on an
old iOS, they'll need to figure out the right incantation.
Update #24161
Update #26355
Change-Id: Ia3ace86b00486dc172ed00c0c6d668a95565bff7
Reviewed-on: https://go-review.googlesource.com/123959
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The test uses functions from C that were introduced in OSX 1.12.
Include stubs for those functions when compiling for 1.11 and earlier.
This test really a compile-time test, it doesn't matter much what the
executed code actually does.
Use a nasty #define hack to work around the fact that cgo doesn't
support static global variables.
Update #24161Fixes#26355
Change-Id: Icf6f7bc9b6b36cacc81d5d0e033a2ebaff7e0298
Reviewed-on: https://go-review.googlesource.com/123715
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Two fixes:
1) Typedefs of the bad typedefs should also not be rewritten to the
underlying type. They shouldn't just be uintptr, though, they should
retain the C naming structure. For example, in C:
typedef const __CFString * CFStringRef;
typedef CFStringRef SecKeyAlgorithm;
we want the Go:
type _Ctype_CFStringRef uintptr
type _Ctype_SecKeyAlgorithm = _Ctype_CFStringRef
2) We need more types than just function arguments/return values.
At least we need types of global variables, so when we see a reference to:
extern const SecKeyAlgorithm kSecKeyAlgorithmECDSASignatureDigestX962SHA1;
we know that we need to investigate the type SecKeyAlgorithm.
Might as well just find every typedef and check the badness of all of them.
This requires looping until a fixed point of known types is reached.
Usually it takes just 2 iterations, sometimes 3.
Fixes#24161
Change-Id: I32ca7e48eb4d4133c6242e91d1879636f5224ea9
Reviewed-on: https://go-review.googlesource.com/123177
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We need to determine whether arguments to and return values from C
functions are "bad" typedef'd pointer types which need to be uintptr
on the Go side.
The type of those arguments are not specified explicitly. As a result,
we never look through the C declarations for the GetTypeID functions
associated with that type, and never realize that they are bad.
However, in another function in the same package there might be an
explicit reference. Then we end up with the declaration being uintptr
in one file and *struct{...} in another file. Badness ensues.
Fix this by doing a 2-pass algorithm. In the first pass, we run as
normal, but record all the argument and result types we see. In the
second pass, we include those argument types also when reading the C
types.
Fixes#24161
Change-Id: I8d727e73a2fbc88cb9d9899f8719ae405f59f753
Reviewed-on: https://go-review.googlesource.com/122575
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This test is slightly flaky on the s390x builder and I suspect that
the 100ms timeout is a little too optimistic when the VM is starved.
Increase the timeout to 5s to match the other part of the test.
Fixes#26231.
Change-Id: Ia6572035fb3efb98749f2c37527d250a4c779477
Reviewed-on: https://go-review.googlesource.com/122315
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Otherwise it is possible that msan will consider the C result to be
partially initialized, which may cause msan to think that the Go stack
is partially uninitialized. The compiler will never mark the stack as
initialized, so without this CL it is possible for stack addresses to
be passed to msanread, which will cause a false positive error from msan.
Fixes#26209
Change-Id: I43a502beefd626eb810ffd8753e269a55dff8248
Reviewed-on: https://go-review.googlesource.com/122196
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Before GCC 8 C code like
const unsigned long long int neg = (const unsigned long long) -1;
void f(void) { static const double x = (neg); }
would get an error "initializer element is not constant". In GCC 8 and
later it does not.
Because a value like neg, above, can not be used as a general integer
constant, this causes cgo to conclude that it is a floating point
constant. The way that cgo handles floating point values then causes
it to get the wrong value for it: 18446744073709551615 rather than -1.
These are of course the same value when converted to int64, but Go
does not permit that kind of conversion for an out-of-range constant.
This CL side-steps the problem by treating floating point constants
with integer type as they would up being treated before GCC 8: as
variables rather than constants.
Fixes#26066
Change-Id: I6f2f9ac2fa8a4b8218481b474f0b539758eb3b79
Reviewed-on: https://go-review.googlesource.com/121035
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is the same retry loop we use in _cgo_try_pthread_create in runtime/cgo.
Fixes#25078
Change-Id: I7ef4d4fc7fb89cbfb674c4f93cbdd7a033dd8983
Reviewed-on: https://go-review.googlesource.com/121096
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Add a test by making misc/cgo/testshared/src/trivial.go marginally less
trivial.
Fixes#25970.
Change-Id: I8815d0c56b8850fcdbf9b45f8406f37bd21b6865
Reviewed-on: https://go-review.googlesource.com/120235
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Current versions of gccgo issue a duplicate definition error when both
a definition and an empty declaration occur. Use build tags to avoid
that case for the issue9400 subdirectory.
Change-Id: I18517af87bab05e9ca43f2f295459cf34347c317
Reviewed-on: https://go-review.googlesource.com/119896
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Use more cryptic names for local variables inside C function wrappers.
Fixes#23356
Change-Id: Ia6a0218f27a13be14f589b1a0facc9683d22ff56
Reviewed-on: https://go-review.googlesource.com/86495
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
With latest gcc (7.3.0), misc/cgo/testsanitizer test will fail with reporting sigmentation
fault when running tsan test. On arm64, tsan is not supported currently and only msan test
can be run. So skip tsan test on arm64.
What needs to be pointed out is that msan test can be really run when setting clang
as c/c++ complier.
Fixes#25601
Change-Id: I6ab1a8d9edd243e2ee00ee40bc0abd6a0e6a125c
Reviewed-on: https://go-review.googlesource.com/114857
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This avoids name conflicts when two identical packages use cgo.
This can happen in practice when the same package is vendored multiple
times in a single build.
Fixes#23555
Change-Id: I9f0ec6db9165dcf9cdf3d314c668fee8ada18f9c
Reviewed-on: https://go-review.googlesource.com/118739
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When using plugins with goroutines calling cgo, we hit a case where
an intermittent SIGSEGV occurs when referencing an address that is based
on r2 (TOC address). When the failure can be generated in gdb, the
contents of r2 is wrong even though the value in the current stack's
slot for r2 is correct. So that means it somehow switched to start
running the code in this function without passing through the beginning
of the function which had the correct value of r2 and stored it there.
It was noted that in runtime.gogo when the state is restored from
gobuf, r2 is not restored from its slot on the stack. Adding the
instruction to restore r2 prevents the SIGSEGV.
This adds a testcase under testplugin which reproduces the problem if
the program is run multiple times. The team who reported this problem
has verified it fixes the issue on their larger, more complex
application.
Fixes#25756
Change-Id: I6028b6f1f8775d5c23f4ebb57ae273330a28eb8f
Reviewed-on: https://go-review.googlesource.com/117515
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This CL removes the rundircmpout action completely
because it is not used anywhere.
The run case already looks for output files. Rename the cmpout action
mentioned in tests to the run action and remove "cmpout" from run.go.
Change-Id: I835ceb70082927f8e9360e0ea0ba74f296363ab3
Reviewed-on: https://go-review.googlesource.com/115575
Run-TryBot: Yury Smolsky <yury@smolsky.by>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Fixes#25143
Change-Id: Ide654fe70651fda827cdeeaaa73d2a1f8aefd7e7
Reviewed-on: https://go-review.googlesource.com/110159
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The test has been flaky, probably due to EAGAIN, but let's find out
for sure.
Updates #25078
Change-Id: I5a5b14bfc52cb43f25f07ca7d207b61ae9d4f944
Reviewed-on: https://go-review.googlesource.com/109359
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
If X depends on Y and X was installed but Y is only present in the cache
(as happens when you "go install X") then we should report X as up-to-date,
not as stale.
This applies whether X is a package or a main binary.
Fixes#24558.
Fixes#23818.
Change-Id: I26a0b375b1f7f7ac909cc0db68e92f4e04529208
Reviewed-on: https://go-review.googlesource.com/107957
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
We were using absolute paths in the #line directives in the export
header file. This makes the header file change if you move GOPATH.
The absolute paths aren't helpful for the final user, which is some C
program elsewhere.
Fixes#24945
Change-Id: I2da32c9b477df578bd5087435a03fe97abe462e3
Reviewed-on: https://go-review.googlesource.com/108315
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We were using file descriptor 100, which requires the Linux kernel to
grow the fdtable size. That step may sometimes require a long time,
causing the test to fail. Switch to file descriptor 30, which should
not require growing the fdtable.
Fixes#23784
Change-Id: I3ac40d6f8569c70d34b470cfca34eff149bf8229
Reviewed-on: https://go-review.googlesource.com/108537
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Fixes#14327
Much of the code is based on the linux/amd64 code that implements these
build modes, and code is shared where possible.
Change-Id: Ia510f2023768c0edbc863aebc585929ec593b332
Reviewed-on: https://go-review.googlesource.com/93875
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When there are plugins, there may not be a unique copy of runtime
functions like goexit, mcall, etc. So identifying them by entry
address is problematic. Instead, keep track of each special function
using a field in the symbol table. That way, multiple copies of
the same runtime function will be treated identically.
Fixes#24351Fixes#23133
Change-Id: Iea3232df8a6af68509769d9ca618f530cc0f84fd
Reviewed-on: https://go-review.googlesource.com/100739
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
bytes.IndexByte is heavily optimized. Use it in findnull.
This is second attempt, similar to CL97523.
In this version we never call IndexByte on region of memory,
that crosses page boundary. A bit slower than CL97523,
but still fast:
name old time/op new time/op delta
GoString-6 164ns ± 2% 118ns ± 0% -28.00% (p=0.000 n=10+6)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: Id843dd4f65a34309d92bdd8df229e484d26b0cb2
Reviewed-on: https://go-review.googlesource.com/98015
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 7365fac2db.
Reason for revert: breaks the build on some architectures, reading unmapped pages?
Change-Id: I3a8c02dc0b649269faacea79ecd8213defa97c54
Reviewed-on: https://go-review.googlesource.com/97995
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
bytes.IndexByte is heavily optimized.
Use it in findnull.
name old time/op new time/op delta
GoString-8 65.5ns ± 1% 40.2ns ± 1% -38.62% (p=0.000 n=19+19)
findnull is also used in gostringnocopy,
which is used in many hot spots in the runtime.
Fixes#23830
Change-Id: I2e6cb279c7d8078f8844065de684cc3567fe89d7
Reviewed-on: https://go-review.googlesource.com/97523
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Increase the sleep and wait for up to 2 seconds for the dup2.
Apparently it can sometimes take a long time.
Fixes#23784
Change-Id: I929530b057bbcd842b28a7640c39dd68d719ff7d
Reviewed-on: https://go-review.googlesource.com/93895
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This replaces frame size -4/-8 with the NOFRAME flag in mips and
mips64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-[84]/\1|NOFRAME,\2$0/' $(find -name '*_mips*.s')
Plus a manual fix to mkduff.go.
The go binary is identical on both architectures before and after this
change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efb
Reviewed-on: https://go-review.googlesource.com/92044
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -8 with the NOFRAME flag in arm64 assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-8/\1|NOFRAME,\2$0/' $(find -name '*_arm64.s')
Plus a manual fix to mkduff.go.
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227efa
Reviewed-on: https://go-review.googlesource.com/92043
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This replaces frame size -4 with the NOFRAME flag in arm assembly.
This was automated with:
sed -i -e 's/\(^TEXT.*[A-Z]\),\( *\)\$-4/\1|NOFRAME,\2$0/' $(find -name '*_arm.s')
Plus three manual comment changes found by:
grep '\$-4' $(find -name '*_arm.s')
The go binary is identical before and after this change.
Change-Id: I0310384d1a584118c41d1cd3a042bb8ea7227ef9
Reviewed-on: https://go-review.googlesource.com/92042
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Both gcc and clang accept an option -fplugin=code.so to load
a plugin from the ELF shared object file code.so.
Obviously that plugin can then do anything it wants
during the build. This is contrary to the goal of "go get"
never running untrusted code during the build.
(What happens if you choose to run the result of
the build is your responsibility.)
Disallow this behavior by only allowing a small set of
known command-line flags in #cgo CFLAGS directives
(and #cgo LDFLAGS, etc).
The new restrictions can be adjusted by the environment
variables CGO_CFLAGS_ALLOW, CGO_CFLAGS_DISALLOW,
and so on. See the documentation.
In addition to excluding cgo-defined flags, we also have to
make sure that when we pass file names on the command
line, they don't look like flags. So we now refuse to build
packages containing suspicious file names like -x.go.
A wrinkle in all this is that GNU binutils uniformly accept
@foo on the command line to mean "if the file foo exists,
then substitute its contents for @foo in the command line".
So we must also reject @x.go, flags and flag arguments
beginning with @, and so on.
Fixes#23672, CVE-2018-6574.
Change-Id: I59e7c1355155c335a5c5ae0d2cf8fa7aa313940a
Reviewed-on: https://team-review.git.corp.google.com/209949
Reviewed-by: Ian Lance Taylor <iant@google.com>
CL 49490 fixed a warning when compiling the C code generated by cgo,
but it introduced typedef conflicts in Go code that cgo is supposed to
avoid.
Original CL description:
cmd/cgo: fix for function taking pointer typedef
Fixes#19832
Updates #19832Fixes#23720
Change-Id: I22a732db31be0b4f7248c105277ab8ee44ef6cfb
Reviewed-on: https://go-review.googlesource.com/92455
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
The dlopen function returns an opaque handle, and it is possible for
it to look like a Go pointer, causing garbage collector and cgo
confusion.
Fixes#23663
Change-Id: Id080e2bbcee8cfa7ac4a457a927f96949eb913f8
Reviewed-on: https://go-review.googlesource.com/91596
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Introduce GOANDROID_ADB_FLAGS for additional flags to adb invocations.
With GOANDROID_ADG_FLAGS, the Android builders can distinguish between
emulator and device builds.
Change-Id: I11729926a523ee27f6a3795cb2cfb64a9454f0a5
Reviewed-on: https://go-review.googlesource.com/88795
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
The cgo checker was issuing an error with cgocheck=2 when a timer
bucket was stored in a pollDesc. The pollDesc values are allocated
using persistentalloc, so they are not in the Go heap. The code is OK
since timer bucket pointers point into a global array, and as such are
never garbage collected or moved.
Mark timersBucket notinheap to avoid the problem. timersBucket values
only occur in the global timers array.
Fixes#23435
Change-Id: I835f31caafd54cdacc692db5989de63bb49e7697
Reviewed-on: https://go-review.googlesource.com/87637
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
After CL 69831, addTransitiveLinkDeps ensures that all dependencies of
a link appear in Deps. We no longer need to traverse through all
actions to find them. And the old scheme of looking through all the
actions and assuming we would see shared library actions before
libraries they depend on no longer works.
Now that we have complete deps, change to a simpler scheme in which we
find the shared libraries in the deps, and then use that to sort the
deps into archives and shared libraries.
Fixes#22224
Change-Id: I14fcc773ac59b6f5c2965cc04d4ed962442cc89e
Reviewed-on: https://go-review.googlesource.com/87497
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The jobject type is declared as a pointer, but some JVMs
(Dalvik, ART) store non-pointer values in them. In Go, we must
use uintptr instead of a real pointer for these types.
This is similar to the CoreFoundation types on Darwin which
were "fixed" in CL 66332.
Update #22906
Update #21897
RELNOTE=yes
Change-Id: I0d4c664501d89a696c2fb037c995503caabf8911
Reviewed-on: https://go-review.googlesource.com/81876
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixed by CL 76025 yesterday, without realizing it:
the testshared and testplugin builds of separate iface_i
packages were colliding incorrectly in the cache.
Including the build directory fixes that.
Fixes#22571.
Change-Id: Id8193781c67c3150823dc1f48eae781dfe3702fb
Reviewed-on: https://go-review.googlesource.com/76371
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This reverts commit 08f19bbde1.
Reason for revert:
The changed transformation takes effect on a larger set
of code snippets than expected.
For example, this:
func foo() {
// Comment
bar()
}
becomes:
func foo() {
// Comment
bar()
}
This is an unintended consequence.
Change-Id: Ifca88d6267dab8a8170791f7205124712bf8ace8
Reviewed-on: https://go-review.googlesource.com/81335
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Joe Tsai <joetsai@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some C types are declared as pointers, but C code
stores non-pointers in them. When the Go garbage
collector sees such a pointer, it gets unhappy.
Instead, for these types represent them on the Go
side with uintptr.
We need this change to handle Apple's CoreFoundation
CF*Ref types. Users of these types might need to
update their code like we do in root_cgo_darwin.go.
The only change that is required under normal
circumstances is converting some nils to 0.
A go fix module is provided to help.
Fixes#21897
RELNOTE=yes
Change-Id: I9716cfb255dc918792625f42952aa171cd31ec1b
Reviewed-on: https://go-review.googlesource.com/66332
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Permit the C preamble to use the _GoString_ type. Permit Go code to
pass string values directly to those C types. Add accessors for C
code to retrieve sizes and pointers.
Fixes#6907
Change-Id: I190c88319ec88a3ef0ddb99f342a843ba69fcaa3
Reviewed-on: https://go-review.googlesource.com/70890
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Also, with this change, error locations don't print absolute positions
in [] brackets following positions relative to line directives. To get
the absolute positions as well, specify the -L flag.
Fixes#22660.
Change-Id: I9ecfa254f053defba9c802222874155fa12fee2c
Reviewed-on: https://go-review.googlesource.com/77090
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The go repository contains a mix of github.com/golang/go/issues/xxxxx
and golang.org/issues/xxxxx URLs for references to issues in the issue
tracker. We should use one for consistency, and golang.org is preferred
in case the project moves the issue tracker in the future.
This reasoning is taken from a comment Sam Whited left on a CL I
recently opened: https://go-review.googlesource.com/c/go/+/73890.
In that CL I referenced an issue using its github.com URL, because other
tests in the file I was changing contained references to issues using
their github.com URL. Sam Whited left a comment on the CL stating I
should change it to the golang.org URL.
If new code is intended to reference issues via golang.org and not
github.com, existing code should be updated so that precedence exists
for contributors who are looking at the existing code as a guide for the
code they should write.
Change-Id: I3b9053fe38a1c56fc101a8b7fd7b8f310ba29724
Reviewed-on: https://go-review.googlesource.com/75673
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This CL makes "go install" behave the way many users expect:
install only the things named on the command line.
Future builds still run as fast, thanks to the new build cache (CL 75473).
To install dependencies as well (the old behavior), use "go install -i".
Actual definitions aside, what most users know and expect of "go install"
is that (1) it installs what you asked, and (2) it's fast, unlike "go build".
It was fast because it installed dependencies, but installing dependencies
confused users repeatedly (see for example #5065, #6424, #10998, #12329,
"go build" and "go test" so that they could be "fast" too, but that only
created new opportunities for confusion. We also had to add -installsuffix
and then -pkgdir, to allow "fast" even when dependencies could not be
installed in the usual place.
The recent introduction of precise content-based staleness logic means that
the go command detects the need for rebuilding packages more often than it
used to, with the consequence that "go install" rebuilds and reinstalls
dependencies more than it used to. This will create more new opportunities
for confusion and will certainly lead to more issues filed like the ones
listed above.
CL 75743 introduced a build cache, separate from the install locations.
That cache makes all operations equally incremental and fast, whether or
not the operation is "install" or "build", and whether or not "-i" is used.
Installing dependencies is no longer necessary for speed, it has confused
users in the past, and the more accurate rebuilds mean that it will confuse
users even more often in the future. This CL aims to end all that confusion
by not installing dependencies by default.
By analogy with "go build -i" and "go test -i", which still install
dependencies, this CL introduces "go install -i", which installs
dependencies in addition to the things named on the command line.
Fixes#5065.
Fixes#6424.
Fixes#10998.
Fixes#12329.
Fixes#18981.
Fixes#22469.
Another step toward #4719.
Change-Id: I3d7bc145c3a680e2f26416e182fa0dcf1e2a15e5
Reviewed-on: https://go-review.googlesource.com/75850
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This CL adds an automatic, limited "go vet" to "go test".
If the building of a test package fails, vet is not run.
If vet fails, the test is not run.
The goal is that users don't notice vet as part of the "go test"
process at all, until vet speaks up and says something important.
This should help users find real problems in their code faster
(vet can just point to them instead of needing to debug a
test failure) and expands the scope of what kinds of things
vet can help with.
The "go vet" runs in parallel with the linking of the test binary,
so for incremental builds it typically does not slow the overall
"go test" at all: there's spare machine capacity during the link.
all.bash has less spare machine capacity. This CL increases
the time for all.bash on my laptop from 4m41s to 4m48s (+2.5%)
To opt out for a given run, use "go test -vet=off".
The vet checks used during "go test" are a subset of the full set,
restricted to ones that are 100% correct and therefore acceptable
to make mandatory. In this CL, that set is atomic, bool, buildtags,
nilfunc, and printf. Including printf is debatable, but I want to
include it for now and find out what needs to be scaled back.
(It already found one real problem in package os's tests that
previous go vet os had not turned up.)
Now that we can rely on type information it may be that printf
should make its function-name-based heuristic less aggressive
and have a whitelist of known print/printf functions.
Determining the exact set for Go 1.10 is #18085.
Running vet also means that programs now have to type-check
with both cmd/compile and go/types in order to pass "go test".
We don't start vet until cmd/compile has built the test package,
so normally the added go/types check doesn't find anything.
However, there is at least one instance where go/types is more
precise than cmd/compile: declared and not used errors involving
variables captured into closures.
This CL includes a printf fix to os/os_test.go and many declared
and not used fixes in the race detector tests.
Fixes#18084.
Change-Id: I353e00b9d1f9fec540c7557db5653e7501f5e1c9
Reviewed-on: https://go-review.googlesource.com/74356
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This CL adds caching of successful test results, keyed by the
action ID of the test binary and its command line arguments.
Suppose you run:
go test -short std
<edit a typo in a comment in math/big/float.go>
go test -short std
Before this CL, the second go test would re-run all the tests
for the std packages. Now, the second go test will use the cached
result immediately (without any compile or link steps) for any
packages that do not transitively import math/big, and then
it will, after compiling math/big and seeing that the .a file didn't
change, reuse the cached test results for the remaining packages
without any additional compile or link steps.
Suppose that instead of editing a typo you made a substantive
change to one function, but you left the others (including their
line numbers) unchanged. Then the second go test will re-link
any of the tests that transitively depend on math/big, but it still
will not re-run the tests, because the link will result in the same
test binary as the first run.
The only cacheable test arguments are:
-cpu
-list
-parallel
-run
-short
-v
Using any other test flag disables the cache for that run.
The suggested argument to mean "turn off the cache" is -count=1
(asking "please run this 1 time, not 0").
There's an open question about re-running tests when inputs
like environment variables and input files change. For now we
will assume that users will bypass the test cache when they
need to do so, using -count=1 or "go test" with no arguments.
This CL documents the new cache but also documents the
previously-undocumented distinction between "go test" with
no arguments (now called "local directory mode") and with
arguments (now called "package list mode"). It also cleans up
a minor detail of package list mode buffering that used to change
whether test binary stderr was sent to go command stderr based
on details like exactly how many packages were listed or
how many CPUs the host system had. Clearly the file descriptor
receiving output should not depend on those, so package list mode
now consistently merges all output to stdout, where before it
mostly did that but not always.
Fixes#11193.
Change-Id: I120edef347b9ddd5b10e247bfd5bd768db9c2182
Reviewed-on: https://go-review.googlesource.com/75631
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
To improve readability when exported fields are removed,
forbid the printer from emitting an empty line before the first comment
in a const, var, or type block.
Also, when printing the "Has filtered or unexported fields." message,
add an empty line before it to separate the message from the struct
or interfact contents.
Before the change:
<<<
type NamedArg struct {
// Name is the name of the parameter placeholder.
//
// If empty, the ordinal position in the argument list will be
// used.
//
// Name must omit any symbol prefix.
Name string
// Value is the value of the parameter.
// It may be assigned the same value types as the query
// arguments.
Value interface{}
// contains filtered or unexported fields
}
>>>
After the change:
<<<
type NamedArg struct {
// Name is the name of the parameter placeholder.
//
// If empty, the ordinal position in the argument list will be
// used.
//
// Name must omit any symbol prefix.
Name string
// Value is the value of the parameter.
// It may be assigned the same value types as the query
// arguments.
Value interface{}
// contains filtered or unexported fields
}
>>>
Fixes#18264
Change-Id: I9fe17ca39cf92fcdfea55064bd2eaa784ce48c88
Reviewed-on: https://go-review.googlesource.com/71990
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
This cuts 6 seconds off all.bash with the new go command.
Not a ton, but also an easy 6 seconds to grab.
The -tags=use_go_run in the misc/cgo tests is just some
go command flag that will make run.go use go run,
but without making everything look stale.
(Those tests have relative imports,
so go tool compile+link is not enough.)
Change-Id: I43bf4bb661d3adde2b2d4aad5e8f64b97bc69ba9
Reviewed-on: https://go-review.googlesource.com/73994
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This CL changes the go command to base all its rebuilding decisions
on the content of the files being processed and not their file system
modification times. It also eliminates the special handling of release
toolchains, which were previously considered always up-to-date
because modification time order could not be trusted when unpacking
a pre-built release.
The go command previously tracked "build IDs" as a backup to
modification times, to catch changes not reflected in modification times.
For example, if you remove one .go file in a package with multiple .go
files, there is no modification time remaining in the system that indicates
that the installed package is out of date. The old build ID was the hash
of a list of file names and a few other factors, expected to change if
those factors changed.
This CL moves to using this kind of build ID as the only way to
detect staleness, making sure that the build ID hash includes all
possible factors that need to influence the rebuild decision.
One such factor is the compiler flags. As of this CL, if you run
go build -gcflags -N cmd/gofmt
you will get a gofmt where every package is built with -N,
regardless of what may or may not be installed already.
Another such factor is the linker flags. As of this CL, if you run
go install myprog
go install -ldflags=-s myprog
the second go install will now correctly build a new myprog with
the updated linker flags. (Previously the installed myprog appeared
up-to-date, because the ldflags were not included in the build ID.)
Because we have more precise information we can also validate whether
the target of a "go test -c" operation is already the right binary and
therefore can avoid a rebuild.
This CL sets us up for having a more general build artifact cache,
maybe even a step toward not having a pkg directory with .a files,
but this CL does not take that step. For now the result of go install
is the same as it ever was; we just do a better job of what needs to
be installed.
This CL does slow down builds a small amount by reading all the
dependent source files in full. (The go command already read the
beginning of every dependent source file to discover build tags
and imports.) On my MacBook Pro, before this CL all.bash takes
3m58s, while after this CL and a few optimizations stacked above it
all.bash takes 4m28s. Given that CL 73850 cut 1m43s off the all.bash
time earlier today, we can afford adding 30s back for now.
More optimizations are planned that should make the go command
more efficient than it was even before this CL.
Fixes#15799.
Fixes#18369.
Fixes#19340.
Fixes#21477.
Change-Id: I10d7ca0e31ca3f58aabb9b1f11e2e3d9d18f0bc9
Reviewed-on: https://go-review.googlesource.com/73212
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The upcoming CL 73212 will see through mtime modifications.
Change the underlying file too.
Change-Id: Ib23b4136a62ee87bce408b76bb0385451ae7dcd2
Reviewed-on: https://go-review.googlesource.com/74130
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Both the linker and the plugin package were inconsistent
about when they applied the path encoding defined in
objabi.PathToPrefix. As a result, only some symbols from
a package path that required encoding were being found.
So always encoding the path.
Fixes#22295
Change-Id: Ife86c79ca20b2e9307008ed83885e193d32b7dc4
Reviewed-on: https://go-review.googlesource.com/72390
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Running test.bash goes from 30s to 10s on a linux workstation.
(The coming pkg cache work in cmd/go would presumably do the same thing,
but this makes all.bash faster today.)
Change-Id: I8c9b0400071a412fce55b386e939906bb1c1d84d
Reviewed-on: https://go-review.googlesource.com/72330
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently we look to see if the main.main symbol address is in the
module data text range. This requires access to the main.main
symbol, which usually the runtime has, but does not when building
a plugin.
To avoid a dynamic relocation to main.main (which I haven't worked
out how to have the linker generate on darwin), stop using the
symbol. Instead record a boolean in the moduledata if the module
has the main function.
Fixes#22175
Change-Id: If313a118f17ab499d0a760bbc2519771ed654530
Reviewed-on: https://go-review.googlesource.com/69370
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Starting in gcc 6, -pie is passed to the linker by default
on some platforms, including ppc64le. If the objects
being linked are not built for -pie then in some cases the
executable could be in error. To avoid that problem, -no-pie
should be used with gcc to override the default -pie option
and generate a correct executable that can be run without error.
Fixes#22126
Change-Id: I4a052bba8b9b3bd6706f5d27ca9a7cebcb504c95
Reviewed-on: https://go-review.googlesource.com/70072
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Everything got a bit tangled together in b.action1.
Try to tease things apart again.
In general this is a refactoring of the existing code, with limited
changes to the effect of the code.
The main additional change is to complete a.Deps for link actions.
That list now directly contains all the inputs the linker will attempt
to read, eliminating the need for a transitive traversal of the entire
action graph to find those. The comepleteness of a.Deps will be
important when we eventually use it to decide whether an cached
action output can be reused.
all.bash passes, but it's possible I've broken some subtety of
buildmode=shared again. Certainly that code took the longest
to get working.
Change-Id: I34e849eda446dca45a9cfce02b07bec6edb6d0d4
Reviewed-on: https://go-review.googlesource.com/69831
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Updates #11058
Change-Id: I2a8bf4403b680ab8bf06fff18291f3bf67261e27
Reviewed-on: https://go-review.googlesource.com/69090
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Otherwise we end up with testp?.exe files after the tests run.
Updates #11058
Change-Id: Ieccfc42da6192622bdab1f9a411ccd50bb59fd5b
Reviewed-on: https://go-review.googlesource.com/68770
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestUnexportedSymbols requires dup2 that
my gcc installation does not have.
TestSignalHandlersWithNotify fails with:
undefined: syscall.SIGIO.
TestSignalHandlers fails with:
sched.h: No such file or directory.
TestExportedSymbolsWithDynamicLoad fails with:
dlfcn.h: No such file or directory.
Also add t.Helper calls to better error messages.
Updates #11058
Change-Id: I7eb514968464256b8337e45f57fcb7d7fe0e4693
Reviewed-on: https://go-review.googlesource.com/68410
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Moves type symbol name mangling out of the object reader
and into a separate pass. Requires some care, as changing
the name of a type may require dealing with duplicate
symbols for the first time.
Disables DWARF for both plugins and programs that use plugin.Open,
because type manging is currently incompatible with the go.info.*
symbol generator in cmd/link. (It relies on the symbol names to
find type information.) A future fix for this would be moving the
go.info.* generation into the compiler, with the logic we use
for generating the type.* symbols.
Fixes#19529
Change-Id: I75615f8bdda86ff9e767e536d9aa36e15c194098
Reviewed-on: https://go-review.googlesource.com/67312
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is a step toward using cached build artifacts: the importcfg
will direct the compiler and linker to read them right from the cache
if necessary. However, this CL does not have a cache yet, so it still
reads them from the usual install location or build location.
Even so, this fixes a long-standing issue that -I and -L (no longer used)
are not expressive enough to describe complex GOPATH setups.
Shared libraries are handled enough that all.bash passes, but
there may still be more work to do here. If so, tests and fixes
can be added in follow-up CLs.
Gccgo will need updating to support -importcfg as well.
Fixes#14271.
Change-Id: I5c52a0a5df0ffbf7436e1130c74e9e24fceff80f
Reviewed-on: https://go-review.googlesource.com/56279
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When rewriting loads and stores accessing global variables to use the
GOT we were making use of REGTMP (R10). Unfortunately loads and stores
with large offsets (larger than 20-bits) were also using REGTMP,
causing it to be clobbered and subsequently a segmentation fault.
This can be fixed by using REGTMP2 (R11) for the rewrite. This is fine
because REGTMP2 only has a couple of uses in the assembler (division,
high multiplication and storage-to-storage instructions). We didn't
use REGTMP2 originally because it used to be used more frequently,
in particular for stores of constants to memory. However we have now
eliminated those uses.
This was found while writing a test case for CL 63030. That test case
is included in this CL.
Change-Id: I13956f1f3ca258a7c8a7ff0a7570d2848adf7f68
Reviewed-on: https://go-review.googlesource.com/65011
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It's not needed, and the current expectation is that it will go away
in the future.
Change-Id: I5f46800e748d9ffa484bda6d1738290c8e00ac2b
Reviewed-on: https://go-review.googlesource.com/63751
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
rune has a well-defined size, but C.int is implementation-specified.
Using one as the other should require an explicit conversion.
updates #13467
Change-Id: I53ab2478427dca790efdcc197f6b8d9fbfbd1847
Reviewed-on: https://go-review.googlesource.com/63730
Reviewed-by: Ian Lance Taylor <iant@golang.org>
I had passed 1 instead of 2 to the SplitAfterN call in
errorstest.check, so all of the cases were erroneously falling through
to the non-regexp case (and passing even if the actual error didn't
match).
Now, we use bytes.HasSuffix to check for the non-regexp case, so we
will not incorrectly match a regexp comment to the non-regexp case.
updates #13467
Change-Id: Ia6be928a495425f2b7bae5001bd01346e115dcfa
Reviewed-on: https://go-review.googlesource.com/63692
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This makes the test easier to run in isolation and easier to change,
and simplifies the code to run the tests in parallel.
updates #13467
Change-Id: I5622b5cc98276970347da18e95d071dbca3c5cc1
Reviewed-on: https://go-review.googlesource.com/63276
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Previously, test7978 failed if the user did not invoke it with
GOTRACEBACK=2 already set in their environment. Environment-sensitive
test are awkward, and in this case there is a very simple workaround:
set the traceback level to the necessary value explicitly.
Change-Id: I7d576f24138aa8a41392148eae11bbeaef558573
Reviewed-on: https://go-review.googlesource.com/63275
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
CL 62593 broken TestExportedSymbols and TestUnexportedSymbols
because it started executing android test binary on host.
Make them run on android.
Hopefully fixes android build.
Change-Id: Ic0bb9f0cbbefca23828574282caa33a03ef72431
Reviewed-on: https://go-review.googlesource.com/62830
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Elias Naur <elias.naur@gmail.com>
Change-Id: Ib35bb7fc9c5b4ccc9b8e1bd16443e0b307be9406
Reviewed-on: https://go-review.googlesource.com/62593
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fixes#21809
Change-Id: Ic43077c6bea3c7cdc9611e74abf07b6deab70433
Reviewed-on: https://go-review.googlesource.com/62670
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: Id1b5939cfcd210a0cb5f61915ce2d077c7fcec11
Reviewed-on: https://go-review.googlesource.com/62592
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The noopt builder sets GO_GCFLAGS when building the standard library.
Set it when building plugins to ensure the -shared packages built for it
have the same inlining in the export data (and thus the same package
version).
Tested locally with GO_GCFLAGS="-N -l" ./all.bash
Fixes#17937
Change-Id: Id037cfbf4af744c05c47bdc58eea60a5dba69533
Reviewed-on: https://go-review.googlesource.com/62511
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
1. remove broken verification
The runtime check assumes that no-pcln symbol entry have zero value,
but the linker emit no entries if the symbol is no-pcln.
As a result, if there are no-pcln symbols at the very end of pcln
table, it will panic.
2. correct condition of export
Handle special chracters in pluginpath correcty.
Export "go.itab.*", so different plugins can share the same itab.
Fixes#18190
Change-Id: Ia4f9c51d83ce8488a9470520f1ee9432802cfc1d
Reviewed-on: https://go-review.googlesource.com/61091
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Along the way, track bad modules. Make sure they don't end up on
the active modules list, and aren't accidentally reprocessed as
new plugins.
Fixes#19004
Change-Id: I8a5e7bb11f572f7b657a97d521a7f84822a35c07
Reviewed-on: https://go-review.googlesource.com/61171
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is common to have multiple plugins built from ephemeral
source files all with the same name:
# generate main.go
go build -buildmode=plugin -o=p1.so main.go
# rm main.go, generate new main.go
go build -buildmode=plugin -o=p2.so main.go
...
These different plugins currently have the same build ID,
and hence the same package path. This means only one can be
loaded.
To remove this restriction, this commit adds the contents of the
main package source files to the plugin hash.
Fixes#19358
Change-Id: Icd42024b085feb29c09c2771aaecb85f8b528dd3
Reviewed-on: https://go-review.googlesource.com/61170
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When compiling a plugin, package main gets a new name so as not to
conflict with the main package in the host binary, or any other
plugins. It is already defined by cmd/go, and used by cmd/link when
filling out the "" package placeholder in symbols.
With this CL, the plugin-specific name for main is also passed to
cmd/compile's -p flag. This is used to fill out the pkgpath field
of types, and ensures that two types defined in two different plugin
mains with the same name will not be mistaken for one another at
runtime.
Fixes#21386
Change-Id: I8a646d8d7451caff533fe0007343ea8b8e1704ed
Reviewed-on: https://go-review.googlesource.com/60910
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The approach of https://golang.org/cl/43476 turned out incorrect.
The problem is that the sniff introduced by the CL only work for simple
expression. And when it fails it fallback to uint64, not int64, which
breaks backward compatibility.
In this CL, we use DWARF for guessing kind instead. That should be more
reliable than previous approach. And importanly, it fallbacks to int64 even
if it fails to guess kind.
Fixes#21708
Change-Id: I39a18cb2efbe4faa9becdcf53d5ac68dba180d46
Reviewed-on: https://go-review.googlesource.com/60510
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently, cgo supports only macros which can be reduced to constants
or variables. The CL addresses remaining parts, macros which can be
represented as niladic functions.
The basic idea is simple:
1. make a thin wrapper function per macros.
2. replace macro expansions with function calls.
Fixes#10715Fixes#18720
Change-Id: I150b4fb48e9dc4cc34466ef6417c04ac93d4bc1a
Reviewed-on: https://go-review.googlesource.com/43970
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Current code uses names like "x" and "s" which can conflict with user's
code easily. Use cryptographic names.
Fixes#21668
Change-Id: Ib6d3d6327aa5b92d95c71503d42e3a79d96c8e15
Reviewed-on: https://go-review.googlesource.com/59710
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit a6ffab6b67.
Reason for revert: with CL 57290 the tests run on Android again.
Change-Id: Ifeb29762a4cd0178463acfeeb3696884d99d2993
Reviewed-on: https://go-review.googlesource.com/57310
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
The testcshared test.bash was rewritten in Go, but the rewritten script
broke on Android. Make the tests run on Android again by:
- Restoring the LD_LIBRARY_PATH path (.).
- Restoring the Android specific C flags (-pie -fuse-ld=gold).
- Adding runExe to run test executables. All other commands must run on
the host.
Fixes#21513.
Change-Id: I3ea617a943c686b15437cc5c118e9802a913d93a
Reviewed-on: https://go-review.googlesource.com/57290
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Hopefully this will fix android build.
Maybe fixes#21513
Change-Id: I98f760562646f06b56e385c36927e79458465b92
Reviewed-on: https://go-review.googlesource.com/56790
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Another attempt to fix build
Change-Id: I26137c115ad4b5f5a69801ed981c146adf6e824c
Reviewed-on: https://go-review.googlesource.com/56750
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Hopefully fixes build.
Change-Id: If0629b95b923a65e4507073cf7aa44a5e178fc0f
Reviewed-on: https://go-review.googlesource.com/56711
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
This makes it much easier to run individual failing subtests.
Use $(go env CC) instead of always defaulting to clang; this makes it
easier to test with other compilers.
Run C binaries to detect incompatible compiler/kernel pairings instead
of sniffing versions.
updates #21196
Change-Id: I0debb3cc4a4244df44b825157ffdc97b5c09338d
Reviewed-on: https://go-review.googlesource.com/52910
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
When calling a Go function that returns multiple values from C, cgo
generates a structure to hold the values. According to the documentation
this structure is called `struct <function-name>_return`. When compiling
for gccgo the generated structure name is `struct <function-name>_result`.
This change updates the output for gccgo to match the documentation and
output for gc.
Fixes#20910
Change-Id: Iaea8030a695a7aaf9d9f317447fc05615d8e4adc
Reviewed-on: https://go-review.googlesource.com/49350
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Otherwise, some test flags don't work.
Change-Id: Iacf3930d0eec28e4d690cd382adbb2ecf866a0e2
Reviewed-on: https://go-review.googlesource.com/55615
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Just like https://golang.org/cl/34783
Given cgo.go:
1 package main
2
3 /*
4 long double x = 0;
5 */
6 import "C"
7
8 func main() {
9 _ = C.x
10 _ = C.x
11 }
Before:
./cgo.go:10:6: unexpected: 16-byte float type - long double
After:
./cgo.go:9:6: unexpected: 16-byte float type - long double
The above test case is not portable. So it is tested on only amd64.
Change-Id: If0b84cf73d381a22e2ada71c8e9a6e6ec77ffd2e
Reviewed-on: https://go-review.googlesource.com/54950
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The activeModules function is called by the cgo pointer checking code,
which is called by the write barrier (when GODEBUG=cgocheck=2), and as
such must be nosplit/nowritebarrier.
Fixes#21306
Change-Id: I57f2124f14de7f3872b2de9532abab15df95d45a
Reviewed-on: https://go-review.googlesource.com/53352
Reviewed-by: Austin Clements <austin@google.com>
Check not only that a tsan program can be built, but also that it runs.
This fails with some installations of GCC 7.
Skip the tsan10 program when using GCC, as it reportedly hangs.
This is a patch to help people build 1.9; we may be able to do a
better fix for 1.10.
Updates #21196
Change-Id: Icd1ffbd018dc65a97ff45cab1264b9b0c7fa0ab2
Reviewed-on: https://go-review.googlesource.com/52790
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
http://golang.org/cl/50251 fixed a regression under TSAN.
This change adds a minimal reproducer for the observed symptom.
Change-Id: Ib9ad01b458b7fdec14d6c2fe3c243f9c64b3dcf2
Reviewed-on: https://go-review.googlesource.com/50371
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
clang can emit some dwarf.VoidType which are wrapped by multiple
dwarf.TypedefType. We need to unwrap those before further processing.
Fixes#20129
Change-Id: I671ce6aef2dc7b55f1a02aec5f9789ac1b369643
Reviewed-on: https://go-review.googlesource.com/44772
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
With current GCC a macro that refers to another macro can report an
error on the macro definition line, with a note on the use.
When cgo is trying to decide which line an error refers to,
it is looking at the uses. So if we see an error on a line that we
don't recognize followed by a note on a line that we do recognize,
treat the note as an error.
Fixes#20125.
Change-Id: I389cd0eb7d56ad2d54bef70e278d9f76c4d36448
Reviewed-on: https://go-review.googlesource.com/44290
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Hiroshi Ioka <hirochachacha@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
For test.go:
package main
import (
"C"
"fmt"
)
func main() {
fmt.Println("Hello, world!")
C.no_such_f()
}
Before:
could not determine kind of name for C.no_such_f
After:
./test.go:10:2: could not determine kind of name for C.no_such_f
Fixes#18452
Change-Id: I49c136b7fa60fab25d2d5b905d440fe4d106e565
Reviewed-on: https://go-review.googlesource.com/34783
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Current code cannot handle string #define macros if those macros are
defined via other macros. This CL solve the issue.
Updates #18720
Change-Id: Ibed0773d10db3d545bb246b97e81c0d19e3af3d5
Reviewed-on: https://go-review.googlesource.com/41312
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, cgo converts integer macros into int64 if it's possible.
As a result, some macros which satisfy
math.MaxInt64 < x <= math.MaxUint64
will lose their original values.
This CL introduces the new probe to check signs,
so we can handle signed ints and unsigned ints separately.
Fixes#20369
Change-Id: I002ba452a82514b3a87440960473676f842cc9ee
Reviewed-on: https://go-review.googlesource.com/43476
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In PPC64 ELF files, the st_other field indicates the number of
prologue instructions between the global and local entry points.
We add the instructions in the compiler and assembler if -shared is used.
We were assuming that the instructions were present when building a
c-archive or PIE or doing dynamic linking, on the assumption that those
are the cases where the go tool would be building with -shared.
That assumption fails when using some other tool, such as Bazel,
that does not necessarily use -shared in exactly the same way.
This CL records in the object file whether a symbol was compiled
with -shared (this will be the same for all symbols in a given compilation)
and uses that information when setting the st_other field.
Fixes#20290.
Change-Id: Ib2b77e16aef38824871102e3c244fcf04a86c6ea
Reviewed-on: https://go-review.googlesource.com/43051
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Here we restrict using cgo builtin references because internally they're go functions
as opposed to C usafe.Pointer values.
Fixes#18889
Change-Id: I1e4332e4884063ccbaf9772c172d4462ec8f3d13
Reviewed-on: https://go-review.googlesource.com/40934
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Previously the "ABI hash" for a package (used to determine if a loaded shared
library has the ABI expected by its loader) was the hash of the entire
__.PKGDEF file. But that means it depends on the build ID generated by the go
tool for the package, which means that if a file is added (even a .c or .h
file!) to the package, the ABI changes, perhaps uncessarily.
Fixes#19920
Change-Id: If919481e1a03afb350c8a9c7a0666bb90ee90270
Reviewed-on: https://go-review.googlesource.com/40401
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Prevent a crash if the same type in two plugins had a recursive
definition, either by referring to a pointer to itself or a map existing
with the type as a value type (which creates a recursive definition
through the overflow bucket type).
Fixes#19258
Change-Id: Iac1cbda4c5b6e8edd5e6859a4d5da3bad539a9c6
Reviewed-on: https://go-review.googlesource.com/40292
Run-TryBot: Todd Neal <todd@tneal.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The hardware divider is an optional component of ARMv7. This patch
detects whether it is available in runtime and use it or not.
1. The hardware divider is detected at startup and a flag is set/clear
according to a perticular bit of runtime.hwcap.
2. Each call of runtime.udiv will check this flag and decide if
use the hardware division instruction.
A rough test shows the performance improves 40-50% for ARMv7. And
the compatibility of ARMv5/v6 is not broken.
fixes#19118
Change-Id: Ic586bc9659ebc169553ca2004d2bdb721df823ac
Reviewed-on: https://go-review.googlesource.com/37496
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Current code doesn't support floating point #define macros.
This CL compiles floats to a object file and retrive values from it.
That approach is the same work as we've already done for integers.
Updates #18720
Change-Id: I88b7ab174d0f73bda975cf90c5aeb797961fe034
Reviewed-on: https://go-review.googlesource.com/35511
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Without this, the load fails during kernel exec, which results in the
mysterious and completely uninformative "Killed: 9" error.
It appears that the stars (or at least the inputs) were properly aligned
with earlier versions of Xcode so that this happened accidentally.
Make it happen on purpose.
Gregory Man bisected the breakage to this change in LLVM,
which fits the theory nicely:
https://github.com/llvm-mirror/llvm/commit/9a41e59cFixes#19734.
Change-Id: Ice67a09af2de29d3c0d5e3fcde6a769580897c95
Reviewed-on: https://go-review.googlesource.com/38854
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The gold linker is used by default in the Android NDK, except on
arm64:
https://github.com/android-ndk/ndk/issues/148
The Go linker already forces the use of the gold linker on arm and
arm64 (CL 22141) for other reasons. However, the test.bash script in
testcshared doesn't, resulting in linker errors on android/arm64:
warning: liblog.so, needed by ./libgo.so, not found (try using -rpath or
-rpath-link)
Add -fuse-ld=gold when running testcshared on Android. Fixes the
android/arm64 builder.
Change-Id: I35ca96f01f136bae72bec56d71b7ca3f344df1ed
Reviewed-on: https://go-review.googlesource.com/38832
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
POSIX Shell only supports = to compare variables inside '[' tests. But
this is Bash, where == is an alias for =. In practice they're the same,
but the current form is inconsisnent and breaks POSIX for no good
reason.
Change-Id: I38fa7a5a90658dc51acc2acd143049e510424ed8
Reviewed-on: https://go-review.googlesource.com/38031
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Compiler errors now show the exact line and line byte offset (sometimes
called "column") of where an error occured. For `go tool compile x.go`:
package p
const c int = false
//line foo.go:123
type t intg
reports
x.go:2:7: cannot convert false to type int
foo.go:123[x.go:4:8]: undefined: intg
(Some errors use the "wrong" position for the error message; arguably
the byte offset for the first error should be 15, the position of 'false',
rathen than 7, the position of 'c'. But that is an indepedent issue.)
The byte offset (column) values are measured in bytes; they start at 1,
matching the convention used by editors and IDEs.
Positions modified by //line directives show the line offset only for the
actual source location (in square brackets), not for the "virtual" file and
line number because that code is likely generated and the //line directive
only provides line information.
Because the new format might break existing tools or scripts, printing
of line offsets can be disabled with the new compiler flag -C. We plan
to remove this flag eventually.
Fixes#10324.
Change-Id: I493f5ee6e78457cf7b00025aba6b6e28e50bb740
Reviewed-on: https://go-review.googlesource.com/37970
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A typo in the previous revision ("act" instead of "oldact") caused us
to return the sa_flags from the new (or zeroed) sigaction rather than
the old one.
In the presence of a signal handler registered before
runtime.libpreinit, this caused setsigstack to erroneously zero out
important sa_flags (such as SA_SIGINFO) in its attempt to re-register
the existing handler with SA_ONSTACK.
Change-Id: I3cd5152a38ec0d44ae611f183bc1651d65b8a115
Reviewed-on: https://go-review.googlesource.com/37852
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
There are a few problems from change 35494, discovered during testing
of change 37852.
1. I was confused about the usage of n.key in the sema variant, so we
were looping on the wrong condition. The error was not caught by
the TryBots (presumably due to missing TSAN coverage in the BSD and
darwin builders?).
2. The sysmon goroutine sometimes skips notetsleep entirely, using
direct usleep syscalls instead. In that case, we were not calling
_cgo_yield, leading to missed signals under TSAN.
3. Some notetsleep calls have long finite timeouts. They should be
broken up into smaller chunks with a yield at the end of each
chunk.
updates #18717
Change-Id: I91175af5dea3857deebc686f51a8a40f9d690bcc
Reviewed-on: https://go-review.googlesource.com/37867
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
write(2) is defined in unistd.h.
For the iOS builder.
Change-Id: I411ffe81988d8fbafffde89e4732a20af1a63325
Reviewed-on: https://go-review.googlesource.com/37962
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This ensures that SIGPROF is handled correctly when using
runtime/pprof in a c-archive or c-shared library.
Separate profiler handling into pre-process changes and per-thread
changes. Simplify the Windows code slightly accordingly.
Fixes#18220.
Change-Id: I5060f7084c91ef0bbe797848978bdc527c312777
Reviewed-on: https://go-review.googlesource.com/34018
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Before this CL, Go programs in c-archive or c-shared buildmodes
would not handle SIGPIPE. That leads to surprising behaviour where
writes on a closed pipe or socket would raise SIGPIPE and terminate
the program. This CL changes the Go runtime to handle
SIGPIPE regardless of buildmode. In addition, SIGPIPE from non-Go
code is forwarded.
This is a refinement of CL 32796 that fixes the case where a non-default
handler for SIGPIPE is installed by the host C program.
Fixes#17393
Change-Id: Ia41186e52c1ac209d0a594bae9904166ae7df7de
Reviewed-on: https://go-review.googlesource.com/35960
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TestMain doesn't make use of any flags.
Change-Id: I98ec582fb004045a5067618f605ccfeb1f9f4bbb
Reviewed-on: https://go-review.googlesource.com/33613
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Modules appear in the moduledata linked list in the order they are
loaded by the dynamic loader, with one exception: the
firstmoduledata itself the module that contains the runtime.
This is not always the first module (when using -buildmode=shared,
it is typically libstd.so, the second module).
The order matters for typelinksinit, so we swap the first module
with whatever module contains the main function.
Updates #18729
This fixes the test case extracted with -linkshared, and now
go test -linkshared encoding/...
passes. However the original issue about a plugin failure is not
yet fixed.
Change-Id: I9f399ecc3518e22e6b0a350358e90b0baa44ac96
Reviewed-on: https://go-review.googlesource.com/35644
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This is needed for typical tests with gccgo, as it passes the
LD_LIBRARY_PATH environment variable to the new program.
Change-Id: I9bf4b0dbdff63f5449c7fcb8124eaeab10ed7f34
Reviewed-on: https://go-review.googlesource.com/35481
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
With GCC 7 (not yet released), cgo fails with errors like
./sigaltstack.go:65:8: call of non-function C.restoreSignalStack
I do not know precisely why. Explicitly declaring that there are no
arguments to the static function is a simple fix for the debug info.
Change-Id: Id96e1cb1e55ee37a9f1f5ad243d7ee33e71584ac
Reviewed-on: https://go-review.googlesource.com/35480
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We already do this for shared libraries. Do it for plugins also.
Suggestions on how to test this would be welcome.
I'd like to get this in for 1.8. It could lead to mysterious
hangs when using plugins.
Fixes#18676
Change-Id: I03209b096149090b9ba171c834c5e59087ed0f92
Reviewed-on: https://go-review.googlesource.com/35117
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Make sure that the same type and itab generated in two
different plugins are actually the same thing.
See also CL 35115
Change-Id: I0c1ecb039d7e2bf5a601d58dfa162a435ae4ef76
Reviewed-on: https://go-review.googlesource.com/35116
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Make sure that the same type and itab generated in two
different shared library are actually the same thing.
Change-Id: Ica45862d65ff8bc7ad04d59a41f57223f71224cd
Reviewed-on: https://go-review.googlesource.com/35115
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Wait longer in case the system is heavily loaded.
Fixes#18324.
Change-Id: If9a6da1cf32d0321302d244ee24fb3f80e54489d
Reviewed-on: https://go-review.googlesource.com/34653
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
golang.org/issue/17594 was caused by additab being called more than once for
an itab. golang.org/cl/32131 fixed that by making the itabs local symbols,
but that in turn causes golang.org/issue/18252 because now there are now
multiple itab symbols in a process for a given (type,interface) pair and
different code paths can end up referring to different itabs which breaks
lots of reflection stuff. So this makes itabs global again and just takes
care to only call additab once for each itab.
Fixes#18252
Change-Id: I781a193e2f8dd80af145a3a971f6a25537f633ea
Reviewed-on: https://go-review.googlesource.com/34173
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Confirm that a trivial executable can build and execute using
-fsanitize=memory.
Fixes#18335 (by skipping the tests when they don't work).
Change-Id: Icb7a276ba7b57ea3ce31be36f74352cc68dc89d5
Reviewed-on: https://go-review.googlesource.com/34505
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This fixes Linux and the *BSD platforms on 386/amd64.
A few OS/arch combinations were already saving registers and/or doing
something that doesn't clearly resemble the SysV C ABI; those have
been left alone.
Fixes#18328.
Change-Id: I6398f6c71020de108fc8b26ca5946f0ba0258667
Reviewed-on: https://go-review.googlesource.com/34501
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Explicitly filter any C-only cgo functions out of pclntable,
which allows them to be duplicated with the host binary.
Updates #18190.
Change-Id: I50d8706777a6133b3e95f696bc0bc586b84faa9e
Reviewed-on: https://go-review.googlesource.com/34199
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that we try to handle qualifiers correctly (as of CL 33325), don't
strip them from a void* pointer. Otherwise we break a case like "const
void**", as the "const" qualifier is dropped and the resulting
"void**" triggers a warning from the C compiler.
Fixes#18298.
Change-Id: If51df1889b0f6a907715298c152e6d4584747acb
Reviewed-on: https://go-review.googlesource.com/34370
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also, if we changed the gsignal stack to match the stack we are
executing on, restore it when returning from the signal handler, for
safety.
Fixes#18255.
Change-Id: Ic289b36e4e38a56f8a6d4b5d74f68121c242e81a
Reviewed-on: https://go-review.googlesource.com/34239
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
The pclntable contains pointers to functions. If the function symbol
is exported in a plugin, and there is a matching symbol in the host
binary, then the pclntable of a plugin ends up pointing at the
function in the host module.
This doesn't work because the traceback code expects the pointer to
be in the same module space as the PC value.
So don't export functions that might overlap with the host binary.
This way the pointer stays in its module.
Updates #18190
Change-Id: Ifb77605b35fb0a1e7edeecfd22b1e335ed4bb392
Reviewed-on: https://go-review.googlesource.com/34196
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
It is reported as failing for two people (issues #18202 and #18212).
The failure mode is that the system gets overloaded and other programs
fail to run.
Fixes#18202.
Change-Id: I1f1ca1f5d8eed6cc3a9dffac3289851e09fa662b
Reviewed-on: https://go-review.googlesource.com/34017
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For reasons that I do not know, OpenBSD does not call pthread_create
directly, but instead looks it up in libpthread.so. That means that we
can't use the code used on other systems to retry pthread_create on
EAGAIN, since that code simply calls pthread_create.
This patch copies that code to an OpenBSD-specific version.
Also, check for an EAGAIN failure in the test, as that seems to be the
underlying cause of the test failure on several systems including OpenBSD.
Fixes#18146.
Change-Id: I3bceaa1e03a7eaebc2da19c9cc146b25b59243ef
Reviewed-on: https://go-review.googlesource.com/33905
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit d24b57a6a1.
Reason for revert: Further complications arised (issue 18100). We'll try again in Go 1.9.
Change-Id: I5ca93d2643a4be877dd9c2d8df3359718440f02f
Reviewed-on: https://go-review.googlesource.com/33770
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
From the garbage collector's perspective, time can move backwards in
cgocall. However, in the midst of this time warp, the pointer
arguments to cgocall can go from dead back to live. If a stack growth
happens while they're dead and then a GC happens when they become live
again, GC can crash with a bad heap pointer.
Specifically, the sequence that leads to a panic is:
1. cgocall calls entersyscall, which saves the PC and SP of its call
site in cgocall. Call this PC/SP "X". At "X" both pointer arguments
are live.
2. cgocall calls asmcgocall. Call the PC/SP of this call "Y". At "Y"
neither pointer argument is live.
3. asmcgocall calls the C code, which eventually calls back into the
Go code.
4. cgocallbackg remembers the saved PC/SP "X" in some local variables,
calls exitsyscall, and then calls cgocallbackg1.
5. The Go code causes a stack growth. This stack unwind sees PC/SP "Y"
in the cgocall frame. Since the arguments are dead at "Y", they are
not adjusted.
6. The Go code returns to cgocallbackg1, which calls reentersyscall
with the recorded saved PC/SP "X", so "X" gets stashed back into
gp.syscallpc/sp.
7. GC scans the stack. It sees there's a saved syscall PC/SP, so it
starts the traceback at PC/SP "X". At "X" the arguments are considered
live, so it scans them, but since they weren't adjusted, the pointers
are bad, so it panics.
This issue started as of commit ca4089ad, when the compiler stopped
marking arguments as live for the whole function.
Since this is a variable liveness issue, fix it by adding KeepAlive
calls that keep the arguments live across this whole time warp.
The existing issue7978 test has all of the infrastructure for testing
this except that it's currently up to chance whether a stack growth
happens in the callback (it currently only happens on the
linux-amd64-noopt builder, for example). Update this test to force a
stack growth, which causes it to fail reliably without this fix.
Fixes#17785.
Change-Id: If706963819ee7814e6705693247bcb97a6f7adb8
Reviewed-on: https://go-review.googlesource.com/33710
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Use an explicit ./ to make sure we link against the libgo.so we just
built, not some other libgo.so that the compiler or linker may decide to
seek out.
Fixes#17986.
Change-Id: Id23f6c95aa2b52f4f42c1b6dac45482c22b4290d
Reviewed-on: https://go-review.googlesource.com/33413
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Before this CL, Go programs in c-archive or c-shared buildmodes
would not handle SIGPIPE. That leads to surprising behaviour where
writes on a closed pipe or socket would raise SIGPIPE and terminate
the program. This CL changes the Go runtime to handle
SIGPIPE regardless of buildmode. In addition, SIGPIPE from non-Go
code is forwarded.
Fixes#17393
Updates #16760
Change-Id: I155e82020a03a5cdc627a147c27da395662c3fe8
Reviewed-on: https://go-review.googlesource.com/32796
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The top-level qualifiers are unimportant for our purposes. If a C
function is defined as `const int f(const int i)`, the `const`s are
meaningless to C, and we want to avoid using them in the struct we
create where the `const` has a completely different meaning.
This unwinds https://golang.org/cl/33097 with regard to top-level
qualifiers.
Change-Id: I3d66b0eb43b6d9a586d9cdedfae5a2306b46d96c
Reviewed-on: https://go-review.googlesource.com/33325
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
CL 33239 changed the polling loops from using sched_yield to a sleep
for 1/1000 of a second. The loop counters were not updated, so failing
tests now take 100 seconds to complete. Lower the loop counts to 5
seconds instead.
Change-Id: I7c9a343dacc8188603ecf7e58bd00b535cfc87f5
Reviewed-on: https://go-review.googlesource.com/33280
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
To generate the correct section offset the shared code path for
R_CALL, R_PCREL, and R_GOTPCREL on darwin when externally linking
walks up the symbol heirarchy adding the differences. This is fine,
except in the case where we are generating a GOT lookup, because
the topmost symbol is left in r.Xsym instead of the symbol we are
looking up. So all funcsym GOT lookups were looking up the outer
"go.func.*" symbol.
Fix this by separating out the R_GOTPCREL code path.
For #17828 (and may fix it).
Change-Id: I2c9f4d135e77c17270aa064d8c876dc6d485d659
Reviewed-on: https://go-review.googlesource.com/33211
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This ensures that runtime's signal handlers pass through the TSAN and
MSAN libc interceptors and subsequent calls to the intercepted
sigaction function from C will correctly see them.
Fixes#17753.
Change-Id: I9798bb50291a4b8fa20caa39c02a4465ec40bb8d
Reviewed-on: https://go-review.googlesource.com/33142
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
If a C union type (or a C++ class type) can contain a pointer field,
then run the cgo checks on pointers to that type. This will test the
pointer as though it were an unsafe.Pointer, and will crash if it points
to Go memory that contains a pointer.
Fixes#15942.
Change-Id: Ic2d07ed9648d4b27078ae7683e26196bcbc59fc9
Reviewed-on: https://go-review.googlesource.com/33237
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In plugins and every program that opens a plugin, include a hash of
every imported package.
There are two versions of each hash: one local and one exported.
As the program starts and plugins are loaded, the first exported
symbol for each package becomes the canonical version.
Any subsequent plugin's local package hash symbol has to match the
canonical version.
Fixes#17832
Change-Id: I4e62c8e1729d322e14b1673bada40fa7a74ea8bc
Reviewed-on: https://go-review.googlesource.com/33161
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Apparently when GOMAXPROCS == 1 a simple sched_yield in a tight loop is
not necessarily sufficient to permit a signal handler to run. Instead,
sleep for 1/1000 of a second.
Fixes#16649.
Change-Id: I83910144228556e742b7a92a441732ef61aa49d9
Reviewed-on: https://go-review.googlesource.com/33239
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The cgo tool used to simply ignore C type qualifiers. To avoid problems
when a C function expected a qualifier that was not present, cgo emitted
a cast to void* around all pointer arguments. Unfortunately, that broke
code that contains both a function declaration and a macro, when the
macro required the argument to have the right type. To fix this problem,
don't ignore qualifiers. They are easy enough to handle for the limited
set of cases that matter for cgo, in which we don't care about array or
function types.
Fixes#17537.
Change-Id: Ie2988d21db6ee016a3e99b07f53cfb0f1243a020
Reviewed-on: https://go-review.googlesource.com/33097
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The exported symbol for a plugin can be the only reference to a
type in a program. In particular, "var F func()" will have
the type *func(), which is uncommon.
Fixes#17140
Change-Id: Ide2104edbf087565f5377374057ae54e0c00c57e
Reviewed-on: https://go-review.googlesource.com/29692
Run-TryBot: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
