Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: 'go get' does not add missing hash to go.sum when ziphash file missing from cache #44749

Closed
navytux opened this issue Mar 3, 2021 · 17 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@navytux
Copy link
Contributor

navytux commented Mar 3, 2021

Hello up there. I've hit a problem with Go1.16 where its module subsystem misbehaves.

The steps that show the problem are in #44129 (comment) . However @bcmills later says he cannot reproduce the problem and asks for more details about how my Go toolchain is bult (#44129 (comment), https://github.com/bcmills). My Go is just a regular Go source checkout built from go1.16-12-g2b7243a62f with $GOROOT_BOOTSTRAP pointing to go1.4.3-16-g4d5426a570. I've reverified building it from scratch in a new user environment and the problem is indeed gone.

HOWEVER

I've tried to rebuild my Go the same way in my original user and the problem persisted.
Then I've removed ~/.cache/go-build and the problem peristed.
Then I've removed ~/go/pkg and the problem disappeared.

I've preserved original content of my ~/go/pkg and tried to trim it down to something minimal.
I attach it here for the reference - if I set ~/go/pkg to that the problem of not updating go.sum on go get lab.nexedi.com/kirr/go123/xnet/[email protected] shows. With ~go reset to empty the problem disappears.

Offhand it looks to me to be a "stale cache not handled correctly" kind of problem.

Thanks beforehand,
Kirill

/cc @rsc, @jayconrod

What version of Go are you using (go version)?

$ go version
go version go1.16 linux/amd64

Does this issue reproduce with the latest release?

No, I had not seen it with Go1.15

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/kirr/.cache/go-build"
GOENV="/home/kirr/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/kirr/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/kirr/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/kirr/src/tools/go/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/kirr/src/tools/go/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/kirr/tmp/trashme/wendelin.core/wcfs/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3989139164=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.16 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.16
uname -sr: Linux 5.10.0-3-amd64
Distributor ID:	Debian
Description:	Debian GNU/Linux bullseye/sid
Release:	testing
Codename:	bullseye
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Debian GLIBC 2.31-9) stable release version 2.31.
gdb --version: GNU gdb (Debian 10.1-1.7) 10.1.90.20210103-git

What did you do?

(copied from #44129 (comment))

kirr@deco:~/tmp/trashme$ git clone https://lab.nexedi.com/kirr/wendelin.core
Клонирование в «wendelin.core»…
warning: переадресация на https://lab.nexedi.com/kirr/wendelin.core.git/
remote: Enumerating objects: 15831, done.
remote: Counting objects: 100% (15831/15831), done.
remote: Compressing objects: 100% (4249/4249), done.
remote: Total 15831 (delta 11537), reused 15632 (delta 11373)
Получение объектов: 100% (15831/15831), 7.04 MiB | 2.96 MiB/s, готово.
Определение изменений: 100% (11537/11537), готово.

kirr@deco:~/tmp/trashme$ cd wendelin.core/

kirr@deco:~/tmp/trashme/wendelin.core$ git checkout 481ffb89e78d310f9bdc79f14771494215d0d55a
Note: switching to '481ffb89e78d310f9bdc79f14771494215d0d55a'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD сейчас на 481ffb8 X wcfs: v↑ * (checkpoint)

kirr@deco:~/tmp/trashme/wendelin.core$ cd wcfs/

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 build
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:28:2: missing go.sum entry for module providing package crawshaw.io/sqlite (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:29:2: missing go.sum entry for module providing package crawshaw.io/sqlite/sqlitex (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/neo/[email protected]/neo/neonet/misc.go:27:2: missing go.sum entry for module providing package github.com/philhofer/fwd (imported by lab.nexedi.com/kirr/neo/go/neo/neonet); to add:
        go get lab.nexedi.com/kirr/neo/go/neo/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 get lab.nexedi.com/kirr/go123/xnet/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 get lab.nexedi.com/kirr/go123/xnet/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ git st
HEAD отделён на 481ffb8
нечего коммитить, нет изменений в рабочем каталоге

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 get -v lab.nexedi.com/kirr/go123/xnet/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 build
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:28:2: missing go.sum entry for module providing package crawshaw.io/sqlite (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:29:2: missing go.sum entry for module providing package crawshaw.io/sqlite/sqlitex (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/neo/[email protected]/neo/neonet/misc.go:27:2: missing go.sum entry for module providing package github.com/philhofer/fwd (imported by lab.nexedi.com/kirr/neo/go/neo/neonet); to add:
        go get lab.nexedi.com/kirr/neo/go/neo/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.15 build

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.15 get -v lab.nexedi.com/kirr/go123/xnet/[email protected]
go: found lab.nexedi.com/kirr/go123/xnet/lonet in lab.nexedi.com/kirr/go123 v0.0.0-20210302025843-863c4602a230

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ git st
HEAD отделён на 481ffb8
нечего коммитить, нет изменений в рабочем каталоге

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 build
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:28:2: missing go.sum entry for module providing package crawshaw.io/sqlite (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:29:2: missing go.sum entry for module providing package crawshaw.io/sqlite/sqlitex (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/neo/[email protected]/neo/neonet/misc.go:27:2: missing go.sum entry for module providing package github.com/philhofer/fwd (imported by lab.nexedi.com/kirr/neo/go/neo/neonet); to add:
        go get lab.nexedi.com/kirr/neo/go/neo/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ GO111MODULE=on go1.15 build

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 build
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:28:2: missing go.sum entry for module providing package crawshaw.io/sqlite (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:29:2: missing go.sum entry for module providing package crawshaw.io/sqlite/sqlitex (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/neo/[email protected]/neo/neonet/misc.go:27:2: missing go.sum entry for module providing package github.com/philhofer/fwd (imported by lab.nexedi.com/kirr/neo/go/neo/neonet); to add:
        go get lab.nexedi.com/kirr/neo/go/neo/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ echo $?
1

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 get -t .

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ git st
HEAD отделён на 481ffb8
нечего коммитить, нет изменений в рабочем каталоге

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ git diff

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 build
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:28:2: missing go.sum entry for module providing package crawshaw.io/sqlite (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/[email protected]/xnet/lonet/registry_sqlite.go:29:2: missing go.sum entry for module providing package crawshaw.io/sqlite/sqlitex (imported by lab.nexedi.com/kirr/go123/xnet/lonet); to add:
        go get lab.nexedi.com/kirr/go123/xnet/[email protected]
/home/kirr/go/pkg/mod/lab.nexedi.com/kirr/neo/[email protected]/neo/neonet/misc.go:27:2: missing go.sum entry for module providing package github.com/philhofer/fwd (imported by lab.nexedi.com/kirr/neo/go/neo/neonet); to add:
        go get lab.nexedi.com/kirr/neo/go/neo/[email protected]

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.15 version
go version go1.15.8 linux/amd64   # built from go1.15.8-6-gf75ab2d5a6

kirr@deco:~/tmp/trashme/wendelin.core/wcfs$ go1.16 version
go version go1.16 linux/amd64     # built from go1.16-12-g2b7243a62f

What did you expect to see?

go.sum updated and second attempt to build succeeds

What did you see instead?

go get lab.nexedi.com/kirr/go123/xnet/[email protected] - that go build suggests to update go.sum, changes nothing.

@navytux
Copy link
Contributor Author

navytux commented Mar 3, 2021

Attaching content of ~/go with which the problem reproduces: homego.zip

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules labels Mar 3, 2021
@bcmills bcmills added this to the Backlog milestone Mar 3, 2021
@bcmills
Copy link
Contributor

bcmills commented Mar 3, 2021

Thanks for the detail! I haven't had a chance to look into it yet, but I expect that the distilled ~/go/pkg contents will be very helpful.

@jayconrod
Copy link
Contributor

Reproduced. @navytux seriously thank you for attaching the module cache.

One of the missing sums is crawshaw.io/[email protected], which is not in the cache. go get doesn't download it or add the sum. If I download it into the cache separately with go mod download crawshaw.io/[email protected], then go get does add the sum.

Definitely seems like a bug here.

@jayconrod
Copy link
Contributor

Still investigating, but will be in meetings for a little while. There's this curious part of the import stack (reported by go mod why -m crawshaw.io/iox):

lab.nexedi.com/kirr/go123/xnet/lonet
crawshaw.io/sqlite/sqlitex
crawshaw.io/sqlite/sqlitex.test
crawshaw.io/iox/ioxtest

go build shouldn't care about an import from a test. go get shouldn't care about that import either unless the -t flag is given. So something's going wrong inside the modload package loader.

@bcmills
Copy link
Contributor

bcmills commented Mar 3, 2021

I wonder if crawshaw.io/iox is a red herring — it wasn't one of the checksums added in the trace in #44129 (comment).

The diff from there:

diff --git i/wcfs/go.sum w/wcfs/go.sum
index be8f8a9..7d7664f 100644
--- i/wcfs/go.sum
+++ w/wcfs/go.sum
@@ -1,6 +1,7 @@
 cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 crawshaw.io/iox v0.0.0-20181124134642-c51c3df30797/go.mod h1:sXBiorCo8c46JlQV3oXPKINnZ8mcqnye1EkVkqsectk=
+crawshaw.io/sqlite v0.3.2 h1:N6IzTjkiw9FItHAa0jp+ZKC6tuLzXqAYIv+ccIWos1I=
 crawshaw.io/sqlite v0.3.2/go.mod h1:igAO5JulrQ1DbdZdtVq48mnZUBAPOeFzer7VhDWNtW4=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/DataDog/czlib v0.0.0-20160811164712-4bc9a24e37f2 h1:/Xf0UwdF1dlv6cNu0q1TWTQgfSsALgXrdHNtBMj7ybU=
@@ -72,6 +73,7 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
+github.com/philhofer/fwd v1.1.1 h1:GdGcTjf5RNAxwS4QLsiMzJYj5KEvPJD3Abr261yRQXQ=
 github.com/philhofer/fwd v1.1.1/go.mod h1:gk3iGcWd9+svBvR0sR+KPcfE+RNWozjowpeBVG3ZVNU=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=

Curiously, both of those checksums are source-code checksums for modules for which the go.mod checksum is already present. I wonder if that's a contributing factor...

@navytux
Copy link
Contributor Author

navytux commented Mar 3, 2021

@bcmills, @jayconrod, you are welcome. And thanks for feedback confirming that you can reproduce the issue on your end now. Good luck with fixing it.

@jayconrod
Copy link
Contributor

@bcmills was right, crawshaw.io/iox was a red herring. crawshaw.io/[email protected] demonstrates the problem. This module has an extracted source directory in $GOMODCACHE/crawshaw.io/[email protected], but it doesn't have the files $GOMODCACHE/cache/download/crawshaw.io/sqlite/@v/v0.3.2.zip or .ziphash.

When go build imports a package from this module in "readonly" mode, it checks whether the sum is present before calling modfetch.Download. Since the sum is not present, it prints this error.

When go get imports a package from this module (not in "readonly" mode), it skips that check and calls modfetch.Download directly. modfetch.Download finds the extracted directory. It looks for the .ziphash file, and if it's present, it verifies it against go.sum. It just returns the directory if the .ziphash file is not present; that's a bug.

@jayconrod jayconrod self-assigned this Mar 3, 2021
@jayconrod jayconrod modified the milestones: Backlog, Go1.17 Mar 3, 2021
@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 3, 2021
@jayconrod jayconrod changed the title cmd/go: ~/go/pkg/mod got stale/corrupt -> modules misbehaving cmd/go: 'go get' does not add missing hash to go.sum when ziphash file missing from cache Mar 3, 2021
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/298352 mentions this issue: cmd/go/internal/modfetch: detect and recover from missing ziphash file

@jayconrod
Copy link
Contributor

@gopherbot Please backport to 1.16. It's a pre-existing issue, but it's much more likely to be observed because of the switch to -mod=readonly by default.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #44812 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@navytux
Copy link
Contributor Author

navytux commented Mar 8, 2021

@jayconrod, @bcmills, thanks for fixing this issue.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 8, 2021

@jayconrod If this affects Go 1.15 as well, we'll need to consider backporting it to both, or neither (see #34536 (comment)).

(That said, it is possible to consider that it doesn't affect Go 1.15 since its default is -mod=mod, not -mod=readonly.)

@jayconrod
Copy link
Contributor

@dmitshur This affects Go 1.15 a little differently. go build does not report an error, even with -mod=readonly, because this bug is neutralized by a different bug fixed in 1.16. But go get does not add the missing sum.

What do you think? This wouldn't normally qualify for a backport since the severity is lower, but we're already porting 1.16, and I expect the fix will apply cleanly to 1.15 as well.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 8, 2021

If the fix applies cleanly to 1.15 and is safe, it makes sense to consider for backport to both. It would keep the two releases more in sync, which generally helps people be able to move from Go 1.15 to 1.16 (or the opposite direction if needed) smoothly. Otherwise, given the go build doesn't report an error in 1.15, fixing just that problem in 1.16 only is also reasonable.

@jayconrod
Copy link
Contributor

Makes sense, keeping the releases consistent is definitely a good thing. I'll open a backport issue and CL.

To be clear, in Go 1.16, go build is correctly reporting a problem. The bug is that go get doesn't fix it. go get doesn't fix that problem in 1.15 either, but the problem is harder to notice, since go build doesn't complain.

@jayconrod
Copy link
Contributor

@gopherbot Please backport to 1.15.

@jayconrod
Copy link
Contributor

Backport issue #44872 opened. (gopherbot won't open it due to #25574).

auxten added a commit to gopher-lang/gopher that referenced this issue Mar 19, 2021
* cmd/compile: graceful handle error in noder LoadPackage

When syntax.Parse returns error, noder.file will be nil. Currently, we
continue accessing it regardlessly and depend on gc.hidePanic to hide
the panic from user.

Instead, we should gracefully handle the error in LoadPackage, then exit
earlier if any error occurred.

Updates #43311

Change-Id: I0a108ef360bd4f0cc9f481071b8967355e1513af
Reviewed-on: https://go-review.googlesource.com/c/go/+/294030
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not set type for OPACK

That's an invalid operation and depend on gc.hidePanic to report error.

Updates #43311

Change-Id: I78d615c40ab1e7887f612491e215c1c2bb758ef6
Reviewed-on: https://go-review.googlesource.com/c/go/+/294031
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: skip diag error in checkassign if one was emitted

While at it, also remove checkassignlist, which is not used.

For #43311

Change-Id: Ie7ed81f68080d8881fca6035da64a9755f2cb555
Reviewed-on: https://go-review.googlesource.com/c/go/+/294032
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not set type for OTYPE

That's an invalid operation and depend on gc.hidePanic to report error.

Updates #43311

Change-Id: Ib0761dcf4d9d2a23c41de7eff0376677a90b942e
Reviewed-on: https://go-review.googlesource.com/c/go/+/294033
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not add invalid key to constSet

After CL 272654, the compiler now use go/constant.Value to represent
constant nodes. That makes ir.ConstantValue requires node type to
correctly return value for untyped int node. But untyped int node can
have nil type after typechecked, e.g: using int value as key for
map[string]int, that makes the compiler crashes.

To fix it, just don't add the invalid key to constSet, since when
it's not important to report duplicated keys when they aren't valid.

For #43311
Fixes #44432

Change-Id: I44d8f2b95f5cb339e77e8a705a94bcb16e62beb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/294034
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: make check2 gracefully exit if it reported errors

Otherwise, if -d=panic was set, check2 will treat already reported
error as internal compiler error.

For #43311
Fixes #44445

Change-Id: I5dbe06334666df21d9107396b9dcfdd905aa1e44
Reviewed-on: https://go-review.googlesource.com/c/go/+/294850
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: register abi, morestack work and mole whacking

Morestack works for non-pointer register parameters

Within a function body, pointer-typed parameters are correctly
tracked.

Results still not hooked up.

For #40724.

Change-Id: Icaee0b51d0da54af983662d945d939b756088746
Reviewed-on: https://go-review.googlesource.com/c/go/+/294410
Trust: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: fix pointer maps for morestack

Verified with test and with single step watching changes to register
values across morestack calls, after reload.

Also added stack-growth test with pointer parameters of varying lifetime.

For #40724.

Change-Id: Idb5fe27786ac5c6665a734d41e68d3d39de2f4da
Reviewed-on: https://go-review.googlesource.com/c/go/+/294429
Trust: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Jeremy Faller <[email protected]>

* runtime: remove a duplicated testcase of TestPallocDataFindScavengeCa…

Change-Id: Ib44729ffb5d4d7b84114dcf028b7e0418c9d5035
GitHub-Last-Rev: 13f59a650aa424b4852ee4a803eebc793dbbfc15
GitHub-Pull-Request: golang/go#42434
Reviewed-on: https://go-review.googlesource.com/c/go/+/268021
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Trust: Martin Möhrmann <[email protected]>

* cmd/go: reject 'go list -m MOD@patch' when no existing version of MOD is required

Noticed while debugging failing tests for #36460.

Fixes #44788

Change-Id: Ic2cf511d871b29284f7372920f6f7d452825dd63
Reviewed-on: https://go-review.googlesource.com/c/go/+/298651
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* runtime: remove GODEBUG=scavenge mode

Change-Id: Ic4c7b5086303c7faa49f4cbf6738e66d5de35c7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/282012
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Austin Clements <[email protected]>

* cmd/go: ensure that the test subprocess always times out in TestScript/test_write_profiles_on_timeout

This test verifies the behavior of a test that fails due to timing
out. However, the test to be timed out was only sleeping for 1s before
returning successfully. That is empirically not always long enough for
the test process itself to detect the timeout and terminate.

We could replace the sleep with a select{}, but that would assume that
the deadlock detector does not terminate a test that reaches that
state (true today, but not necessarily so).

We could replace the arbitrarily sleep with an arbitrarily longer
sleep, but that's, well, arbitrary.

Instead, have the test sleep in an unbounded loop to ensure that it
always continues to run until the timeout is detected, and check the
test output to ensure that it actually reached the timeout path.

Fixes #32983

Change-Id: Ie7f210b36ef0cc0a4db473f780e15a3d6def8bda
Reviewed-on: https://go-review.googlesource.com/c/go/+/289889
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>

* cmd/internal: Add 6 args to ppc64 optab

This is a preparatory patch to support 6 arg opcodes on POWER10,
and simplify 5 arg opcode processing (e.g RLWNM and similar).

This expands the optab structure, and renames a4 arguments to a6.
No actual change in functionality is made.

Change-Id: I785e4177778e4bf1326cf8e46e8aeaaa0e4d406b
Reviewed-on: https://go-review.googlesource.com/c/go/+/295031
Run-TryBot: Lynn Boger <[email protected]>
Run-TryBot: Carlos Eduardo Seo <[email protected]>
Reviewed-by: Lynn Boger <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Trust: Keith Randall <[email protected]>

* cmd/compile: implement simple register results

at least for ints and strings

includes simple test

For #40724.

Change-Id: Ib8484e5b957b08f961574a67cfd93d3d26551558
Reviewed-on: https://go-review.googlesource.com/c/go/+/295309
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* syscall: treat proc thread attribute lists as unsafe.Pointers

It turns out that the proc thread update function doesn't actually
allocate new memory for its arguments and instead just copies the
pointer values into the preallocated memory. Since we were allocating
that memory as []byte, the garbage collector didn't scan it for pointers
to Go allocations and freed them. We _could_ fix this by requiring that
all users of this use runtime.KeepAlive for everything they pass to the
update function, but that seems harder than necessary. Instead, we can
just do the allocation as []unsafe.Pointer, which means the GC can
operate as intended and not free these from beneath our feet. In order
to ensure this remains true, we also add a test for this.

Fixes #44662.

Change-Id: Ib392ba8ceacacec94b11379919c8179841cba29f
Reviewed-on: https://go-review.googlesource.com/c/go/+/297389
Trust: Jason A. Donenfeld <[email protected]>
Trust: Alex Brainman <[email protected]>
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Jason A. Donenfeld <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

* cmd/compile: implement too-big-to-SSA struct passing in registers

Added a test that exercises named results

Change-Id: Ie228b68f4f846266595a95e0f65a6e4b8bf79635
Reviewed-on: https://go-review.googlesource.com/c/go/+/297029
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: remove I-saw-a-register-pragma chatter

It is not multithreaded-compilation-safe, and also seems
to cause problems on the noopt-builder.

Change-Id: I52dbcd507d256990f1ec7c8040ec7b76595aae4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/298850
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile/internal/syntax: print type parameters and type lists

types2 uses the syntax printer to print expressions (for tracing
or error messages), so we need to (at least) print type lists in
interfaces.

While at it, also implement the printing of type parameter lists.

Fixes #44766.

Change-Id: I36a4a7152d9bef7251af264b5c7890aca88d8dc3
Reviewed-on: https://go-review.googlesource.com/c/go/+/298549
Trust: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

* cmd/go: clarify errors for commands run outside a module

The new error message tells the user what was wrong (no go.mod found)
and directs them to 'go help modules', which links to tutorials.

Fixes #44745

Change-Id: I98f31fec4a8757eb1792b45491519da4c552cb0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/298650
Trust: Jay Conrod <[email protected]>
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>

* cmd/go: don't report missing std import errors for tidy and vendor

'go mod tidy' and 'go mod vendor' normally report errors when a
package can't be imported, even if the import appears in a file that
wouldn't be compiled by the current version of Go. These errors are
common for packages introduced in higher versions of Go, like "embed"
in 1.16.

This change causes 'go mod tidy' and 'go mod vendor' to ignore
missing package errors if the import path appears to come from the
standard library because it lacks a dot in the first path element.

Fixes #44557
Updates #27063

Change-Id: I61d6443e77ab95fd8c0d1514f57ef4c8885a77cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/298749
Trust: Jay Conrod <[email protected]>
Run-TryBot: Jay Conrod <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* cmd/compile: add register abi tests

Change-Id: I4b2b62a8eb1c4bf47f552214127d4ed5710af196
Reviewed-on: https://go-review.googlesource.com/c/go/+/297030
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: tweak offset-generator to elide more +0 offsets

this caused a problem in write barrier code when a spurious
zero-offset prevented a write barrier elision.

removed cache after instrumenting it and discovering
zero safe hits (one value must dominate the other, else
unsafe).

Change-Id: I42dfdb4d38ebfe158b13e766a7fabfc514d773f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/297349
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: return (and receive) medium-large results

includes three tests

Change-Id: I33ac0cfe35085d4b6ad2775abcaa3d7d6527b49f
Reviewed-on: https://go-review.googlesource.com/c/go/+/297031
Trust: David Chase <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: remove types2.(*Selection).TArgs(), now that instance bug seems fixed

Previously, we would sometimes see an internal (*instance) type for a
receiver of a types2 method, which was a bug. To deal with that, we put
in an extra (*Selection).TArgs() method. However, that (*instance) type
is no longer showing up for receivers, so we can remove the types2
method we added and do the work with existing types2 API methods.

Change-Id: I03e68f5bbaaf82fe706b6efecbb02e951bbd3cd4
Reviewed-on: https://go-review.googlesource.com/c/go/+/298869
Run-TryBot: Dan Scales <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Trust: Dan Scales <[email protected]>

* encoding/json: marshal maps using reflect.Value.MapRange

Map serialization using reflect.Value.MapIndex cannot retrieve
map keys that contain a NaN, resulting in a panic.
Switch the implementation to use the reflect.Value.MapRange method
instead, which iterates over all map entries regardless of whether
they are directly retrievable.

Note that according to RFC 8259, section 4, a JSON object should
have unique names, but does not forbid the occurrence of duplicate names.

Fixes #43207

Change-Id: If4bc55229b1f64b8ca4b0fed37549725efdace39
Reviewed-on: https://go-review.googlesource.com/c/go/+/278632
Trust: Meng Zhuo <[email protected]>
Trust: Joe Tsai <[email protected]>
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Meng Zhuo <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Joe Tsai <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

* cmd/go: update error expectations in TestScript/mod_install_pkg_version

This test was missed in CL 298650, and not caught by TryBots because
it is skipped it short mode (and we forgot to add longtest TryBots on
that CL).

Updates #44745

Change-Id: I696d01307dabf351b0e4735db0644f4e09c8e369
Reviewed-on: https://go-review.googlesource.com/c/go/+/298794
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* docs: fix case of GitHub

Change `Github` to `GitHub`

Change-Id: I514e8dc9a19182fcf9fcf5bc1b5fbff253c1a947
GitHub-Last-Rev: 7124c7058e0c7ff19dc8440fa79271eb6cfdaea9
GitHub-Pull-Request: golang/go#44260
Reviewed-on: https://go-review.googlesource.com/c/go/+/291950
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* cmd/go: update PWD variable for 'go generate'

Most subprocess invocations in the go command use base.AppendPWD to
append an accurate value of PWD to the command's environment, which can
speed up calls like os.Getwd and also help to provide less-confusing
output from scripts. Update `go generate` to do so.

Fixes #43862

Change-Id: I3b756f1532b2d922f7d74fd86414d5567a0122c0
GitHub-Last-Rev: 3ec8da265a2777d1dcbcea00f107b8f5905f3640
GitHub-Pull-Request: golang/go#43940
Reviewed-on: https://go-review.googlesource.com/c/go/+/287152
Reviewed-by: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Baokun Lee <[email protected]>

* math/big: remove bounds checks for shrVU_g inner loop

Make explicit a shrVU_g precondition.
Replace i with i+1 throughout the loop.
The resulting loop is functionally identical,
but the compiler can do better BCE without the i-1 slice offset.

Benchmarks results on amd64 with -tags=math_big_pure_go.

name                          old time/op  new time/op  delta
NonZeroShifts/1/shrVU-8       4.55ns ± 2%  4.45ns ± 3%   -2.27%  (p=0.000 n=28+30)
NonZeroShifts/1/shlVU-8       4.07ns ± 1%  4.13ns ± 4%   +1.55%  (p=0.000 n=26+29)
NonZeroShifts/2/shrVU-8       6.12ns ± 1%  5.55ns ± 1%   -9.30%  (p=0.000 n=28+28)
NonZeroShifts/2/shlVU-8       5.65ns ± 3%  5.70ns ± 2%   +0.92%  (p=0.008 n=30+29)
NonZeroShifts/3/shrVU-8       7.58ns ± 2%  6.79ns ± 2%  -10.46%  (p=0.000 n=28+28)
NonZeroShifts/3/shlVU-8       6.62ns ± 2%  6.69ns ± 1%   +1.07%  (p=0.000 n=29+28)
NonZeroShifts/4/shrVU-8       9.02ns ± 1%  7.79ns ± 2%  -13.59%  (p=0.000 n=27+30)
NonZeroShifts/4/shlVU-8       7.74ns ± 1%  7.82ns ± 1%   +0.92%  (p=0.000 n=26+28)
NonZeroShifts/5/shrVU-8       10.6ns ± 1%   8.9ns ± 3%  -16.31%  (p=0.000 n=25+29)
NonZeroShifts/5/shlVU-8       8.59ns ± 1%  8.68ns ± 1%   +1.13%  (p=0.000 n=27+29)
NonZeroShifts/10/shrVU-8      18.2ns ± 2%  14.4ns ± 1%  -20.96%  (p=0.000 n=27+28)
NonZeroShifts/10/shlVU-8      14.1ns ± 1%  14.1ns ± 1%   +0.46%  (p=0.001 n=26+28)
NonZeroShifts/100/shrVU-8      161ns ± 2%   118ns ± 1%  -26.83%  (p=0.000 n=29+30)
NonZeroShifts/100/shlVU-8      119ns ± 2%   120ns ± 2%   +0.92%  (p=0.000 n=29+29)
NonZeroShifts/1000/shrVU-8    1.54µs ± 1%  1.10µs ± 1%  -28.63%  (p=0.000 n=29+29)
NonZeroShifts/1000/shlVU-8    1.10µs ± 1%  1.10µs ± 2%     ~     (p=0.701 n=28+29)
NonZeroShifts/10000/shrVU-8   15.3µs ± 2%  10.9µs ± 1%  -28.68%  (p=0.000 n=28+28)
NonZeroShifts/10000/shlVU-8   10.9µs ± 2%  10.9µs ± 2%   -0.57%  (p=0.003 n=26+29)
NonZeroShifts/100000/shrVU-8   154µs ± 1%   111µs ± 2%  -28.04%  (p=0.000 n=27+28)
NonZeroShifts/100000/shlVU-8   113µs ± 2%   113µs ± 2%     ~     (p=0.790 n=30+30)

Change-Id: Ib6a621ee7c88b27f0f18121fb2cba3606c40c9b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/297049
Trust: Josh Bleecher Snyder <[email protected]>
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/link: add relocs type for mips64x

The race detector of llvm adds four reloc types even with -fPIC

elf.R_MIPS_CALL16
elf.R_MIPS_GPREL32
elf.R_MIPS_64
elf.R_MIPS_GOT_DISP

Change-Id: If73119dcba14ef74395273eb680f52a0aa853217
Reviewed-on: https://go-review.googlesource.com/c/go/+/270297
Trust: Meng Zhuo <[email protected]>
Run-TryBot: Meng Zhuo <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* net/http/httptrace: fix doc typo

Change-Id: I919d9c3968c0fcd33774e714f22182504790bd01
Reviewed-on: https://go-review.googlesource.com/c/go/+/284143
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
Trust: Daniel Martí <[email protected]>

* testing: add TB.Setenv

Add a new method TB.Setenv that'll set environment variables
only for the isolated lifetime of the test, and will clean up
and unset these variables when the test ends.
This method disables the test or benchmark from running in
parallel.

Fixes #41260

Change-Id: I0a18f094ec1c6ec3157b4b12993ea3075e2e9867
GitHub-Last-Rev: 0ca12fa565318f350b927e2ef94f3b4f792c75c2
GitHub-Pull-Request: golang/go#41857
Reviewed-on: https://go-review.googlesource.com/c/go/+/260577
Trust: Daniel Martí <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
Run-TryBot: Daniel Martí <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: roger peppe <[email protected]>

* cmd/go/internal/modfetch: detect and recover from missing ziphash file

Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.

With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.

Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.

Fixes #44749

Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <[email protected]>
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>

* reflect: use global variables for register count

This change switches reflect to use global variables for ABI-related
register counts instead of internal/abi constants. The advantage of
doing so is that we can make the internal/abi constants non-zero and
enable the runtime register argument spiller/unspiller even if they're
not used. It's basically turning two things we need to flip when we
switch to the register ABI into one.

It also paves the way for testing the reflect register ABI path
independently, because now we can switch the global variables at will
and run the register-assignment algorithm in tests without having the
rest of the runtime be broken.

Change-Id: Ie23629a37a5c80aeb24909d4bd9eacbd3f0c06d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/293149
Trust: Michael Knyszek <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* reflect: include the alignment of zero-sized types in stack offsets

This change modifies the reflect ABI assignment algorithm to catch
zero-sized types at the top level of each argument and faux-stack-assign
them. It doesn't actually generate an ABI step, which is unnecessary,
but it ensures that the offsets of further stack-assigned arguments are
aligned to the alignment of that zero-sized argument.

This change is necessary to have the register ABI assignment algorithm
gracefully degrade to ABI0 when no registers are present in the ABI.

Fixes #44377.

Change-Id: Ia95571688a61259302bb3c6d5fb33fbb6b5e8db8
Reviewed-on: https://go-review.googlesource.com/c/go/+/293789
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Trust: Than McIntosh <[email protected]>

* cmd/go: require a module root in 'go list -m' with an unversioned path

Fixes #44803

Change-Id: Ie6ee2e3bca1809c91ecedec75d2c6620da914b29
Reviewed-on: https://go-review.googlesource.com/c/go/+/298752
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>

* runtime: update paniclk ordering

Now that allglock is no longer taken in throw, paniclk can move to the
bottom of the lock order where it belongs.

There is no fundamental reason that we really need to skip checks on
paniclk in lockWithRank (despite the recursive throws that could be
caused by lock rank checking, startpanic_m would still allow the crash
to complete). However, the partial order of lockRankPanic should be
every single lock that may be held before a throw, nil dereference,
out-of-bounds access, which our partial order doesn't cover.

Updates #42669

Change-Id: Ic3efaea873dc2dd9fd5b0d6ccdd5319730b29a22
Reviewed-on: https://go-review.googlesource.com/c/go/+/270862
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>

* text/template: wrap errors returned by template functions instead of stringifying them

Fixes #34201

Change-Id: Ic2e2967e4b01167345cf38bd006cabb206a64377
GitHub-Last-Rev: 5d0c4856550614484a8dbfb68c37aa1abcfcc529
GitHub-Pull-Request: golang/go#42398
Reviewed-on: https://go-review.googlesource.com/c/go/+/267838
Reviewed-by: Daniel Martí <[email protected]>
Trust: Daniel Martí <[email protected]>
Trust: Pontus Leitzler <[email protected]>
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Daniel Martí <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* cmd/compile: only check return for valid functions

CheckReturn uses fn.Type() unconditionally, so for invalid function,
fn.Type() will be nil, causes the compiler crashes.

Updates #43311

Change-Id: I4420dd296c72ea83986b38fbf2c7f51fa59757c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/298709
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: don't expand invalid embedded interface

The invalid interface type will be reported already, so don't expand
that invalid one, which causes the compiler crashes.

Updates #43311

Change-Id: Ic335cfa74f0b9fcfd0929dc5fd31d9156a8f5f5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298710
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not set type for OTYPESW

Same as CL 294031, but for OTYPESW.

Updates #43311

Change-Id: I996f5938835baff1d830c17ed75652315106bdfd
Reviewed-on: https://go-review.googlesource.com/c/go/+/298712
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not report error for invalid constant

Invalid constant was already reported by noder, so don't re-check in
typecheck, which lead to compiler crashing.

Updates #43311

Change-Id: I48e2f540601cef725c1ff628c066ed15d848e771
Reviewed-on: https://go-review.googlesource.com/c/go/+/298713
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: fix wrong condition in tcShift

CL 279442 refactored typecheck arithmetic operators, but using wrong
condition for checking invalid rhs.

Updates #43311

Change-Id: I7a03a5535b82ac4ea4806725776b0a4f7af1b79a
Reviewed-on: https://go-review.googlesource.com/c/go/+/298714
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/compile: do not set ONAME type when evaluated in type context

Updates #43311

Change-Id: I26e397d071b434256dab0cc7fff9d134b80bd6e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/298711
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* test: enable "-d=panic" by default for errorcheck*

Fixes #43311

Change-Id: I134d6c0524c198998a3c093dd3a8144052e8f7a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/298715
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/go/internal/modfetch/codehost: report git errors more accurately

Previously, if you attempted to fetch a private repository, or your
Git/curl client failed for an unknown reason, codehost would return an
UnknownRevisionError, which reported that a given revision in go.mod
was "unknown". This is confusing to many users who can go look in
their browser for example and see that the commit-ish exists.

Instead check whether "git ls-remote" exited with an error, and if so,
return that instead of the UnknownRevision message.

Fixes #42751.

Change-Id: I0dbded878b2818280e61126a4493767d719ad577
Reviewed-on: https://go-review.googlesource.com/c/go/+/297950
Reviewed-by: Bryan C. Mills <[email protected]>
Trust: Bryan C. Mills <[email protected]>
Trust: Jay Conrod <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* runtime: encapsulate access to allgs

Correctly accessing allgs is a bit hairy. Some paths need to lock
allglock, some don't. Those that don't are safest using atomicAllG, but
usage is not consistent.

Rather than doing this ad-hoc, move all access* through forEachG /
forEachGRace, the locking and atomic versions, respectively. This will
make it easier to ensure safe access.

* markroot is the only exception, as it has a far-removed guarantee of
safe access via an atomic load of allglen far before actual use.

Change-Id: Ie1c7a8243e155ae2b4bc3143577380c695680e89
Reviewed-on: https://go-review.googlesource.com/c/go/+/279994
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>

* cmd/compile: match Aux and AuxInt explicitly in store combining rule

CL 280456 introduced a new store combining rule. On the LHS some
of the Aux and AuxInt of the stores are not specified, therefore
ignored during the matching. The rule is only correct if they
match. This CL adds explict match.

TODO: maybe we want the rule matcher require Aux/AuxInt to be
always specified on the LHS (using _ to explicitly ignore)? Or
maybe we want it to match the zero value if not specified? The
current approach is error-prone.

Fixes #44823.

Change-Id: Ic12b4a0de63117f2f070039737f0c905f28561bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/299289
Trust: Cherry Zhang <[email protected]>
Trust: Josh Bleecher Snyder <[email protected]>
Reviewed-by: Josh Bleecher Snyder <[email protected]>

* cmd/compile: use getcallerpc for racefuncentry

Currently, when instrumenting for the race detector, the compiler
inserts racefuncentry/racefuncentryfp at the entry of instrumented
functions. racefuncentry takes the caller's PC. On AMD64, we synthesize
a node which points to -8(FP) which is where the return address is
stored. Later this node turns to a special Arg in SSA that is not
really an argument. This causes problems in the new ABI work so that
special node has to be special-cased.

This CL changes the special node to a call to getcallerpc, which lowers
to an intrinsic in SSA. This also unifies AMD64 code path and LR machine
code path, as getcallerpc works on all platforms.

Change-Id: I1377e140b91e0473cfcadfda221f26870c1b124d
Reviewed-on: https://go-review.googlesource.com/c/go/+/297929
Trust: Cherry Zhang <[email protected]>
Reviewed-by: David Chase <[email protected]>

* cmd/compile: use getcallersp for gorecover "fp" arg

Currently, the compiler synthesize a special ".fp" node, which
points to the FP of the current frame, be to used to call
gorecover. Later that node turns to an Arg in SSA that is not
really an arg, causing problems for the new ABI work which changes
the handling of Args, so we have to special-case that node.

This CL changes the compiler to get the FP by using getcallersp,
which is an intrinsic in SSA and works on all platforms. As we
need the FP, not the caller SP, one drawback is that we have to
add FixedFrameSize for LR machines. But it does allow us to remove
that special node.

Change-Id: Ie721d51efca8116c9d23cc4f79738fffcf847df8
Reviewed-on: https://go-review.googlesource.com/c/go/+/297930
Trust: Cherry Zhang <[email protected]>
Reviewed-by: David Chase <[email protected]>

* cmd/internal/goobj: regenerate builtin list

Change-Id: Ib8cb5f90e084838f00ecba78641bbb5d48ecac32
Reviewed-on: https://go-review.googlesource.com/c/go/+/297931
Trust: Cherry Zhang <[email protected]>
Reviewed-by: David Chase <[email protected]>

* runtime: remove racefuncenterfp

No longer needed with previous CL.

Change-Id: I7c01f9e0e34ecb9553ef1b3d662f33419fd3a244
Reviewed-on: https://go-review.googlesource.com/c/go/+/297932
Trust: Cherry Zhang <[email protected]>
Reviewed-by: David Chase <[email protected]>

* cmd/internal/goobj: store relocation type as uint16

Currently, relocation type is stored as uint8 in object files, as
Go relocations do not exceed 255. In the linker, however, it is
used as a 16-bit type, because external relocations can exceed
255. The linker has to store the extra byte in a side table. This
complicates many things.

Just store it as uint16 in object files. This simplifies things,
with a small cost of increasing the object file sizes.

               before      after
hello.o         1672        1678
runtime.a    7927784     8056194

Change-Id: I313cf44ad0b8b3b76e35055ae55d911ff35e3158
Reviewed-on: https://go-review.googlesource.com/c/go/+/268477
Trust: Cherry Zhang <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>

* reflect: add VisibleFields function

When writing code that reflects over a struct type, it's a common requirement to know the full set of struct fields, including fields available due to embedding of anonymous members while excluding fields that are erased because they're at the same level as another field with the same name.

The logic to do this is not that complex, but it's a little subtle and easy to get wrong.

This CL adds a new `VisibleFields` function to the reflect package that returns the full set of effective fields that apply in a given struct type.

Performance isn't a prime consideration, as it's common to cache results by type.

Fixes #42782

Change-Id: I7f1af76cecff9b8a2490f17eec058826e396f660
Reviewed-on: https://go-review.googlesource.com/c/go/+/281233
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Daniel Martí <[email protected]>

* cmd/internal/objabi: use a separate bit to mark weak relocation

Instead of using two relocation types R_XXX and R_WEAKXXX, use a
separate bit, R_WEAK, to mark weak relocations. This makes it
easier to add more weak relocation types.

Change-Id: Iec4195c2aefa65f59e464c83018246e17cd08173
Reviewed-on: https://go-review.googlesource.com/c/go/+/268478
Trust: Cherry Zhang <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Jeremy Faller <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>

* math/big: add shrVU and shlVU benchmarks

Change-Id: Id67d6ac856bd9271de99c3381bde910aa0c166e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/296011
Trust: Josh Bleecher Snyder <[email protected]>
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/compile: rename internal-abi.md to abi-internal.md

Allows muscle-memoried tab completion of cmd/compile/internal/...
paths to work again.

Change-Id: Ib54a5f2cc9fabcb876c2e62635828ab28b565501
Reviewed-on: https://go-review.googlesource.com/c/go/+/299530
Reviewed-by: Austin Clements <[email protected]>
Trust: Matthew Dempsky <[email protected]>

* cmd/compile: improve IsNonNil rule on riscv64

IsNonNil is readily implemented using SNEZ on riscv64, removing over 8,000
instructions from the go binary. Other rules will improve on this sequence,
however in this case it makes sense to use a direct simplification.

Change-Id: Ib4068599532398afcd05f51d160673ef5fb5e5a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/299230
Trust: Joel Sing <[email protected]>
Reviewed-by: Michael Munday <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* encoding/binary: limit bytes read by Uvarint to <= 10

Limits the number of bytes that can be consumed by Uvarint
to MaxVarintLen64 (10) to avoid wasted computations.
With this change, if Uvarint reads more than MaxVarintLen64
bytes, it'll return the erroring byte count of n=-(MaxVarintLen64+1)
which is -11, as per the function signature.

Updated some tests to reflect the new change in expectations of n
when the number of bytes to be read exceeds the limits..

Fixes #41185

Change-Id: Ie346457b1ddb0214b60c72e81128e24d604d083d
Reviewed-on: https://go-review.googlesource.com/c/go/+/299531
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Trust: Emmanuel Odeke <[email protected]>

* cmd/compile: minor doc improvements

These are left over from comments I failed to leave on CL 249463;
apparently I never hit "Reply".

Change-Id: Ia3f8a900703c347f8f98581ec1ac172c0f72cd9e
Reviewed-on: https://go-review.googlesource.com/c/go/+/299589
Trust: Josh Bleecher Snyder <[email protected]>
Run-TryBot: Josh Bleecher Snyder <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

* cmd/link/internal/ld: fix typo in a comment

Change-Id: I6d0fafd38f0ad9392f163a9d8cd94d103bf2a059
Reviewed-on: https://go-review.googlesource.com/c/go/+/299669
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: fix width not calculated for imported type

The compiler currently has problem that some imported type is missing
size calculation. The problem is not triggered until CL 283313 merged,
due to the compiler can compile the functions immediately when it sees
them, so during SSA generation, size calculation is still ok.

CL 283313 makes the compiler always push functions to compile queue,
then drain from it for compiling function. During this process, the
types calculation size is disabled, so calculating size during SSA now
make the compiler crashes.

To fix this, we can just always calculate type size during typechecking,
when importing type from other packages.

Fixes #44732

Change-Id: I8d00ea0b5aadd432154908280e55d85c75f3ce92
Reviewed-on: https://go-review.googlesource.com/c/go/+/299689
Trust: Cuong Manh Le <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dan Scales <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>

* cmd/link: use testing.T.TempDir in tests

Change-Id: I6fc8c9ee6d2246bfd874eb58b411e34ddbeaf723
Reviewed-on: https://go-review.googlesource.com/c/go/+/299670
Reviewed-by: Cherry Zhang <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

* os/signal: remove comments about SA_RESTART

It's no longer necessary for non-Go signal handlers to use SA_RESTART.

For #20400
Fixes #44761

Change-Id: Ie3c7fecc631a4a2822331754296ea09b308e1391
Reviewed-on: https://go-review.googlesource.com/c/go/+/298269
Trust: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* cmd/vet: bring in sigchanyzer to report unbuffered channels to signal.Notify

Brings in the static analyzer "sigchanyzer", that we created at
Orijtech, Inc, and already submitted in CL 274352, as

    golang.org/x/tools/go/analysis/passes/sigchanyzer

and add it to cmd/vet as one of the passes.

Fixes #9399

Change-Id: I83708b8ea5ca8ede5ee31efab55cbce7419434ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/299532
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Cuong Manh Le <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Trust: Cuong Manh Le <[email protected]>
Trust: Bryan C. Mills <[email protected]>

* cmd/compile: remove 4-byte alignment requirement of stack slot on arm

This CL applies CL 267999 to arm.

Updates #42385

Change-Id: Iad82aafcb7b0a5a77a4bea32f648320f57a17cdd
Reviewed-on: https://go-review.googlesource.com/c/go/+/297773
Reviewed-by: eric fang <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>
Trust: eric fang <[email protected]>
Run-TryBot: eric fang <[email protected]>

* runtime: document netpollclose

Document that network poller implementations need to define
netpollclose.

Change-Id: Idc73dea7cfd503d4de7e1d95902f0f102cf5ed8f
Reviewed-on: https://go-review.googlesource.com/c/go/+/297809
Trust: Tobias Klauser <[email protected]>
Run-TryBot: Tobias Klauser <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

* cmd/compile: make ValAndOff.{Val,Off} return an int32

The ValAndOff type is a 64bit integer holding a 32bit value and a
32bit offset in each half, but for historical reasons its Val and Off
methods returned an int64. This was convenient when AuxInt was always
an int64, but now that AuxInts are typed we can return int32 from Val
and Off and get rid of a several casts and now unnecessary range
checks.

This change:

- changes the Val and Off methods to return an int32 (from int64);
- adds Val64 and Off64 methods for convenience in the few remaining
  places (in the ssa.go files) where Val and Off are stored in int64
  fields;
- deletes makeValAndOff64, renames makeValAndOff32 to makeValAndOff
- deletes a few ValAndOff methods that are now unused;
- removes several validOff/validValAndOff check that will always
  return true.

Passes:

  GOARCH=amd64 gotip build -toolexec 'toolstash -cmp' -a std
  GOARCH=386 gotip build -toolexec 'toolstash -cmp' -a std
  GOARCH=s390x gotip build -toolexec 'toolstash -cmp' -a std

(the three GOARCHs with SSA rules files impacted by the change).

Change-Id: I2abbbf42188c798631b94d3a55ca44256f140be7
Reviewed-on: https://go-review.googlesource.com/c/go/+/299149
Trust: Alberto Donizetti <[email protected]>
Trust: Keith Randall <[email protected]>
Run-TryBot: Alberto Donizetti <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>

* cmd/link/internal/ld: deflake TestWindowsIssue36495

Over a dozen of the ld tests were missing closes. That was less
obvious before CL 299670 started using T.TempDir instead, which fails
a test when the tempdir can't be cleaned up (as it can't on Windows
when things are still open), insteading of leaving tempdirs around on
disk after the test.

Most of the missing closes were fixed in CL 299670, but the builders
helpfully pointed out that I missed at least this one.

Change-Id: I35f695bb7cbfba31e16311c5af965c148f9d7943
Reviewed-on: https://go-review.googlesource.com/c/go/+/299929
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dan Scales <[email protected]>
Reviewed-by: Alex Brainman <[email protected]>
Trust: Dan Scales <[email protected]>
Trust: Alex Brainman <[email protected]>

* net: use io.Discard in TestSendfileOnWriteTimeoutExceeded

Replace ioutil.Discard which is deprecated as of Go 1.16.

This was already done in CL 263142 but accidentially re-introduced in
CL 285914.

Change-Id: Ife0944d416294b1ba7c8f6b602aa68a3b9213c50
Reviewed-on: https://go-review.googlesource.com/c/go/+/299989
Trust: Tobias Klauser <[email protected]>
Run-TryBot: Tobias Klauser <[email protected]>
Reviewed-by: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* cmd/compile:  get instantiated generic types working with interfaces

Get instantiatiated generic types working with interfaces, including
typechecking assignments to interfaces and instantiating all the methods
properly. To get it all working, this change includes:

 - Add support for substituting in interfaces in subster.typ()

 - Fill in the info for the methods for all instantiated generic types,
   so those methods will be available for later typechecking (by the old
   typechecker) when assigning an instantiated generic type to an
   interface. We also want those methods available so we have the list
   when we want to instantiate all methods of an instantiated type. We
   have both for instantiated types encountered during the initial noder
   phase, and for instantiated types created during stenciling of a
   function/method.

 - When we first create a fully-instantiated generic type (whether
   during initial noder2 pass or while instantiating a method/function),
   add it to a list so that all of its methods will also be
   instantiated. This is needed so that an instantiated type can be
   assigned to an interface.

 - Properly substitute type names in the names of instantiated methods.

 - New accessor methods for types.Type.RParam.

 - To deal with generic types which are empty structs (or just don't use
   their type params anywhere), we want to set HasTParam if a named type
   has any type params that are not fully instantiated, even if the
   type param is not used in the type.

 - In subst.typ() and elsewhere, always set sym.Def for a new forwarding
   type we are creating, so we always create a single unique type for
   each generic type instantiation. This handles recursion within a
   type, and also recursive relationships across many types or methods.
   We remove the seen[] hashtable, which was serving the same purpose,
   but for subst.typ() only. We now handle all kinds of recursive types.

 - We don't seem to need to force types.CheckSize() on
   created/substituted generic types anymore, so commented out for now.

 - Add an RParams accessor to types2.Signature, and also a new
   exported types2.AsSignature() function.

Change-Id: If6c5dd98427b20bfe9de3379cc16f83df9c9b632
Reviewed-on: https://go-review.googlesource.com/c/go/+/298449
Run-TryBot: Dan Scales <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Dan Scales <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/compile: add debugging mode for import/export

Just add a simple magic number with each op, to detect when
the reader gets desynchronized from the writer.

Change-Id: Iac7dab7f465b0021b1d7ae31c8f8a353ac3663a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/299769
Trust: Keith Randall <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dan Scales <[email protected]>

* runtime: add pollDesc partial edges

gscan is taken during stack growth, which may occur while pollDesc is
held.

mallocgc may also be called while pollDesc is held. mallocgc may take
mheap or mheapSpecial. The former exists, but is out of order; the
latter is missing.

Fixes #44881

Change-Id: Ie25935d9d433e813c11a528ee47255b317a09f41
Reviewed-on: https://go-review.googlesource.com/c/go/+/300009
Trust: Michael Pratt <[email protected]>
Trust: Dan Scales <[email protected]>
Reviewed-by: Dan Scales <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>

* cmd/compile: detect duplicate importing earlier

Change-Id: I05ba944e189a884b727e40a9526d212612c3e923
Reviewed-on: https://go-review.googlesource.com/c/go/+/299770
Trust: Keith Randall <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dan Scales <[email protected]>

* cmd/compile: remove skipping of implicit operations during export

We'll need to attach types to these operations, so we need to
represent them in the import/export data.

Some of the operations use a selector indicating a different package,
so we need to provide an option to encode the package of a selector.
The default selector() function can't encode that extra information,
as selector's exact encoding is used by go/types.

Change-Id: I4c110fe347b3d915f88a722834bc4058baea7854
Reviewed-on: https://go-review.googlesource.com/c/go/+/299771
Trust: Keith Randall <[email protected]>
Run-TryBot: Keith Randall <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Dan Scales <[email protected]>

* cmd/compile: fix OpArg decomposer for registers in expandCalls

Includes test taken from
https://github.com/golang/go/issues/44816#issuecomment-791618179
and improved debugging output.

Updates #44816

Change-Id: I94aeb9c5255f175fe80727be29d218bad54bf7ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/299389
Trust: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: fix broken type+offset calc for register args

Includes more enhancements to debugging output.

Updates #44816.

Change-Id: I5b21815cf37ed21e7dec6c06f538090f32260203
Reviewed-on: https://go-review.googlesource.com/c/go/+/299409
Trust: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: fix confusion in generating SelectN index

Old: return the ABI register index of the result (wrong!)
New: return the index w/in sequence of result registers (right!)

Fixed bug:
genCaller0/genCaller0.go:43:9: internal compiler error: 'Caller0':
   panic during schedule while compiling Caller0:
runtime error: index out of range [10] with length 9

Updates #44816.

Change-Id: I1111e283658a2d6422986ae3d61bd95d1b9bde5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/299549
Trust: David Chase <[email protected]>
Reviewed-by: Cherry Zhang <[email protected]>

* cmd/compile: fix failure to communicate between ABIinfo producer&consumer

ABI info producer and consumer had different ideas for register
order for parameters.

Includes a test, includes improvements to debugging output.

Updates #44816.

Change-Id: I4812976f7a6c08d6fc02aac1ec0544b1f141cca6
Reviewed-on: https://go-review.googlesource.com/c/go/+/299570
Trust: David Chase <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>

* net: don't append a dot to TXT records on Plan 9

TXT records are not domain names, so no need to call absDomainName.
The output now matches the pure Go resolver.

Fixes #44619

Change-Id: I1ebf09152ff5c0446d2e2b4c26671358892d9dc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/296589
Reviewed-by: David du Colombier <[email protected]>
Run-TryBot: David du Colombier <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: David du Colombier <[email protected]>
Trust: Ian Lance Taylor <[email protected]>

* runtime, cmd/link/internal/ld: disable memory profiling when data unreachable

If runtime.MemProfile is unreachable, default to not collecting any
memory profiling samples, to save memory on the hash table.

Fixes #42347

Change-Id: I9a4894a5fc77035fe59b1842e1ec77a1182e70c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/299671
Reviewed-by: Cherry Zhang <[email protected]>
Trust: Keith Randall <[email protected]>

* cmd/asm,cmd/compile: support 5 operand RLWNM/RLWMI on ppc64

These instructions are actually 5 argument opcodes as specified
by the ISA.  Prior to this patch, the MB and ME arguments were
merged into a single bitmask operand to workaround the limitations
of the ppc64 assembler backend.

This limitation no longer exists. Thus, we can pass operands for
these opcodes without having to merge the MB and ME arguments in
the assembler frontend or compiler backend.

Likewise, support for 4 operand variants is unchanged.

Change-Id: Ib086774f3581edeaadfd2190d652aaaa8a90daeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/298750
Reviewed-by: Lynn Boger <[email protected]>
Reviewed-by: Carlos Eduardo Seo <[email protected]>
Trust: Carlos Eduardo Seo <[email protected]>

* go/types: improve the positioning of broken import errors

The heuristic gopls uses to guess error spans can get tripped-up on
certain valid characters in an import path (for example '-').

Update the error for broken imports to capture the full import path
span, so that gopls doesn't need to rely on heuristics.

Change-Id: Ieb8e0dce11933643f701b32271ff5f3477fecaaa
Reviewed-on: https://go-review.googlesource.com/c/go/+/300169
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/compile/internal/types2: better error reporting framework (starting point)

Until now, errors which came with additional details (e.g., a declaration
cycle error followed by the list of objects involved in the cycle, one per
line) were reported as an ordinary error followed by "secondary" errors,
with the secondary errors marked as such by having a tab-indented error
message.

This approach often required clients to filter these secondary errors
(as they are not new errors, they are just clarifying a previously
reported error).

This CL introduces a new internal error_ type which permits accumulating
various error information that may then be reported as a single error.

Change-Id: I25b2f094facd37e12737e517f7ef8853d465ff77
Reviewed-on: https://go-review.googlesource.com/c/go/+/296689
Trust: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

* cmd/compile/internal/types2: remove concept of finals

This is a 1:1 port of the respective change in go/types
in https://golang.org/cl/299590.

Change-Id: I65ad723f2e21e3d95fc0b94665e0121e31871a48
Reviewed-on: https://go-review.googlesource.com/c/go/+/300250
Trust: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

* cmd/go: allow '+' in package import paths in module mode

This change upgrades x/mod to pull in the fix from CL 300149.

Fixes #44776.

Change-Id: I273f41df2abfff76d91315b7f19fce851c8770d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/300176
Trust: Bryan C. Mills <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
Reviewed-by: Jay Conrod <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* internal/poll: implement a pipe pool for splice() call

In scenarios where splice() is called, splice() is usually called not just once, but many times,
which means that a lot of pipes will be created and destroyed frequently, costing an amount of system resources
and slowing down performance, thus I suggest that we add a pipe pool for reusing pipes.

Benchmark tests:

goos: linux
goarch: amd64
pkg: internal/poll
cpu: AMD EPYC 7K62 48-Core Processor

name                  old time/op    new time/op    delta
SplicePipe-8            1.36µs ± 1%    0.02µs ± 0%   -98.57%  (p=0.001 n=7+7)
SplicePipeParallel-8     747ns ± 4%       4ns ± 0%   -99.41%  (p=0.001 n=7+7)

name                  old alloc/op   new alloc/op   delta
SplicePipe-8             24.0B ± 0%      0.0B       -100.00%  (p=0.001 n=7+7)
SplicePipeParallel-8     24.0B ± 0%      0.0B       -100.00%  (p=0.001 n=7+7)

name                  old allocs/op  new allocs/op  delta
SplicePipe-8              1.00 ± 0%      0.00       -100.00%  (p=0.001 n=7+7)
SplicePipeParallel-8      1.00 ± 0%      0.00       -100.00%  (p=0.001 n=7+7)

Fixes #42740

Change-Id: Idff654b7264342084e089b5ba796c87c380c471b
Reviewed-on: https://go-review.googlesource.com/c/go/+/271537
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Brad Fitzpatrick <[email protected]>

* testing: fix typo in a comment

Change-Id: I781808327be84113cd55c52bc214b821cd166114
Reviewed-on: https://go-review.googlesource.com/c/go/+/300269
Trust: fannie zhang <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

* crypto/rand: supports for getrandom syscall in DragonFlyBSD

Since the 5.7 release, DragonFlyBSD supports as well
the getrandom function, the actual stable is 5.8.

Change-Id: I2b8fc468771b10ac12b38ea7e8e5314342de6375
GitHub-Last-Rev: c5c496f41898d58f2c6f3ccc81f754792f49edbe
GitHub-Pull-Request: golang/go#42617
Reviewed-on: https://go-review.googlesource.com/c/go/+/269999
Run-TryBot: Ian Lance Taylor <[email protected]>
Trust: Ian Lance Taylor <[email protected]>
Trust: Tobias Klauser <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Tobias Klauser <[email protected]>

* cmd/go: avoid password prompts in TestScript/mod_get_private_vcs

In some cases, this test would prompt for interactive SSH passwords in
order to authenticate to github.com over SSH. Setting GIT_SSH_COMMAND
to /bin/false prevents that, while still provoking the desired Git
failure mode.

Updates #44904.

Change-Id: Idc9fe9f47d2ccb6c8a4ea988b73d9c8c774e4079
Reviewed-on: https://go-review.googlesource.com/c/go/+/300156
Trust: Bryan C. Mills <[email protected]>
Trust: Kevin Burke <[email protected]>
Run-TryBot: Bryan C. Mills <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Kevin Burke <[email protected]>

* runtime: update signature of reflectcall functions

reflectcall tail calls runtime.call{16,32,...} functions, so they
have the same signature as reflectcall. It is important for them
to have the correct arg map, because those functions, as well as
the function being reflectcall'd, could move the stack. When that
happens, its pointer arguments, in particular regArgs, need to be
adjusted. Otherwise it will still point to the old stack, causing
memory corruption.

This only caused failures on the regabi builder because it is the
only place where internal/abi.RegArgs is not a zero-sized type.

May fix #44821.

Change-Id: Iab400ea6b60c52360d0b43a793f6bfe50ca9989b
Reviewed-on: https://go-review.googlesource.com/c/go/+/300154
Trust: Cherry Zhang <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>

* cmd/compile: remove ".fp" fake arg

No longer needed with previous CLs.

Change-Id: I9a1c11092a2736c190fa8e8ddfbb913b708957eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/300155
Trust: Cherry Zhang <[email protected]>
Run-TryBot: Cherry Zhang <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: David Chase <[email protected]>

* runtime/race: update dead link

LLVM changed their main branch name, so this link didn't work anymore.

Change-Id: I4c3a67b26e2bda012071281e29ea3c932c185130
Reviewed-on: https://go-review.googlesource.com/c/go/+/300469
Trust: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
Reviewed-by: Dmitry Vyukov <[email protected]>
TryBot-Result: Go Bot <[email protected]>

* go/types: add missing build tag to api_go1.18_test.go

This file has a go:build comment without a corresponding +build comment.

Change-Id: Id01604242a14d8ead16ffb9aa1b45eef7706956a
Reviewed-on: https://go-review.googlesource.com/c/go/+/300450
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/gofmt: fix const association to avoid inaccurate comment

The const parseTypeParams was grouped with printer-related consts in
gofmt.go, implicitly suggesting that it must be kept in sync with
go/format/format.go.

Change-Id: Ia65dc15c27fef2c389f963071252adee32ec6bd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/300451
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* cmd/compile:  deal with helper generic types that add methods to T

Deal with cases like: 'type P[T any] T' (used to add methods to an
arbitrary type T), In this case, P[T] has kind types.TTYPEPARAM (as does
T itself), but requires more code to substitute than a simple TTYPEPARAM
T. See the comment near the beginning of subster.typ() in stencil.go.

Add new test absdiff.go. This test has a case for complex types (which
I've commented out) that will only work when we deal better with Go
builtins in generic functions (like real and imag).

Remove change in fmt.go for TTYPEPARAMS that is no longer needed (since
all TTYPEPARAMS have a sym) and was sometimes causing an extra prefix
when formatting method names.

Separate out the setting of a TTYPEPARAM bound, since it can reference
the TTYPEPARAM being defined, so must be done separately. Also, we don't
currently (and may not ever) need bounds after types2 typechecking.

Change-Id: Id173057e0c4563b309b95e665e9c1151ead4ba77
Reviewed-on: https://go-review.googlesource.com/c/go/+/300049
Run-TryBot: Dan Scales <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Dan Scales <[email protected]>
Trust: Robert Griesemer <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>

* archive/zip: fix panic in Reader.Open

When operating on a Zip file that contains a file prefixed with "../",
Open(...) would cause a panic in toValidName when attempting to strip
the prefixed path components.

Fixes CVE-2021-27919
Fixes #44916

Change-Id: Ic755d8126cb0897e2cbbdacf572439c38dde7b35
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1004761
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Katie Hockman <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/300489
Trust: Katie Hockman <[email protected]>
Run-TryBot: Katie Hockman <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>

* encoding/xml: prevent infinite loop while decoding

This change properly handles a TokenReader which
returns an EOF in the middle of an open XML
element.

Thanks to Sam Whited for reporting this.

Fixes CVE-2021-27918
Fixes #44913

Change-Id: Id02a3f3def4a1b415fa2d9a8e3b373eb6cb0f433
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1004594
Reviewed-by: Russ Cox <[email protected]>
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/300391
Trust: Katie Hockman <[email protected]>
Run-TryBot: Katie Hockman <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Alexander Rakoczy <[email protected]>
Reviewed-by: Filippo Valsorda <[email protected]>

* flag: panic if flag name begins with - or contains =

Fixes #41792

Change-Id: I9b4aae8a899e3c3ac9532d27932d275cfb1fab48
GitHub-Last-Rev: f06b1e17674bf77bdc2d3e798df4c4379748c8d2
GitHub-Pull-Request: golang/go#42737
Reviewed-on: https://go-review.googlesource.com/c/go/+/271788
Reviewed-by: Emmanuel Odeke <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
Run-TryBot: Emmanuel Odeke <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Rob Pike <[email protected]>

* runtime: check partial lock ranking order

To ease readability we typically keep the partial order lists sorted by
rank. This isn't required for correctness, it just makes it (slightly)
easier to read the lists.

Currently we must notice out-of-order entries during code review, which
is an error-prone process.

Add a test to enforce ordering, and fix the errors that have crept in.
Most of the existing errors were misordered lockRankHchan or
lockRankPollDesc.

While we're here, I've moved the correctness check that the partial
ordering satisfies…
@golang golang locked and limited conversation to collaborators Mar 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants