The runtime is not instrumented, but the calls to msanread in the
runtime can sometimes refer to the system stack. An example is the call
to copy in stkbucket in mprof.go. Depending on what C code has done,
the system stack may appear uninitialized to msan.
Change-Id: Ic21705b9ac504ae5cf7601a59189302f072e7db1
Reviewed-on: https://go-review.googlesource.com/16660
Reviewed-by: David Crawshaw <crawshaw@golang.org>
This is a fix for the -msan option when using cgo callbacks. A cgo
callback works by writing out C code that puts a struct on the stack and
passes the address of that struct into Go. The result parameters are
fields of the struct. The Go code will write to the result parameters,
but the Go code thinks it is just writing into the Go stack, and
therefore won't call msanwrite. This CL adds a call to msanwrite in the
cgo callback code so that the C knows that results were written.
Change-Id: I80438dbd4561502bdee97fad3f02893a06880ee1
Reviewed-on: https://go-review.googlesource.com/16611
Reviewed-by: David Crawshaw <crawshaw@golang.org>
When using gccgo it's OK if a pointer passed to C remains on the stack.
Gccgo does not have the clear distinction between C and Go stacks.
Change-Id: I3af9dd6fe078214ab16d9d8dad2d206608d7891d
Reviewed-on: https://go-review.googlesource.com/16774
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Minux Ma <minux@golang.org>
This implements part of the proposal in issue 12416 by adding dynamic
checks for passing pointers from Go to C. This code is intended to be
on at all times. It does not try to catch every case. It does not
implement checks on calling Go functions from C.
The new cgo checks may be disabled using GODEBUG=cgocheck=0.
Update #12416.
Change-Id: I48de130e7e2e83fb99a1e176b2c856be38a4d3c8
Reviewed-on: https://go-review.googlesource.com/16003
Reviewed-by: Russ Cox <rsc@golang.org>
This is, in effect, what the gc toolchain does. It fixes cases where Go
code refers to a C global variable; without this, if the global variable
was the only thing visible in the C code, the generated cgo file might
not get pulled in from the archive, leaving the Go variable
uninitialized.
This was reported against gccgo as https://gcc.gnu.org/PR68255 .
Change-Id: I3e769dd174f64050ebbff268fbbf5e6fab1e2a1b
Reviewed-on: https://go-review.googlesource.com/16775
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
And enable PIE in cmd/go because that's all it seems to take.
Change-Id: Ie017f427ace5e91de333a9f7cba9684c4641dfd5
Reviewed-on: https://go-review.googlesource.com/14222
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The current mechanism fails if clang cannot be executed by the current
user. Using the `-x` operator for `test` return TRUE if the file is
executable by the user.
Change-Id: I0f3c8dc3880c1ce5a8a833ff3109eb96853184af
Reviewed-on: https://go-review.googlesource.com/16752
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The width of the type of an external variable defined with a type
literal may not be set when the instrumentation pass is run. There are
two cases in the standard library that fail without the call to dowidth:
../../../src/encoding/base32/base32.go:322: constant -1000000000 overflows uintptr
../../../src/encoding/base32/base32.go:329: constant -1000000000 overflows uintptr
../../../src/encoding/json/encode.go:385: constant -1000000000 overflows uintptr
../../../src/encoding/json/encode.go:387: constant -1000000000 overflows uintptr
Change-Id: I7c3334f7decdb7488595ffe4090cd262d7334283
Reviewed-on: https://go-review.googlesource.com/16331
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
On older versions of GCC we need to pass a file name before GCC will
report an unrecognized option.
Fixes#13065.
Change-Id: I7ed34c01a006966a446059025f7d10235c649072
Reviewed-on: https://go-review.googlesource.com/16589
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Depends on external linking right now. I have no immediate use for
this, but wanted to check how hard it is to support as android/amd64
is coming and it will require PIE.
Change-Id: I65c6b19159f40db4c79cf312cd0368c2b2527bfd
Reviewed-on: https://go-review.googlesource.com/16072
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also fix the msan_fail test. It was bogus, since it always aborted one
way or another.
Change-Id: Ic693327d1bddb7bc5c7d859ac047fc93cb9b5b1c
Reviewed-on: https://go-review.googlesource.com/16172
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The -msan option compiles Go code to use the memory sanitizer. This is
intended for use when linking with C/C++ code compiled with
-fsanitize=memory. When memory blocks are passed back and forth between
C/C++ and Go, code in both languages will agree as to whether the memory
is correctly initialized or not, and will report errors for any use of
uninitialized memory.
Change-Id: I2dbdbd26951eacb7d84063cfc7297f88ffadd70c
Reviewed-on: https://go-review.googlesource.com/16169
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Replace the confusing game where a frame size of $-8 would suppress the
implicit setting up of a stack frame with a nice explicit flag.
The code to set up the function prologue is still a little confusing but better
than it was.
Change-Id: I1d49278ff42c6bc734ebfb079998b32bc53f8d9a
Reviewed-on: https://go-review.googlesource.com/15670
Reviewed-by: Minux Ma <minux@golang.org>
Also, handle the case where 'read' returns EINVAL instead of EBADF
when the descriptor is not ready. (android 4.4.4/cyanogenmod, nexus7)
Change-Id: I56c5949d27303d44a4fd0de38951b85e20cef167
Reviewed-on: https://go-review.googlesource.com/15810
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes these warnings from go vet:
buildid_linux.go:25: no formatting directive in Fatalf call
callback.go:180: arg pc[i] for printf verb %p of wrong type: uintptr
env.go:34: possible misuse of unsafe.Pointer
issue7665.go:22: possible misuse of unsafe.Pointer
Change-Id: I83811b9c10c617139713a626b4a34ab05564d4fe
Reviewed-on: https://go-review.googlesource.com/15802
Reviewed-by: David Crawshaw <crawshaw@golang.org>
cgo panics in Package.rewriteRef for:
var a = C.enum_test(1)
or
p := new(C.enum_test)
when the corresponding enum type is not defined.
Check nil values for Type fields and issue a proper
error instead.
Fixes#11097
Updates #12160
Change-Id: I5821d29097ef0a36076ec5273125b09846c7d832
Reviewed-on: https://go-review.googlesource.com/15264
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The memory sanitizer (msan) is a nice compiler feature that can
dynamically check for memory errors in C code. It's not useful for Go
code, since Go is memory safe. But it is useful to be able to use the
memory sanitizer on C code that is linked into a Go program via cgo.
Without this change it does not work, as msan considers memory passed
from Go to C as uninitialized.
To make this work, change the runtime to call the C mmap function when
using cgo. When using msan the mmap call will be intercepted and marked
as returning initialized memory.
Work around what appears to be an msan bug by calling malloc before we
call mmap.
Change-Id: I8ab7286d7595ae84782f68a98bef6d3688b946f9
Reviewed-on: https://go-review.googlesource.com/15170
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Glibc uses some special signals for special thread operations. These
signals will be used in programs that use cgo and invoke certain glibc
functions, such as setgid. In order for this to work, these signals
need to not be masked by any thread. Before this change, they were
being masked by programs that used os/signal.Notify, because it
carefully masks all non-thread-specific signals in all threads so that a
dedicated thread will collect and report those signals (see ensureSigM
in signal1_unix.go).
This change adds the two glibc special signals to the set of signals
that are unmasked in each thread.
Fixes#12498.
Change-Id: I797d71a099a2169c186f024185d44a2e1972d4ad
Reviewed-on: https://go-review.googlesource.com/14297
Reviewed-by: David Crawshaw <crawshaw@golang.org>
It's because runtime links to ntdll, and ntdll exports a couple
incompatible libc functions. We must link to msvcrt first and
then try ntdll.
Fixes#12030.
Change-Id: I0105417bada108da55f5ae4482c2423ac7a92957
Reviewed-on: https://go-review.googlesource.com/14472
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently Go produces shared libraries that cannot be shared between processes
because they have relocations against the text segment (not text section). This
fixes this by moving some data to sections with magic names recognized by the
static linker.
The change in genasmsym to add STYPELINK to the switch should fix things on
darwin/arm64.
Fixes#10914
Updates #9210
Change-Id: Iab4a6678dd04cec6114e683caac5cf31b1063309
Reviewed-on: https://go-review.googlesource.com/14306
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 2c2cbb69c8.
Broke darwin/arm64
Change-Id: Ibd2dea475d6ce6a8b4b40e2da19a83fc0514025d
Reviewed-on: https://go-review.googlesource.com/14301
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Currently Go produces shared libraries that cannot be shared between processes
because they have relocations against the text segment (not text section). This
fixes this by moving some data to sections with magic names recognized by the
static linker.
Fixes#10914
Updates #9210
Change-Id: I7178daadc0ae87953d5a084aa3d580f4e3b46d47
Reviewed-on: https://go-review.googlesource.com/10300
Run-TryBot: Michael Hudson-Doyle <michael.hudson@canonical.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Fixes#12083
Change-Id: I8256739b33cf08d84dec23120d527667de2e6eea
Reviewed-on: https://go-review.googlesource.com/13822
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In order to fix issue #9401 the compiler was changed to add a padding
byte to any non-empty Go struct that ends in a zero-sized field. That
causes the Go version of such a C struct to have a different size than
the C struct, which can considerable confusion. Change cgo so that it
discards any such zero-sized fields, so that the Go and C structs are
the same size.
This is a change from previous releases, in that it used to be
possible to refer to a zero-sized trailing field (by taking its
address), and with this change it no longer is. That is unfortunate,
but something has to change. It seems better to visibly break
programs that do this rather than to silently break programs that rely
on the struct sizes being the same.
Update #9401.
Fixes#11925.
Change-Id: I3fba3f02f11265b3c41d68616f79dedb05b81225
Reviewed-on: https://go-review.googlesource.com/12864
Reviewed-by: Russ Cox <rsc@golang.org>
This is mostly Russ's https://golang.org/cl/12145 but with some extra fixes to
account for the fact that function declarations without implementations now
break shared libraries, and including my test case.
Fixes#11480.
Change-Id: Iabdc2934a0378e5025e4e7affadb535eaef2c8f1
Reviewed-on: https://go-review.googlesource.com/12340
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The one in misc/makerelease/makerelease.go is particularly bad and
probably warrants rotating our keys.
I didn't update old weekly notes, and reverted some changes involving
test code for now, since we're late in the Go 1.5 freeze. Otherwise,
the rest are all auto-generated changes, and all manually reviewed.
Change-Id: Ia2753576ab5d64826a167d259f48a2f50508792d
Reviewed-on: https://go-review.googlesource.com/12048
Reviewed-by: Rob Pike <r@golang.org>
All of the heavy-lifting was done by minux@, with his external-linking support
for darwin/arm64: golang.org/cl/8781
Change-Id: I7c9fbc19246f418c065c92fb2c13c00026ff0f82
Reviewed-on: https://go-review.googlesource.com/11127
Run-TryBot: Srdjan Petrovic <spetrovic@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Adjust timestamps in TestABIChecking to make sure that the library and
executable are rebuilt when expected.
Change-Id: I3288c254ba8201b5b4255347b0cb056fa0908657
Reviewed-on: https://go-review.googlesource.com/11128
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Yves Junqueira <yves.junqueira@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change reintroduces CL 8523. CL 8523 was reverted because
it broke darwin and netbsd builds. Now that this test is part
of "go tool dist test" command we could skip OSes that fail.
Updates #10360
Change-Id: Iaaeb5b800126492f36415a439c333a218fe4ab67
Reviewed-on: https://go-review.googlesource.com/11119
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This makes the behaviour match what happens when duplicate symbols are read
from regular object files and fixes errors about cgoAlwaysFalse when linking
an executable that uses cgo against a shared library.
Change-Id: Ibb8cd8fe3f7813cde504b7483f1e857868d7e063
Reviewed-on: https://go-review.googlesource.com/11117
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: Id93b8ab42fa311ce32209734ec9a0813f8736e25
Reviewed-on: https://go-review.googlesource.com/9914
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
I would like to re-apply reverted http://golang.org/cl/8523.
Reverted tests still fail in some environments (see issue #10360).
It is easier to run tests selectively when in Go.
This CL prepares for the changes.
Updates #10360
Change-Id: Iefeb1d71cb3d1cfa653a6ccd9f6e35686c0c5b24
Reviewed-on: https://go-review.googlesource.com/10608
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We already read the address of a gcmask/gcprog out of the type data, but I
didn't know how many bytes to read. But it turns out that it's easy to
calculate, so change to do that. This means that we no longer depend on the
local symbols being present, allowing me to strip the shared libraries for
distribution and make them a lot smaller.
As a bonus, this makes LSym another 24 bytes smaller, down to 296 bytes now.
Change-Id: I379d359e28d63afae6753efd23efdf1fbb716992
Reviewed-on: https://go-review.googlesource.com/10377
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The motivation for this is the innocuous looking test case that is added. This
creates a stack exe -> libdep2.so -> libdep.so -> libruntime.so. The problem
comes from the fact that a function from libdep.so gets inlined all the way
into exe. This (unsurprisingly) means that the object file for exe references
symbols from libdep.so, which means that -ldep needs to be passed when linking
exe and it isn't. The fix is simply to pass it -- there is no harm in passing
it when it's not needed.
The thing is, it's not clear at all in the current code to see how the linker
can know that libdep2 is linked against libdep. It could look through the
DT_NEEDED entries in libdep2 and try to guess which are Go libraries, but it
feels better to be explicit. So this adds another SHT_NOTE section that lists
the shared libraries a shared library was linked against, and makes sure the
complete set of depended upon shared libraries is passed to the external
linker.
Change-Id: I79aa6f98b4db4721d657a7eb7b7f062269bf49e2
Reviewed-on: https://go-review.googlesource.com/10376
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This makes for a more stable API for tools (including cmd/link itself) to
extract the abi hash from a shared library and makes it possible at all for a
library that has had the local symbol table removed.
The existing note-writing code only supports writing notes into the very start
of the object file so they are easy to find in core dumps. This doesn't apply
to the "go" notes and means that all notes have to fit into a fixed size
budget. That's annoying now we have more notes (and the next CL will add
another one) so this does a little bit of work to make adding notes that do not
have to go at the start of the file easier and moves the writing of the package
list note over to that mechanism, which lets me revert a hack that increased
the size budget mentioned above for -buildmode=shared builds.
Change-Id: I6077a68d395c8a2bc43dec8506e73c71ef77d9b9
Reviewed-on: https://go-review.googlesource.com/10375
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Change-Id: I8694ee5e5642c31815ae63cd414a3b1fcd9c95b0
Reviewed-on: https://go-review.googlesource.com/10411
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Fix build error when CL=clang introduced by CL 10173.
Change-Id: I8edf210787a9803280c0779ff710c7e634a820d6
Reviewed-on: https://go-review.googlesource.com/10341
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ian proposed an improved way of handling signals masks in Go, motivated
by a problem where the Android java runtime expects certain signals to
be blocked for all JVM threads. Discussion here
https://groups.google.com/forum/#!topic/golang-dev/_TSCkQHJt6g
Ian's text is used in the following:
A Go program always needs to have the synchronous signals enabled.
These are the signals for which _SigPanic is set in sigtable, namely
SIGSEGV, SIGBUS, SIGFPE.
A Go program that uses the os/signal package, and calls signal.Notify,
needs to have at least one thread which is not blocking that signal,
but it doesn't matter much which one.
Unix programs do not change signal mask across execve. They inherit
signal masks across fork. The shell uses this fact to some extent;
for example, the job control signals (SIGTTIN, SIGTTOU, SIGTSTP) are
blocked for commands run due to backquote quoting or $().
Our current position on signal masks was not thought out. We wandered
into step by step, e.g., http://golang.org/cl/7323067 .
This CL does the following:
Introduce a new platform hook, msigsave, that saves the signal mask of
the current thread to m.sigsave.
Call msigsave from needm and newm.
In minit grab set up the signal mask from m.sigsave and unblock the
essential synchronous signals, and SIGILL, SIGTRAP, SIGPROF, SIGSTKFLT
(for systems that have it).
In unminit, restore the signal mask from m.sigsave.
The first time that os/signal.Notify is called, start a new thread whose
only purpose is to update its signal mask to make sure signals for
signal.Notify are unblocked on at least one thread.
The effect on Go programs will be that if they are invoked with some
non-synchronous signals blocked, those signals will normally be
ignored. Previously, those signals would mostly be ignored. A change
in behaviour will occur for programs started with any of these signals
blocked, if they receive the signal: SIGHUP, SIGINT, SIGQUIT, SIGABRT,
SIGTERM. Previously those signals would always cause a crash (unless
using the os/signal package); with this change, they will be ignored
if the program is started with the signal blocked (and does not use
the os/signal package).
./all.bash completes successfully on linux/amd64.
OpenBSD is missing the implementation.
Change-Id: I188098ba7eb85eae4c14861269cc466f2aa40e8c
Reviewed-on: https://go-review.googlesource.com/10173
Reviewed-by: Ian Lance Taylor <iant@golang.org>
