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

runtime: stack trace on panic does not include all parameters in 1.17 #47159

Closed
stapelberg opened this issue Jul 13, 2021 · 3 comments
Closed

runtime: stack trace on panic does not include all parameters in 1.17 #47159

stapelberg opened this issue Jul 13, 2021 · 3 comments
Labels
Milestone

Comments

@stapelberg
Copy link
Contributor

@stapelberg stapelberg commented Jul 13, 2021

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

$ go version
go version go1.17beta1 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/michael/.cache/go-build"
GOENV="/home/michael/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/michael/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/michael/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/michael/sdk/go1.17beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/michael/sdk/go1.17beta1/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17beta1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/michael/go/src/github.com/maruel/panicparse/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-build258492713=/tmp/go-build -gno-record-gcc-switches"

What did you do?

git clone https://github.com/maruel/panicparse
cd panicparse/cmd/panic
go run -gcflags '-N -l' main.go main_no_race.go args_elided

What did you expect to see?

1.16 output:

go run -gcflags '-N -l' main.go main_no_race.go args_elided
GOTRACEBACK=
panic: 1

goroutine 1 [running]:
main.panicArgsElided(0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa, ...)
	/root/panicparse/cmd/panic/main.go:105 +0x4f
main.glob..func1()
	/root/panicparse/cmd/panic/main.go:221 +0x88
main.main()
	/root/panicparse/cmd/panic/main.go:72 +0x3f9
exit status 2

What did you see instead?

GOTRACEBACK=
panic: 1

goroutine 1 [running]:
main.panicArgsElided(0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xa)
	/home/michael/go/src/github.com/maruel/panicparse/cmd/panic/main.go:105 +0x5f
main.glob..func1()
	/home/michael/go/src/github.com/maruel/panicparse/cmd/panic/main.go:221 +0x5b
main.main()
	/home/michael/go/src/github.com/maruel/panicparse/cmd/panic/main.go:72 +0x662
exit status 2

Note that the last parameter is neither included, nor indicated as elided.

This seems like a regression compared to go 1.16.

related to maruel/panicparse#61

might be the same as #45728 but I figured I’d file a separate one just to be sure

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Jul 13, 2021

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 13, 2021

Thanks for reporting. There may be an off-by-one error in argument counting code. If there are 10 args, it prints the 10 args without ... correctly. If there are 12 args, it prints the first 10 and then ..., correctly as well. It doesn't print ... when there are exactly 11 args. I'll look into it.

Loading

@cherrymui cherrymui added this to the Go1.17 milestone Jul 13, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 14, 2021

Change https://golang.org/cl/334710 mentions this issue: cmd/compile: fix off-by-one error in traceback argument counting

Loading

@gopherbot gopherbot closed this in 404127c Jul 19, 2021
steeve added a commit to znly/go that referenced this issue Aug 19, 2021
For traceback argument printing, we want to print at most 10
words, then print "..." if there are still more args and/or
fields. The current code has off-by-one error that for 11
non-aggregate typed args, it prints the first 10 but without the
"...". Also, for aggregate-typed args, in some cases it may print
an extra "..." when there is actually no more fields.

The problem for this is that visitType return false (meaning not
to continue visiting) if it reaches the limit anywhere during the
recursive visit. It doesn't distinguish whether it has printed
anything for the current arg. If it reaches the limit before it
prints anything, it means that we're visiting the extra arg/field,
so the caller should print "..." and stop. If it prints
something then reaches the limit, however, the caller should keep
going, and only print "..." at the next iteration when there is
actually an extra arg/field. This CL does so.

Fixes golang#47159.

Change-Id: I93fc25b73ada2b5a98df780c45e5b0c9565dc2fc
Reviewed-on: https://go-review.googlesource.com/c/go/+/334710
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants