In D55582#1331145, @labath wrote:

Hm.. so the way, I'd imagine this working is that -o and friends (as well as pretty much every other command line argument) would be captured by the SB recorder, once they cross the SB boundary. For the -o commands that would be the moment the driver calls m_debugger.SetInputFileHandle https://github.com/llvm-mirror/lldb/blob/master/tools/driver/Driver.cpp#L697 , followed by m_debugger.RunCommandInterpreter a couple of lines lower. So, the SB recorder would notice that we're changing what the debugger considers as it's stdin, and direct the "stdin" recorder (or create a new instance of it, or whatever), to capture it.

Then, during replay, the SB replayer would notice that the driver called SBDebugger::SetInputFileHandle, so it would create a fake FILE* object (or maybe just open the file containing the captured commands from the disk?), and pass *that* as the stdin handle. Then it could just call SBDebugger::RunCommandInterperer the same way as the driver did. And I am hoping that pretty much everything below RunCommandInterpreter could run oblivious to the fact that replay is taking place.

I played around with this today:

Sorry for the late review, I had a look originally but didn't have any comments. LGTM!

In D55582#1329479, @labath wrote:

I am very sorry about this, but I am having some second thoughts about the RunReplay thingy. I don't think I've correctly expressed what is bothering me about it (and that's probably because I wasn't clear with myself about what is bothering me). I'll try to formulate it better this time

Address Adrian's feedback

When sourcing a file we should also ignore the individual commands otherwise they end up getting executed twice, once as part of the command source and once for every individual command in the file.

Skip driver logic when replaying, otherwise commands are executed or sourced twice, once in the driver and once by replaying the commands.

Address Pavel's comments.

Remove changes that introduced static_cast<bool>

In D55584#1328087, @labath wrote:

I also find the static_cast<bool> thingy weird. The rest of the changes seem to be towards the better (based on a pseudo-random sample), but the change is a quite big.

I'm going to abandon this for now until we have better tooling to address Jim's concerns here. I'm also not super happy with the formatting, which seems to be off in quite a few locations. The problem is that clang-tidy knows to reformat the source range it touched, but in this case the code below is affected. I hacked this up by having clang-format looking one line below the lines that were changed, but looks like that was not sufficient. Also a few case are covered by D55584 so definitely want to land that first.

Feedback Adrian

Add more pages

• Rebase
• Copy over permissions
• Fix path when launching.

ping

LGTM with the question answered/addressed. Thanks Stefan, these patches are really great work!

This LGTM if it works on Windows.

What happens today with NVPTX for other debug info sections that use label arithmetic? I know for sure that the accelerator tables/debug_names is using this, but maybe they're not used with this target?

LGTM. Thanks!

I'm in favor of using labels for the reasons you mentioned. It's also consistent with what we do elsewhere.

Initialize the FS in the unit test.

In D55038#1317099, @stella.stamenova wrote:

One of these tests fail on Windows (TestDriverOptions):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/1848/steps/test/logs/stdio

In D55038#1314336, @labath wrote:

In D55038#1314026, @JDevlieghere wrote:

Test didn't run. Is there a way to REQUIRE either darwin or linux?

I think the canonical way to do that would be to define a new feature in lit, which gets set when the target supports remote debugging and then use that feature in the REQUIRES directive.

Make initialize return an error.

This makes sense to me. I'm don't know if there's a better property but I think this matches the intended use, so I think it is fine.

ping

• Get the external path with modifying the VFS in LLVM.
• Integrate with the new integration logic.
• Add a test. (that doesn't work yet, still WIP)

Test didn't run. Is there a way to REQUIRE either darwin or linux?

Move RedirectingFileSystem interface into header so we can extend it from LLDB.

Address feedback from Pavel.