[LLDB] [Windows] Allow making subprocesses use the debugger's console with LLDB_INHERIT_CONSOLE=true

[mstorsjo]

Summary

When running the lldb command line debugger in a console, it can be convenient to get the output of the debuggee inline in the same console.

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.

Also extend a nearby getenv() call to check for both "true" and "1".

There seems to be a number of different patterns for using getenv() in lldb; either just check for a variable being set (most common), or check for a number of different values. The check for LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE only used to check for the string "true", while e.g. the newly added check for LLDB_USE_LLDB_SERVER in source/Plugins/Process/Windows/Common/ProcessWindows.cpp checks for any of on/yes/1/true.

[labath]

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.

[clayborg]

When running the lldb command line debugger in a console, it can be convenient to get the output of the debuggee inline in the same console.

agreed. It just did the default behavior of launching in a command.exe since that is what windows does by default.

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.

(lldb) process launch --no-stdio

Also extend a nearby getenv() call to check for both "true" and "1".

There seems to be a number of different patterns for using getenv() in lldb; either just check for a variable being set (most common), or check for a number of different values. The check for LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE only used to check for the string "true", while e.g. the newly added check for LLDB_USE_LLDB_SERVER in source/Plugins/Process/Windows/Common/ProcessWindows.cpp checks for any of on/yes/1/true.

So by default Windows has done things differently since the start. The way things work now is:

• by default the program is launched in a terminal window on windows (right?)
• if you add --tty to "process launch" it probably does nothing as it is already doing this
• there is no way to just get the process output in the current lldb window

The way I would like to see things work if we can indeed get the output in the LLDB process:

• by default we don't open a console window, and just display the output in LLDB (do what LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE does?)
• if we specify --tty, then allow the process to be run in a separate window (like it defaults to currently)
• if we specify --disable--stdio, then disable STDIO (all output goes to equivalent of /dev/null)

Thoughts?

[mstorsjo]

Thanks for the excellent explanations of the situation so far. I'll have a look at how it behaves with input and other aspects of console sharing. You might be right that it can cause some issues, as I did run into cases where the main lldb console UI hung if the lldb-server subprocess crashed (due to unchecked Error), but I didn't realize and correlate this to the inherited/shared console at that time.

When passing flags like --tty or --disable-stdio to process launch, is it possible to set a persistent preference for that somehow via e.g. the .lldbinit file?

[amccarth]

labath has added great context here, and I generally agree with clayborg's ideal of optimal behavior. That said, if memory serves, getting that behavior on Windows can be quite challenging, so I'm not sure if it's worth the effort.

[labath]

In D70387#1750578, @mstorsjo wrote:

Thanks for the excellent explanations of the situation so far. I'll have a look at how it behaves with input and other aspects of console sharing. You might be right that it can cause some issues, as I did run into cases where the main lldb console UI hung if the lldb-server subprocess crashed (due to unchecked Error), but I didn't realize and correlate this to the inherited/shared console at that time.

You'll probably want to look at the IOHandler classes at some point. The way input forwarding works on unixes is that we create a separate IOHandler object when the process is running. This object reads the lldb input and forwards it to the inferior pty (instead of the "normal" IOHandler, which interprets the input as commands to execute). If you'll have the inferior read from the console directly, then we don't want to have the forwarding IOHandler there. However, we also don't want the "normal" IOHandler to be active, as that will race with the inferior in reading the input (I am not sure what windows does in these cases). So you may need to create some "fake" IOHandler, which will not read anything, but just prevent the regular IOHandler from kicking in.

It's good that you mentioned lldb-server, because it's presence complicates this behavior even further. If we wanted the console sharing approach to work, then we'd have to have both lldb-server and the inferior launched in the console-sharing mode, I presume? This would be quite different from the unix scenario where we can just send the pty device name to the lldb-server, and have it connect the inferior to that. I'm not entirely sure what that means though...

When passing flags like --tty or --disable-stdio to process launch, is it possible to set a persistent preference for that somehow via e.g. the .lldbinit file?

There's a target.disable-stdio setting. You could set that in your .lldbinit, if you wanted to. I doesn't look like there is a --tty equivalent of that, but I don't see a problem with adding one.

[mstorsjo]

In D70387#1751090, @labath wrote:

It's good that you mentioned lldb-server, because it's presence complicates this behavior even further. If we wanted the console sharing approach to work, then we'd have to have both lldb-server and the inferior launched in the console-sharing mode, I presume? This would be quite different from the unix scenario where we can just send the pty device name to the lldb-server, and have it connect the inferior to that. I'm not entirely sure what that means though...

Yeah, I hadn't really thought much about that initially, and when using the environment variable, it was propagated implicitly to lldb-server as well, so this aspect worked just as I would have hoped.

When passing flags like --tty or --disable-stdio to process launch, is it possible to set a persistent preference for that somehow via e.g. the .lldbinit file?

There's a target.disable-stdio setting. You could set that in your .lldbinit, if you wanted to. I doesn't look like there is a --tty equivalent of that, but I don't see a problem with adding one.

Ok, thanks for the pointers!

So it turns out this is a harder (and messier) thing than I first expected (even though my naive fix worked fine for the simplest cases at least). I'm not sure if I have time to actually complete this right now, so it might end up on the pile of unfinished things for now.

FWIW, I did test with a command line executable that does a loop of fgets+printf, to read lines of input and echo them back. This seems very prone to hanging; on arm64 I manage to read one line and print it back, before the debuggee becomes hung, stuck in some IO write call, and later on (after interrupting and resuming it) the debugger itself also seems to easily get hung, waiting for some event from the debuggee. On x86_64, the same setup even seems to hang even earlier, before managing to type the first line to the separate console.

lldb/source/Host/windows/ProcessLauncherWindows.cpp

ProcessLauncherWindows::LaunchProcess(const ProcessLaunchInfo &launch_info,
   startupinfo.hStdInput =
       stdin_handle ? stdin_handle : ::GetStdHandle(STD_INPUT_HANDLE);
   startupinfo.hStdOutput =
       stdout_handle ? stdout_handle : ::GetStdHandle(STD_OUTPUT_HANDLE);
 
   const char *hide_console_var =
       getenv("LLDB_LAUNCH_INFERIORS_WITHOUT_CONSOLE");
   if (hide_console_var &&
-      llvm::StringRef(hide_console_var).equals_lower("true")) {
+      (llvm::StringRef(hide_console_var).equals_lower("true") ||
+       llvm::StringRef(hide_console_var).equals_lower("1"))) {
     startupinfo.dwFlags |= STARTF_USESHOWWINDOW;
     startupinfo.wShowWindow = SW_HIDE;
   }
 
   DWORD flags = CREATE_NEW_CONSOLE | CREATE_UNICODE_ENVIRONMENT;
   if (launch_info.GetFlags().Test(eLaunchFlagDebug))
     flags |= DEBUG_ONLY_THIS_PROCESS;
 
-  if (launch_info.GetFlags().Test(eLaunchFlagDisableSTDIO))
+  const char *inherit_console_var = getenv("LLDB_INHERIT_CONSOLE");
+  if (launch_info.GetFlags().Test(eLaunchFlagDisableSTDIO) ||
+      (inherit_console_var &&
+       (llvm::StringRef(inherit_console_var).equals_lower("true") ||
+        llvm::StringRef(inherit_console_var).equals_lower("1"))))
     flags &= ~CREATE_NEW_CONSOLE;
 
   LPVOID env_block = nullptr;
   ::CreateEnvironmentBuffer(launch_info.GetEnvironment(), environment);
   if (!environment.empty())
     env_block = environment.data();
 
   executable = launch_info.GetExecutableFile().GetPath();