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.

Sign up for GitHub

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

Jump to bottom

workflowcheck v0.1.0 fails with Go 1.22 #1382

Closed
dancavallaro opened this issue Feb 8, 2024 · 5 comments · Fixed by uber/cadence#5915
Closed

workflowcheck v0.1.0 fails with Go 1.22 #1382

dancavallaro opened this issue Feb 8, 2024 · 5 comments · Fixed by uber/cadence#5915
Labels
potential-bug

Comments

@dancavallaro
Copy link

Expected Behavior

workflowcheck@latest (which points to 0.1.0, the only version available at pkg.go.dev) should work with the Go 1.22 toolchain.

Actual Behavior

When built with Go 1.22, workflowcheck fails with:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1908f0]

goroutine 68 [running]:
go/types.(*Checker).handleBailout(0x400019e200, 0x4000247b98)
	/usr/local/go/src/go/types/check.go:367 +0x9c
panic({0x316ee0?, 0x5b3c50?})
	/usr/local/go/src/runtime/panic.go:770 +0x124
go/types.(*StdSizes).Sizeof(0x0, {0x3ee798, 0x5b78a0})
	/usr/local/go/src/go/types/sizes.go:228 +0x320
go/types.(*Config).sizeof(...)
	/usr/local/go/src/go/types/sizes.go:333
go/types.representableConst.func1({0x3ee798?, 0x5b78a0?})
	/usr/local/go/src/go/types/const.go:76 +0x9c
go/types.representableConst({0x3efe30, 0x5ac4e0}, 0x400019e200, 0x5b78a0, 0x4000245f98)
	/usr/local/go/src/go/types/const.go:92 +0x138
go/types.(*Checker).representation(0x400019e200, 0x4000071600, 0x5b78a0)
	/usr/local/go/src/go/types/const.go:256 +0x68
go/types.(*Checker).implicitTypeAndValue(0x400019e200, 0x4000071600, {0x3ee798, 0x5b78a0})
	/usr/local/go/src/go/types/expr.go:375 +0x340
go/types.(*Checker).convertUntyped(0x400019e200, 0x4000071600, {0x3ee798, 0x5b78a0})
	/usr/local/go/src/go/types/const.go:289 +0x30
go/types.(*Checker).matchTypes(0x400019e200, 0x40000715c0, 0x4000071600)
	/usr/local/go/src/go/types/expr.go:926 +0x7c
go/types.(*Checker).binary(0x400019e200, 0x40000715c0, {0x3ef340, 0x40000743c0}, {0x3eefb0, 0x40000109c0}, {0x3ef850, 0x40000109e0}, 0x28, 0x359a)
	/usr/local/go/src/go/types/expr.go:800 +0x114
go/types.(*Checker).exprInternal(0x400019e200, 0x0, 0x40000715c0, {0x3ef340, 0x40000743c0}, {0x0, 0x0})
	/usr/local/go/src/go/types/expr.go:1416 +0x1d4
go/types.(*Checker).rawExpr(0x400019e200, 0x0, 0x40000715c0, {0x3ef340?, 0x40000743c0?}, {0x0?, 0x0?}, 0x0)
	/usr/local/go/src/go/types/expr.go:979 +0x12c
go/types.(*Checker).expr(0x400019e200, 0x3ee540?, 0x40000715c0, {0x3ef340?, 0x40000743c0?})
	/usr/local/go/src/go/types/expr.go:1513 +0x38
go/types.(*Checker).stmt(0x400019e200, 0x0, {0x3ef6a0, 0x4000070880})
	/usr/local/go/src/go/types/stmt.go:570 +0xdb0
go/types.(*Checker).stmtList(0x400019e200, 0x0, {0x4000010b40?, 0x0?, 0x0?})
	/usr/local/go/src/go/types/stmt.go:121 +0x88
go/types.(*Checker).funcBody(0x400019e200, 0x3ee798?, {0x400000e3f0?, 0x5b7a80?}, 0x4000071300, 0x4000074450, {0x0?, 0x0?})
	/usr/local/go/src/go/types/stmt.go:41 +0x21c
go/types.(*Checker).funcDecl.func1()
	/usr/local/go/src/go/types/decl.go:852 +0x44
go/types.(*Checker).processDelayed(0x400019e200, 0x0)
	/usr/local/go/src/go/types/check.go:467 +0x12c
go/types.(*Checker).checkFiles(0x400019e200, {0x4000048120, 0x1, 0x1})
	/usr/local/go/src/go/types/check.go:411 +0x188
go/types.(*Checker).Files(...)
	/usr/local/go/src/go/types/check.go:372
golang.org/x/tools/go/packages.(*loader).loadPackage(0x4000120000, 0x40001729c0)
	/go/pkg/mod/golang.org/x/tools@v0.10.0/go/packages/packages.go:1055 +0x870
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1()
	/go/pkg/mod/golang.org/x/tools@v0.10.0/go/packages/packages.go:854 +0x178
sync.(*Once).doSlow(0x0?, 0x0?)
	/usr/local/go/src/sync/once.go:74 +0x100
sync.(*Once).Do(...)
	/usr/local/go/src/sync/once.go:65
golang.org/x/tools/go/packages.(*loader).loadRecursive(0x0?, 0x0?)
	/go/pkg/mod/golang.org/x/tools@v0.10.0/go/packages/packages.go:842 +0x50
golang.org/x/tools/go/packages.(*loader).loadRecursive.func1.1(0x0?)
	/go/pkg/mod/golang.org/x/tools@v0.10.0/go/packages/packages.go:849 +0x30
created by golang.org/x/tools/go/packages.(*loader).loadRecursive.func1 in goroutine 16
	/go/pkg/mod/golang.org/x/tools@v0.10.0/go/packages/packages.go:848 +0x84
exit status 2

As far as I can tell, this is a known/expected issue resulting from a Go change in 1.22: golang/go#62167. Other libraries/packages have also run into this and fixed it, e.g. ent/ent#3864.

And indeed from what I can tell, you've already fixed this in workflowcheck on HEAD, you just need to publish a new version. TL;DR of the issue, as I understand it, is that Go analyzers when built with the 1.22 toolchain also require a version of x/tools at least as new as v0.13.0. workflowcheck's go.mod has x/tools v0.13.0 now in the repo, but workflowcheck v0.1.0 depends on x/tools v0.10.0.

Steps to Reproduce the Problem

Minimal example showing the error with Go 1.22:

> cat main.go
package main

import "fmt"

func main() {
	fmt.Println("Hello, world!")
}
> docker run -it --rm -v .:/workflowcheck --workdir /workflowcheck golang:1.22-bookworm sh -c "go run go.temporal.io/sdk/contrib/tools/workflowcheck@latest main.go"
go: downloading go.temporal.io/sdk v1.25.1
go: downloading go.temporal.io/sdk/contrib/tools/workflowcheck v0.1.0
go: downloading golang.org/x/tools v0.10.0
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading golang.org/x/sys v0.9.0
go: downloading golang.org/x/mod v0.11.0
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1908f0]

But it works fine with Go 1.21.7, as expected:

> docker run -it --rm -v .:/workflowcheck --workdir /workflowcheck golang:1.21.7-bookworm sh -c "go run go.temporal.io/sdk/contrib/tools/workflowcheck@latest main.go"
go: downloading go.temporal.io/sdk/contrib/tools/workflowcheck v0.1.0
go: downloading go.temporal.io/sdk v1.25.1
go: downloading golang.org/x/tools v0.10.0
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading golang.org/x/sys v0.9.0
go: downloading golang.org/x/mod v0.11.0
>

Alternatively, a consumer can work around it by adding workflowcheck to one's go.mod/tools.go while ensuring you have a version of x/tools at least as new as v0.13.0, and then omitting @latest when go installing workflowcheck so that it builds in module-aware mode, using the newer version of x/tools from the consumer's go.mod.

> cat go.mod
module github.com/dancavallaro

go 1.22.0

require go.temporal.io/sdk/contrib/tools/workflowcheck v0.1.0

require (
	golang.org/x/mod v0.14.0 // indirect
	golang.org/x/tools v0.17.0 // indirect
	gopkg.in/yaml.v2 v2.4.0 // indirect
)
> docker run -it --rm -v .:/workflowcheck --workdir /workflowcheck golang:1.22-bookworm sh -c "go run go.temporal.io/sdk/contrib/tools/workflowcheck main.go"
go: downloading go.temporal.io/sdk/contrib/tools/workflowcheck v0.1.0
go: downloading golang.org/x/tools v0.17.0
go: downloading gopkg.in/yaml.v2 v2.4.0
go: downloading golang.org/x/mod v0.14.0
>

That seems like a best practice anyway so I'll stick with that, but nonetheless wanted to give you a heads up about this issue so you can publish a new version of workflowcheck.

Specifications

  • Version: Go 1.22 toolchain, with workflowcheck v0.1.0
  • Platform:
> uname -a
Darwin MAC-KJ9HW94GKX 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:31:00 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6020 arm64
> docker run -it --rm -v .:/workflowcheck --workdir /workflowcheck golang:1.22-bookworm uname -a
Linux c23f0ee858ea 6.5.11-linuxkit #1 SMP PREEMPT Wed Dec  6 17:08:31 UTC 2023 aarch64 GNU/Linux
@Quinn-With-Two-Ns
Copy link
Contributor

There is already a fix merged for this 72a5b6f. We will release a v0.2.0 after the Go SDK is released.

@dancavallaro
Copy link
Author

Roger! I did see that you'd upgraded x/tools already but I wasn't sure if this specific issue was on your radar for an upcoming release. Thanks for confirming!

@tssurya tssurya mentioned this issue Feb 13, 2024
Merged
@cretz cretz mentioned this issue Feb 29, 2024
Closed
@jth-0 jth-0 mentioned this issue Mar 4, 2024
Open
@guygof
Copy link

guygof commented Mar 12, 2024 •
edited
Loading

There is already a fix merged for this 72a5b6f. We will release a v0.2.0 after the Go SDK is released.

I saw 1.26.0 was released with the fix - will workflowcheck 0.2.0 be released soon? I see the latest version still crashes.
Thanks @Quinn-With-Two-Ns !

@Quinn-With-Two-Ns
Copy link
Contributor

Yes it will be

@Quinn-With-Two-Ns
Copy link
Contributor

Closing since workflowcheck v0.2.0 is released https://github.com/temporalio/sdk-go/releases/tag/contrib%2Ftools%2Fworkflowcheck%2Fv0.2.0

@Groxx Groxx mentioned this issue Apr 17, 2024
Merged
Groxx added a commit to uber/cadence that referenced this issue Apr 17, 2024
@Groxx
Bugfix for enumer in go 1.22 (#5915)
d0661b7 
Enumer is currently crashing if `make go-generate` is run with Go 1.22: alvaroloes/enumer#71
Temporal helpfully found an easy fix: temporalio/sdk-go#1382
For the underlying cause: golang/go#62167
So I've mimicked that by just doing a `go get golang.org/x/tools@latest` in the tools-module.

And now `make clean` -> `GOTOOLCHAIN=[go1.20.1 or go1.22.1] make go-generate` both work correctly.
(you need Go 1.21 or newer to use GOTOOLCHAIN.  highly recommended!)

Since this only affects build-time tools and doesn't change any generated code, it seems trivially safe, but I have not checked what all has changed in golang.org/x/tools across these versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
potential-bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bugfix for enumer in go 1.22 Groxx/cadence
3 participants
@dancavallaro @Quinn-With-Two-Ns @guygof