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

Add Go 1.21 loopvar=2 GC flag in CI tests to surface loops which compile differently #145

Closed
atc0005 opened this issue Aug 18, 2023 · 2 comments
Assignees

Comments

@atc0005
Copy link
Owner

atc0005 commented Aug 18, 2023

From https://www.sethvargo.com/things-im-excited-for-in-go-1-21/:

...

The Go team made it very easy to identify places where your code might be affected by this change. Building your Go program with Go 1.21 and a special GC flag will print out the loops that are compiling differently:

go build -gcflags=all=-d=loopvar=2 ./...

I can add this as a new linting task specific to the stable go-ci image.

EDIT:

I can add this as a new linting task specific to a particular "mirror" go-ci image.

See later notes on this GH issue.

References:

@atc0005
Copy link
Owner Author

atc0005 commented Aug 23, 2023

From earlier testing as part of the work on atc0005/check-vmware#916:

podman container run --platform linux/amd64 --rm -it -v $PWD:/code -w /code golang:1.21.0
go build -gcflags=all=-d=loopvar=2 ./...

Output:

root@c70a67fbeea0:/code# go build -gcflags=all=-d=loopvar=2 ./...
# runtime
/usr/local/go/src/runtime/proc.go:1907:7: loop variable freem now per-iteration, stack-allocated
/usr/local/go/src/runtime/proc.go:3269:7: loop variable enum now per-iteration, stack-allocated
/usr/local/go/src/runtime/mgcmark.go:810:6: loop variable d now per-iteration, stack-allocated
/usr/local/go/src/runtime/traceback.go:623:7: loop variable iu now per-iteration, stack-allocated
/usr/local/go/src/runtime/traceback.go:980:7: loop variable iu now per-iteration, stack-allocated
# vendor/golang.org/x/net/dns/dnsmessage
/usr/local/go/src/vendor/golang.org/x/net/dns/dnsmessage/message.go:1161:10: loop variable q now per-iteration, stack-allocated
/usr/local/go/src/vendor/golang.org/x/net/dns/dnsmessage/message.go:1168:10: loop variable a now per-iteration, stack-allocated
/usr/local/go/src/vendor/golang.org/x/net/dns/dnsmessage/message.go:1175:10: loop variable a now per-iteration, stack-allocated
/usr/local/go/src/vendor/golang.org/x/net/dns/dnsmessage/message.go:1182:10: loop variable a now per-iteration, stack-allocated
/usr/local/go/src/vendor/golang.org/x/net/dns/dnsmessage/message.go:2633:9: loop variable o now per-iteration, stack-allocated
# net
/usr/local/go/src/net/cgo_unix.go:74:9: loop variable addr now per-iteration, stack-allocated
/usr/local/go/src/net/dnsclient_unix.go:567:9: loop variable ip now per-iteration, stack-allocated
/usr/local/go/src/net/interface_linux.go:27:9: loop variable m now per-iteration, stack-allocated
/usr/local/go/src/net/interface_linux.go:150:9: loop variable m now per-iteration, stack-allocated
/usr/local/go/src/net/sockopt_posix.go:28:9: loop variable ifi now per-iteration, heap-allocated
/usr/local/go/src/net/interface.go:147:9: loop variable ifi now per-iteration, heap-allocated (loop inlined into /usr/local/go/src/net/interface.go:139)
/usr/local/go/src/net/interface.go:147:9: loop variable ifi now per-iteration, heap-allocated (loop inlined into /usr/local/go/src/net/interface_linux.go:159)
# crypto/x509
/usr/local/go/src/crypto/x509/x509.go:1208:11: loop variable name now per-iteration, stack-allocated
/usr/local/go/src/crypto/x509/x509.go:1220:11: loop variable ipNet now per-iteration, stack-allocated
/usr/local/go/src/crypto/x509/x509.go:1228:11: loop variable email now per-iteration, stack-allocated
/usr/local/go/src/crypto/x509/x509.go:1240:11: loop variable uriDomain now per-iteration, stack-allocated
/usr/local/go/src/crypto/x509/verify.go:856:9: loop variable ext now per-iteration, heap-allocated
/usr/local/go/src/crypto/x509/verify.go:871:10: loop variable ext now per-iteration, heap-allocated
# crypto/tls
/usr/local/go/src/crypto/tls/common.go:1146:9: loop variable cert now per-iteration, heap-allocated
/usr/local/go/src/crypto/tls/ticket.go:121:10: loop variable extra now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/ticket.go:143:10: loop variable chain now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/ticket.go:150:12: loop variable cert now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/ticket.go:374:9: loop variable key now per-iteration, heap-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:195:12: loop variable proto now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:233:12: loop variable ks now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:268:12: loop variable psk now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:276:12: loop variable binder now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:355:11: loop variable binder now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:693:12: loop variable sct now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:1186:14: loop variable ca now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:1389:7: loop variable i now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:1389:10: loop variable cert now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_messages.go:1411:15: loop variable sct now per-iteration, stack-allocated
/usr/local/go/src/crypto/tls/handshake_client.go:1080:9: loop variable chain now per-iteration, heap-allocated
/usr/local/go/src/crypto/tls/handshake_server_tls13.go:186:10: loop variable ks now per-iteration, heap-allocated
# github.com/vmware/govmomi/vim25/xml
vendor/github.com/vmware/govmomi/vim25/xml/typeinfo.go:82:13: loop variable finfo now per-iteration, stack-allocated
# net/http/cookiejar
/usr/local/go/src/net/http/cookiejar/jar.go:188:10: loop variable e now per-iteration, stack-allocated
# github.com/vmware/govmomi/object
vendor/github.com/vmware/govmomi/object/authorization_manager.go:43:9: loop variable role now per-iteration, heap-allocated (loop inlined into <autogenerated>:1)
vendor/github.com/vmware/govmomi/object/authorization_manager.go:53:9: loop variable role now per-iteration, heap-allocated (loop inlined into <autogenerated>:1)
vendor/github.com/vmware/govmomi/object/custom_fields_manager.go:140:9: loop variable def now per-iteration, heap-allocated (loop inlined into <autogenerated>:1)
# github.com/atc0005/check-vmware/internal/vsphere
internal/vsphere/snapshots.go:877:10: loop variable snapTree now per-iteration, heap-allocated
internal/vsphere/snapshots.go:984:10: loop variable snapTree now per-iteration, heap-allocated
# github.com/atc0005/check-vmware/cmd/check_vmware_hs2ds2vms
cmd/check_vmware_hs2ds2vms/main.go:297:14: loop variable pairing now per-iteration, stack-allocated

root@c70a67fbeea0:/code# echo $?
0

root@c70a67fbeea0:/code# go build -gcflags=all=-d=loopvar=2 ./... 2>&1 | grep -Ev '/usr|vendor'
# runtime
# net
# crypto/x509
# crypto/tls
# github.com/vmware/govmomi/vim25/xml
# net/http/cookiejar
# github.com/vmware/govmomi/object
# github.com/atc0005/check-vmware/internal/vsphere
internal/vsphere/snapshots.go:877:10: loop variable snapTree now per-iteration, heap-allocated
internal/vsphere/snapshots.go:984:10: loop variable snapTree now per-iteration, heap-allocated

root@c70a67fbeea0:/code# go build -gcflags=all=-d=loopvar=2 ./... 2>&1 | grep -Ev '/usr|vendor|^#'
internal/vsphere/snapshots.go:877:10: loop variable snapTree now per-iteration, heap-allocated
internal/vsphere/snapshots.go:984:10: loop variable snapTree now per-iteration, heap-allocated

The latter approach will collect just the relevant bits I'm interested in.

The CI check could look for any captured content from that build attempt (e.g., as a Bash array) and emit the entries for review.

@atc0005
Copy link
Owner Author

atc0005 commented Feb 6, 2024

Based on the Go 1.22 release notes (https://go.dev/doc/go1.22), the 1.22 series should work just as well as the 1.21 series provided that the go.mod file does not specify a version of Go newer than 1.21.

To be on the safe side, I should probably explicitly specify a 1.21 series image (and disable any automatic "upgrade" of the toolchain).

atc0005 added a commit that referenced this issue Feb 22, 2024
Use Go 1.21 series "mirror" image to perform builds using the
`loopvar` compiler flag. This new workflow job will be ued to
identify loops that compile differently using the new loop
semantics introduced as a preview in Go 1.21 and enabled by
default with the release of Go 1.22.

This new workflow job is set to `continue-on-error` in order to
verify that it works as intended. Once it has been proven to
work as intended `continue-on-error` will be set to `false`
and any identified loop behavior changes will be considered
a real CI failure requiring resolution.

refs GH-145
@atc0005 atc0005 closed this as completed Feb 22, 2024
atc0005 added a commit to atc0005/send2teams that referenced this issue Feb 23, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
atc0005 added a commit to atc0005/send2teams that referenced this issue Feb 23, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
atc0005 added a commit to atc0005/dnsc that referenced this issue Mar 1, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
atc0005 added a commit to atc0005/dnsc that referenced this issue Mar 1, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
atc0005 added a commit to atc0005/check-mail that referenced this issue Mar 1, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
atc0005 added a commit to atc0005/check-mail that referenced this issue Mar 1, 2024
Workaround flagged loop behavior change by explicitly creating a
per-iteration loop variable.

refs:

- atc0005/shared-project-resources#145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant