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/pprof: completely unable to use dumb tty #26254

Closed
rsc opened this issue Jul 6, 2018 · 12 comments
Closed

cmd/pprof: completely unable to use dumb tty #26254

rsc opened this issue Jul 6, 2018 · 12 comments
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Jul 6, 2018

I have TERM=dumb set in my environment. pprof should not be printing ANSI escape codes:

$ go tool pprof x.prof
�[0;31mMain binary filename not available.
�[0mType: alloc_space
Time: Jul 6, 2018 at 4:00pm (EDT)
Entering interactive mode (type "help" for commands, "o" for options)
@rsc
Copy link
Contributor Author

@rsc rsc commented Jul 6, 2018

Actually, the situation is far worse than I realized. If I continue the session:

$ go tool pprof x.prof
�[0;31mMain binary filename not available.
�[0mType: alloc_space
Time: Jul 6, 2018 at 4:00pm (EDT)
Entering interactive mode (type "help" for commands, "o" for options)
(pprof) top10
��[D �[D��[D �[D��[D �[D��[D �[D��[D �[D

The garbage at the bottom is me typing ^C, because top10 seems to be taking forever. But if I move over to Apple Terminal then top10 runs instantly. I conclude from this that not only is pprof printing garbage to my screen, it has lost the ability to read from standard input!

echo top10 | go tool pprof x.prof still works.

@rsc rsc changed the title cmd/pprof: prints garbage to terminal cmd/pprof: completely unable to use dumb tty Jul 6, 2018
@rsc
Copy link
Contributor Author

@rsc rsc commented Jul 6, 2018

@hyangah Are you still taking care of pprof?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 6, 2018

May be related to https://golang.org/cl/112436 for #14041.

@hyangah hyangah self-assigned this Jul 6, 2018
@robpike
Copy link
Contributor

@robpike robpike commented Jul 6, 2018

https://golang.org/cl/112436 is the biggest CL I have ever seen.

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jul 6, 2018

It currently uses golang.org/x/crypto/ssh/terminal and assumes that if terminal.MakeRaw works, terminal is supported.

What's the right way to detect dumb tty? Is checking TERM env var sufficient?

And, what's the easiest way to create a dumb tty environment for testing? (Just setting TERM=dumb doesn't do anything for me :-()

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 7, 2018

For many readline libraries, the environment variable TERM specifies the kind of terminal. This terminal is looked up in (on GNU/Linux systems) /usr/share/terminfo. That defines a set of character sequences corresponding to various capabilities. This all dates back to the long ago, when different terminals had different escape sequences.

TERM=dumb is a terminfo entry that doesn't have any special character sequences. For example, compare the behavior of gdb and TERM=dumb gdb when you type ^L.

At a quick glance, x/crypto/ssh/terminal doesn't use terminfo, but instead assumes that it is running on a VT100. This is mostly fine today as xterms implement the VT100 escape sequence. But using TERM=dumb still ought to avoid any special escape sequences.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 9, 2018

Change https://golang.org/cl/122597 mentions this issue: cmd/pprof: disable colorization of error message

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jul 9, 2018

Thanks for the info, Ian.

Not wanting to fix x/crypto/ssh/terminal to recognize terminfo now, I checked if we can simply disable the readline feature completely by checking the environment var TERM.

But here is a bad news - 'go tool pprof' sets the "TERM" env var to "dumb" unconditionally.
https://golang.org/src/cmd/go/internal/envcmd/env.go?s=#L46

cl/122597 disables the colorization code that uses the troublesome ANSI escape code.
I don't know how to debug/diagnose the second issue (i.e. unable to read in the input) because I can't set up the identical environment for reproducing the issue.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jul 9, 2018

We can change the go tool to only set TERM when invoking non-Go tools. But maybe not for 1.11.

@hyangah
Copy link
Contributor

@hyangah hyangah commented Jul 9, 2018

Reproduced the problem similar to the second issue using emacs shell.
When the enter key is pressed in the interactive mode, the input ends with '\n' in the restricted, dumb terminal mode in emacs. In a regular non-dumb terminal mode, the key is translated to '\r'.

https://github.com/golang/crypto/blob/master/ssh/terminal/terminal.go#L116 uses only '\r' for enter key, so the readline hangs.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 10, 2018

Change https://golang.org/cl/122879 mentions this issue: cmd/pprof: disable realine ui support for TERM=dumb

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 10, 2018

Change https://golang.org/cl/122975 mentions this issue: cmd/go, cmd/cgo: only set TERM=dumb when running the compiler

gopherbot pushed a commit that referenced this issue Jul 10, 2018
The clang compiler on some terminals will issue colored error
messages, which can confuse tools like cgo. To avoid this we used to
set TERM=dumb for all programs started by the go tool. However, that
confuses the pprof tool, which doesn't know whether to support fancy
editing and colors itself.

Instead, change the go tool and the cgo tool to set TERM=dumb where it
matters--when invoking the C compiler--rather than in all cases.

Updates #26254

Change-Id: I95174f961ac269a50a83f5f9d268219043cba968
Reviewed-on: https://go-review.googlesource.com/122975
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot gopherbot closed this in 5e70b13 Jul 11, 2018
@golang golang locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.