[Lldb-commits] [PATCH] D70387: [LLDB] [Windows] Allow making subprocesses use the debugger's console with LLDB_INHERIT_CONSOLE=true

Mon Nov 18 05:54:57 PST 2019

labath added a comment.

> The same behaviour also seems possible to enable by setting the eLaunchFlagDisableSTDIO flag somehow, but I wasn't able to figure out how to set that when calling the normal lldb.exe.

You can set this flag via the `-n`/`--disable-stdio` flag of "process launch". However, I think you've got the meaning of that flag backwards -- on unix systems, the *default* behavior is for the inferior to "share" the terminal of the lldb process, and this flag disables it.

The sharing part is in quotes, because that's not really how that works in lldb. What we do is create a new "pty" which the process gets connected to, and then lldb forwards I/O from that pty to its own terminal manually. This enables to better control the "sharing" aspects of terminal output than what would be possible if the process was writing there directly. In also enables us to make the process output available programatically (SBProcess::GetSTDOUT). On top of that, there are various flags which modify this behavior. `eLaunchFlagDisableSTDIO` disables stdio completely (redirects to /dev/null or something). `eLaunchFlagLaunchInTTY` launches it in a fresh terminal window (currently only works on osx, I believe).

On windows the situation is more complicated. There are no pseudo terminals, so doing the same forwarding tricks was considered hard (though I understand now that this is still possible with sufficient effort). That's why it was chosen that the default launch behavior is launching it in a new terminal, as if `eLaunchFlagLaunchInTTY` was specified, but completely ignoring the actual value of that flag. However, *then* it was seen that this is really annoying when running the test suite, because it ends up opening zillions of terminal windows, and so the `LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE` variable was introduced.

This is the main reason why this behavior is controlled by an environment variable instead of a flag or something -- it's much easier to set that from a test harness. And, it was never meant to be a user-facing feature.

What you're proposing here sounds like a user-facing feature, and so I don't think it should be controlled by an environment variable. I see two paths forward here:

1. a short and hacky one: change the meaning (and possibly name) of `LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE` -- right now that creates a hidden console, you could change it to re-use the lldb console. This would make the lldb behavior with this variable kind of similar to the default behavior on other platforms. However, I still wouldn't advertise it as a user-facing feature anywhere. Also, this would only work if the test suite is actually happy with this kind of change.
2. the "right" way: Make this a proper setting controllable by a command line flag, etc. -- for this, I think we could/should recycle `eLaunchFlagLaunchInTTY`, as that's pretty much what it is supposed to do. However, we'd probably want to make it so that the default behavior on windows is to launch in a new window.(*) This could be done by changing the launch code so that it asks the Platform class for the default value of this flag if it is not specified.

(*) The main reason I think this should be the default is because this behavior would not really be the same as on posix systems -- for instance you couldn't get the output programatically, or type input the the process. Or maybe you could? I haven't checked that part. If that works, then maybe this could indeed be the default. That's probably for you windows people to decide on.

> There seems to be a number of different patterns for using getenv() in lldb

This is a consequence of these not being intended to be used by actual users. I wouldn't worry too much about making those consistent.

Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70387/new/

https://reviews.llvm.org/D70387