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

Jump to bottom

cmd/pprof: unescaped labels cause dot output to be malformed #53703

Open
Snaipe opened this issue Jul 6, 2022 · 6 comments
Open

cmd/pprof: unescaped labels cause dot output to be malformed #53703

Snaipe opened this issue Jul 6, 2022 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@Snaipe
Copy link

Snaipe commented Jul 6, 2022

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

$ go version
1.18.3

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/snaipe/.local/var/cache/go-build"
GOENV="/home/snaipe/.local/etc/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/snaipe/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/snaipe/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
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-build4178454973=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Label values don't seem to be escaped in the dot output, causing the result to be malformed. For example, given this test program: https://go.dev/play/p/duT2NS2iD7I

Running the program prints the profile base64-encoded to be demonstrable on the go playground. Using go tool pprof on the decoded profile shows the following (emphasis mine):

File: play
Type: cpu
Time: Nov 11, 2009 at 12:00am (CET)
Duration: 200ms, Total samples = 350ms (175.00%)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) tags
 unescaped: Total 350.0ms
            350.0ms (  100%): some "string" with quotes

(pprof) svg
Error: <stdin>: syntax error in line 5 near '"'
failed to execute dot. Is Graphviz installed? Error: exit status 1
(pprof) dot
digraph "play" {
node [style=filled fillcolor="#f8f8f8"]
subgraph cluster_L { "File: play" [shape=box fontsize=16 label="File: play\lType: cpu\lTime: Nov 11, 2009 at 12:00am (CET)\lDuration: 200ms, Total samples = 350ms (175.00%)\lShowing nodes accounting for 350ms, 100% of 350ms total\l\lSee https://git.io/JfYMW for how to read the graph\l" tooltip="play"] }
N1 [label="sha256\nblock\n270ms (77.14%)" id="node1" fontsize=24 shape=box tooltip="crypto/sha256.block (270ms)" color="#b20d00" fillcolor="#edd7d5"]
N1_0 [label = "unescaped:some "string" with quotes" id="N1_0" fontsize=8 shape=box3d tooltip="270ms"]

<snip>

As you can see, the label = "unescaped:some "string" with quotes" does not parse because "string" terminates the label string. The label should have been label = "unescaped:some \"string\" with quotes" instead.

I confirmed that the correction works by running sed 's/"string"/\"string\"/g' on the generated profile.dot.

See attached profile.zip for the profile file used in this session, as well as expected dot and expected resulting svg.
@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 6, 2022
@heschi heschi added this to the Backlog milestone Jul 6, 2022
@heschi
Copy link
Contributor

heschi commented Jul 6, 2022

cc @golang/runtime

cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 7, 2022
@cuiweixie
runtime: pprof label need to escape double quote
2cc08d5 
golang#53703
@cuiweixie cuiweixie mentioned this issue Jul 7, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/416355 mentions this issue: runtime: pprof label need to escape double quote

@prattmic
Copy link
Member

prattmic commented Jul 7, 2022

Thanks for the report.

This certainly needs to be fixed, but I think it should be fixed upstream in https://github.com/google/pprof (which we pull back into go tool pprof). pprof labels are just arbitrary strings, I don't think it makes sense to modify them with "escaping" on the Go side (plus how do we know what form of escaping is correct for all downstream usage?).

In https://github.com/google/pprof/blob/c488b8fa1db3fa467bf30beb5a1d6f4f10bb1b87/internal/graph/dotgraph.go#L250, I see escaping (escapeForDot) on a variety of values, but not on the labels. This bug could affect dot/svg generation in pprof from profiles for sources other than Go as well.

@aalexand @evanj

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@Snaipe
Copy link
Author

Snaipe commented Jul 7, 2022

Ah, I wasn't aware that go tool pprof was pulling in google/pprof -- yes, it makes sense to fix it there.

@prattmic
Copy link
Member

prattmic commented Jul 7, 2022

Sounds good, please open an issue on https://github.com/google/pprof. We pull the latest pprof every release, so a fix will be included for go tool pprof in a subsequent Go release.

@prattmic prattmic closed this as completed Jul 7, 2022
@prattmic prattmic reopened this Jul 7, 2022
@evanj
Copy link
Contributor

evanj commented Jul 7, 2022 •
edited

My PR google/pprof#689 should fix this bug. It is waiting on review (although there is an edge case related to non-printable characters that I would like to revisit).

@evanj evanj mentioned this issue Jul 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
Go Compiler / Runtime
Status: Done
Milestone
Backlog
Development

No branches or pull requests
5 participants
@evanj @Snaipe @prattmic @gopherbot @heschi