This fixes two problems missed in CL 164877.
First, p.Internal.BuildInfo is now part of the cache key. This is
important since p.Internal.BuildInfo causes the build action to
synthesize a new source file, which affects the output.
Second, recompileForTest is always called for test
packages. Previously, it was only called when there were internal test
sources, so the fix in CL 164877 did not apply to packages that only
had external tests.
Fixes#30937
Change-Id: Iac2d7e8914f0313f9ab4222299a866f67889eb2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/168200
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
(cherry picked from commit d34548e0b6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168717
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The current wasm write barrier implementation incorrectly implements
the "deletion" part of the barrier. It correctly greys the new value
of the pointer, but rather than also greying the old value of the
pointer, it greys the object containing the slot (which, since the old
value was just overwritten, is not going to contain the old value).
This can lead to unmarked, reachable objects.
Often, this is masked by other marking activity, but one specific
sequence that can lead to an unmarked object because of this bug is:
1. Initially, GC is off, object A is reachable from just one pointer
in the heap.
2. GC starts and scans the stack of goroutine G.
3. G copies the pointer to A on to its stack and overwrites the
pointer to A in the heap. (Now A is reachable only from G's stack.)
4. GC finishes while A is still reachable from G's stack.
With a functioning deletion barrier, step 3 causes A to be greyed.
Without a functioning deletion barrier, nothing causes A to be greyed,
so A will be freed even though it's still reachable from G's stack.
This CL fixes the wasm write barrier.
Fixes#30873.
Change-Id: I8a74ee517facd3aa9ad606e5424bcf8f0d78e754
Reviewed-on: https://go-review.googlesource.com/c/go/+/167743
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit d9db9e32e9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167745
Reviewed-by: Katie Hockman <katie@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This reverts commit 731ebf4d87.
Reason for revert: It's not working for large directories.
Change-Id: Ie0f88e0ed1d36c4ea4f32d2acd4e223bd8229ca0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170882
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Getdirentries is implemented with the __getdirentries64 function
in libSystem.dylib. That function works, but it's on Apple's
can't-be-used-in-an-app-store-application list.
Implement Getdirentries using the underlying fdopendir/readdir_r/closedir.
The simulation isn't faithful, and could be slow, but it should handle
common cases.
Don't use Getdirentries in the stdlib, use fdopendir/readdir_r/closedir
instead (via (*os.File).readdirnames).
(Incorporates CL 170837 and CL 170698, which were small fixes to the
original tip CL.)
Fixes#31244
Update #28984
RELNOTE=yes
Change-Id: Ia6b5d003e5bfe43ba54b1e1d9cfa792cc6511717
Reviewed-on: https://go-review.googlesource.com/c/go/+/168479
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 9da6530faa)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170640
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
fd.l.Lock shouldn't be called in a loop.
Manual backport of CL 165598. It could not be cherry-picked due to conflicts.
Fixes#31211
Change-Id: Ib76e679f6a276b32fe9c1594b7e9a506017a7967
Reviewed-on: https://go-review.googlesource.com/c/go/+/170680
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
CL 135377 introduces pass strings and slices to convT2{E,I} by value.
Before that CL, all types, except interface will be allocated temporary
address. The CL changes the logic that only constant and type which
needs address (determine by convFuncName) will be allocated.
It fails to cover the case where type is static composite literal.
Adding condition to check that case fixes the issue.
Also, static composite literal node implies constant type, so consttype
checking can be removed.
Fixes#31209
Change-Id: Ifc750a029fb4889c2d06e73e44bf85e6ef4ce881
Reviewed-on: https://go-review.googlesource.com/c/go/+/168858
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit d47db6dc0c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170437
Run-TryBot: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In CL 120215 the cgo resolver was changed to have different logic based
on the network being queried. However, the singleflight cache key wasn't
updated to also include the network. This way it was possible for
concurrent queries to return the result for the wrong network.
This CL changes the key to include both network and host, fixing the
problem.
Fixes#31062
Change-Id: I8b41b0ce1d9a02d18876c43e347654312eba22fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/166037
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit e341bae08d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170320
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It is possible that a "volatile" value (one that can be clobbered
by preparing args of a call) to be used in multiple write barrier
calls. We used to copy the volatile value right before each call.
But this doesn't work if the value is used the second time, after
the first call where it is already clobbered. Copy it before
emitting any call.
Updates #30977.
Fixes#30996.
Change-Id: Iedcc91ad848d5ded547bf37a8359c125d32e994c
Reviewed-on: https://go-review.googlesource.com/c/go/+/168677
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit f23c601bf9)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168817
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
A regression was introduced in CL 137335 (5440bfc) that caused FlushInterval
to not be honored until the first Write() call was encountered. This change
starts the flush timer as part of setting up the maxLatencyWriter.
Fixes#31144
Change-Id: I75325bd926652922219bd1457b2b00ac6d0d41b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/170066
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 2cc347382f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/170137
This change is a re-apply of the reverted CL 140863 with changes to
address issue #30821. Specifically, path.Split continues to be used
to split the '/'-separated import path, rather than filepath.Split.
Document the algorithm for how the default executable name is determined
in DefaultExecName.
Rename a variable returned from os.Stat from bs to fi for consistency.
CL 140863 factored out the logic to determine the default executable
name from the Package.load method into a DefaultExecName function,
and started using it in more places to avoid having to re-implement
the logic everywhere it's needed. Most previous callers already computed
the default executable name based on the import path. The load.Package
method, before CL 140863, was the exception, in that it used the p.Dir
value in GOPATH mode instead. There was a NOTE(rsc) comment that it
should be equivalent to use import path, but it was too late in Go 1.11
cycle to risk implementing that change.
This is part 1, a more conservative change for backporting to Go 1.12.2,
and it keeps the original behavior of splitting on p.Dir in GOPATH mode.
Part 2 will address the NOTE(rsc) comment and modify behavior in
Package.load to always use DefaultExecName which splits the import path
rather than directory. It is intended to be included in Go 1.13.
Updates #27283
Updates #26869
Updates #30821Fixes#30266
Change-Id: Ib1ebb95acba7c85c24e3a55c40cdf48405af34f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/167503
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
(cherry picked from commit 94563de87f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168958
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Google's 8.8.8.8 DNS server used to reports its reverse DNS name
as ending in ".google.com". Now it's "dns.google.".
Change-Id: I7dd15f03239e5c3f202e471618ab867690cb4f9d
Reviewed-on: https://go-review.googlesource.com/c/go/+/169679
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit 3089d18956)
Reviewed-on: https://go-review.googlesource.com/c/go/+/169717
When generating DWARF inline info records, the post-SSA code looks
through the original "pre-inline" dcl list for the function so as to
handle situations where formal params are promoted or optimized away.
This code was not properly handling the case where an output parameter
was promoted to the heap -- in this case the param node is converted
in place from class PPARAMOUT to class PAUTOHEAP. This caused
inconsistencies later on, since the variable entry in the abstract
subprogram DIE wound up as a local and not an output parameter.
Updates #30908.
Fixes#31028.
Change-Id: Ia70b89f0cf7f9b16246d95df17ad6e307228b8c7
Reviewed-on: https://go-review.googlesource.com/c/go/+/168818
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 68a98d5279)
Reviewed-on: https://go-review.googlesource.com/c/go/+/169417
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
While many other call sites have been moved to using the proper
higher-level system loading, these areas were left out. This prevents
DLL directory injection attacks. This includes both the runtime load
calls (using LoadLibrary prior) and the implicitly linked ones via
cgo_import_dynamic, which we move to our LoadLibraryEx. The goal is to
only loosely load kernel32.dll and strictly load all others.
Meanwhile we make sure that we never fallback to insecure loading on
older or unpatched systems.
This is CVE-2019-9634.
Fixes#30666
Updates #14959
Updates #28978
Updates #30642
Change-Id: I401a13ed8db248ab1bb5039bf2d31915cac72b93
Reviewed-on: https://go-review.googlesource.com/c/go/+/165798
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
(cherry picked from commit 9b6e9f0c8c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/168339
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
`go build` has chosen the last element of the package import path
as the default output name when -o option is given. That caused
the output of a package build when the module root is the major
version component such as 'v2'.
A similar issue involving `go install` was fixed in
https://golang.org/cl/128900. This CL refactors the logic added
with the change and makes it available as
internal/load.DefaultExecName.
This CL makes 'go test' to choose the right default test binary
name when the tested package is in the module root. (E.g.,
instead of v2.test, choose pkg.test for the test of 'path/pkg/v2')
Fixes#27283Fixes#30266
Change-Id: I6905754f0906db46e3ce069552715f45356913ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/140863
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit bf94fc3ae3)
Reviewed-on: https://go-review.googlesource.com/c/go/+/167384
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Trying to call a method on a nil interface is a panic in Go. For
example:
var stringer fmt.Stringer
println(stringer.String()) // nil pointer dereference
In https://golang.org/cl/143097 we started recovering panics encountered
during function and method calls. However, we didn't handle this case,
as text/template panics before evalCall is ever run.
In particular, reflect's MethodByName will panic if the receiver is of
interface kind and nil:
panic: reflect: Method on nil interface value
Simply add a check for that edge case, and have Template.Execute return
a helpful error. Note that Execute shouldn't just error if the interface
contains a typed nil, since we're able to find a method to call in that
case.
Finally, add regression tests for both the nil and typed nil interface
cases.
Fixes#30464.
Change-Id: Iffb21b40e14ba5fea0fcdd179cd80d1f23cabbab
Reviewed-on: https://go-review.googlesource.com/c/161761
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
(cherry picked from commit 15b4c71a91)
Reviewed-on: https://go-review.googlesource.com/c/go/+/164457
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Currently, runtime.KeepAlive applied on a stack object doesn't
actually keeps the stack object alive, and the heap object
referenced from it could be collected. This is because the
address of the stack object is rematerializeable, and we just
ignored KeepAlive on rematerializeable values. This CL fixes it.
Updates #30476.
Fixes#30478.
Change-Id: Ic1f75ee54ed94ea79bd46a8ddcd9e81d01556d1d
Reviewed-on: https://go-review.googlesource.com/c/164537
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 40df9cc606)
Reviewed-on: https://go-review.googlesource.com/c/go/+/164627
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>
(cherry picked from commit 711ea1e716)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165748
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Previously, the result of sorting a map[interface{}] containing
multiple concrete types was non-deterministic. To ensure consistent
results, sort first by type name, then by concrete value.
Fixes#30484
Change-Id: I10fd4b6a74eefbc87136853af6b2e689bc76ae9d
GitHub-Last-Rev: 1b07f0c275
GitHub-Pull-Request: golang/go#30406
Reviewed-on: https://go-review.googlesource.com/c/163745
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 9d40fadb1c)
Reviewed-on: https://go-review.googlesource.com/c/go/+/164617
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Make sure the side effects inside short-circuited operations (&& and ||)
happen correctly.
Before this CL, we attached the side effects to the node itself using
exprInPlace. That caused other side effects in sibling expressions
to get reordered with respect to the short circuit side effect.
Instead, rewrite a && b like:
r := a
if r {
r = b
}
That code we can keep correctly ordered with respect to other
side-effects extracted from part of a big expression.
exprInPlace seems generally unsafe. But this was the only case where
exprInPlace is called not at the top level of an expression, so I
don't think the other uses can actually trigger an issue (there can't
be a sibling expression). TODO: maybe those cases don't need "in
place", and we can retire that function generally.
This CL needed a small tweak to the SSA generation of OIF so that the
short circuit optimization still triggers. The short circuit optimization
looks for triangle but not diamonds, so don't bother allocating a block
if it will be empty.
Go 1 benchmarks are in the noise.
Fixes#30567
Change-Id: I19c04296bea63cbd6ad05f87a63b005029123610
Reviewed-on: https://go-review.googlesource.com/c/go/+/165617
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit 4a9064ef41)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165858
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The -coverpkg lets users specify a list of packages that should have
coverage instrumentation. This may include packages not transitively
imported by tests. For each tested package, the synthetic main package
imports all covered packages so they can be registered with
testing.RegisterCover. This makes it possible for a main package to
import another main package.
When we compile a package with p.Internal.BuildInfo set (set on main
packages by Package.load in module mode), we set
runtime/debug.modinfo. Multiple main packages may be passed to the
linker because of the above scenario, so this causes duplicate symbol
errors.
This change copies p.Internal.BuildInfo to the synthetic main package
instead of the internal test package. Additionally, it forces main
packages imported by the synthetic test main package to be recompiled
for testing. Recompiled packages won't have p.Internal.BuildInfo set.
Fixes#30684
Change-Id: I06f028d55905039907940ec89d2835f5a1040203
Reviewed-on: https://go-review.googlesource.com/c/go/+/164877
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 10156b6783)
Reviewed-on: https://go-review.googlesource.com/c/go/+/166318
TryBot-Result: Gobot Gobot <gobot@golang.org>
EvalSymlinks was mishandling cases like "/x/../../y" or "../../../x"
where there is an extra ".." that goes past the start of the path.
Updates #30520Fixes#30586
Change-Id: I07525575f83009032fa1a99aa270c8d42007d276
Reviewed-on: https://go-review.googlesource.com/c/go/+/164762
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 294edb272d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/165197
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There's a "lib/time" sub-section in the Go 1.12 relase notes that
points to a non-existent golang.org/pkg/lib/time page.
The note is about a change in the tz database in the src/lib/time
directory, but the section's title (and the link) should probably just
refer to the time package.
Change-Id: Ibf9dacd710e72886f14ad0b7415fea1e8d25b83a
Reviewed-on: https://go-review.googlesource.com/c/164977
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 0dc6256540)
Reviewed-on: https://go-review.googlesource.com/c/164964
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
With stack objects, when we scan the stack, it scans defers with
tracebackdefers, but it seems to me that tracebackdefers doesn't
include the func value itself, which could be a stack allocated
closure. Scan it explicitly.
Alternatively, we can change tracebackdefers to include the func
value, which in turn needs to change the type of stkframe.
Updates #30453.
Fixes#30470.
Change-Id: I55a6e43264d6952ab2fa5c638bebb89fdc410e2b
Reviewed-on: https://go-review.googlesource.com/c/164118
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 4f4c2a79d4)
Reviewed-on: https://go-review.googlesource.com/c/164629
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
If GOCACHE is set but is not an absolute path, we cannot build.
And GOCACHE=off also returns the error message "build cache is
disabled by GOCACHE=off".
Fixes#30493
Change-Id: I24f64bc886599ca0acd757acada4714aebe4d3ae
Reviewed-on: https://go-review.googlesource.com/c/164200
Run-TryBot: Baokun Lee <nototon@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
(cherry picked from commit 13d24b685a)
Reviewed-on: https://go-review.googlesource.com/c/164717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Revert CL 137055, which changed Clean("\\somepath\dir\") to return
"\\somepath\dir" on Windows. It's not entirely clear this is correct,
as this path is really "\\server\share\", and as such the trailing
slash may be the path on that share, much like "C:\". In any case, the
change broke existing code, so roll it back for now and rethink for 1.13.
Updates #27791
Updates #30307
Change-Id: I69200b1efe38bdb6d452b744582a2bfbb3acbcec
Reviewed-on: https://go-review.googlesource.com/c/163077
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
(cherry picked from commit 153c0da89b)
Reviewed-on: https://go-review.googlesource.com/c/163078
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Current assembler reports error when it assembles
"TSTW $1689262177517664, R3", but go1.11 was building
fine.
Fixes#30334
Change-Id: I9c16d36717cd05df2134e8eb5b17edc385aff0a9
Reviewed-on: https://go-review.googlesource.com/c/163259
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ben Shi <powerman1st@163.com>
(cherry picked from commit 2ef8abb41f)
Reviewed-on: https://go-review.googlesource.com/c/163419
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>