labath added a comment.

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

What I was thinking about were the boundaries at which we want to record/replay 
events. If I understand correctly, one of those boundaries should be the SB 
layer (is that correct?). So, with that in mind, it makes sense for the lldb 
driver to do something "special" in replay mode, and invoke some other function 
which would replay the SB calls it would have normally made. That function 
might as well be called `RunReplay`.

Now, if we imagine we have this SB record/replay capability, during recording 
it would detect that the driver made a call to 
`SBDebugger::RunCommandInterpreter`. Then, during recording it would again make 
a call to SBDebugger::RunCommandInterpreter, but ensure that it behaves the 
same way as it did the first time (this is the auto-detection part I was 
speaking about).

The reason this came out all wrong is because I was also thinking about a 
(somewhat) unrelated thing, and that is the layer at which we do the capturing 
of the commands. I was thinking that maybe the command interpreter doesn't need 
to know anything about the record/replay taking place. If this SB replay engine 
substituted the `stdin` of the debugger with a recorded version of it, then it 
could go normally go about it's business and things would "just work" (you'd 
still have to make sure that libedit doesn't do something funny, but that could 
hopefully be done at the IOHandler level by substituting the editline IO 
handler).

So, I mean, that's how I'd do it, but this is your feature, and where you wan't 
to draw the record/replay line ultimately depends on what you want to get out 
of the feature. However, I do see some advantages to doing the r/r at a lower 
level that I'd like to try to sell to you:

- better possibility for capturing ^C. I think ^C is fairly important, 
particularly in console debugging, as it's the only way to interrupt a program 
without it voluntarily hitting a breakpoint. It will also be fairly tricky to 
reproduce. With the current framework, I'm not sure how you would manage to 
capture that. If you worked with IO handlers and was capturing stdin, I think 
you'd have a better chance of getting it right (since both stdin and ^C come 
from the terminal).
- no need for special handling of command sourcing. As you would capture 
almost-raw stdin, you wouldn't need to do anything special to the avoid sourced 
commands being captured twice. The filesystem would just provide the captured 
file, and stdin-recorder would capture the stdin. (which I think would make the 
stdin-capturer fit nicely with the Filesystem one, as they're both fairly 
low-level).
- right now you're merging commands coming from multiple sources (stdin via 
`RunCommandInterpreter`, command strings from `HandleCommand`) into one 
monolithic command stream. I think this means that once you have an SB 
recorder, it will also have to special case command APIs to avoid doing things 
twice. If you work at a lower level, this won't happen, because HandleCommand 
would be naturally captured as part of the SB stream, and stdin commands would 
be captured by the stdin-recorder (which would probably also be initialized and 
managed by the SB recorder, since it needs to be hooked into 
RunCommandInterpreter)

So, basically, my point is:

- let's bring back `RunReplay`. I don't think it makes sense to force this to 
go through `SBDebugger::RunCommandInterpreter` in the current setup, since both 
the caller and the callee special-case replay mode anyway.
- I'd urge you to consider whether you really want to do the capturing at this 
level


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

https://reviews.llvm.org/D55582



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to