[Lldb-commits] [PATCH] D55582: [Reproducers] Add command reproducer

Fri Dec 14 10:24:11 PST 2018

labath added a comment.

In D55582#1331333 <https://reviews.llvm.org/D55582#1331333>, @JDevlieghere wrote:

> Yes, this is how SB record/replay will work. I don't disagree with this
>  approach, it's just that, as I said in my earlier reply, I was hoping to get
>  the driver working without the need for the SB reproducers. Maybe I should give
>  up on that?

Hm... I don't know.. as this patch shows, it seems it is possible to get some results without the SB reproducers. My fear is that this will produce a solution which is very specialized to this particular use case, and which will have to be redone once you want more functionality out of it. Maybe that doesn't matter? I'm not sure..

> 
> 
>> In this (imaginary) world, the recording/replay would only deal with the things that happen below the SB API. Anything that happens above it could only be glimpsed through it's effects on the SB. So, when initializing replay, as soon the driver notices the `--replay` argument it would just load that file, call `RunReplay` and be done. There wouldn't be a need for any `command-line-argument` recorder, or making sure the command line arguments match in some other way.
>> 
>> By "capturing stdin" I meant capturing anything which the `RunCommandInterpreter` considers as it's "stdin", i.e., everything where the debugger gets its commands from.
> 
> This is the tricky part though. Where and how are you going to do this? In my
>  opinion this shouldn't be the responsibility of the SB recorder. The latter
>  should be concerned with "serializing" and "deserializing" its input and
>  tracking what SB layer calls are being made. It should be relatively agnostic
>  of what the debugger does with that at a lower level.
> 
> The `FILE*` is particularly annoying. For the sake of argument let's say that
>  "serializing" a `FILE*` means writing its content to the reproducer, how would
>  we accomplish this? Maybe there's an API I don't know about, but wouldn't we
>  have to do this at the `fgets` level? For the command interpreter this is
>  either in libedit or in the IO handler (depending on the fidelity you touched
>  on below).

Yes, the FILE* is quite tricky, and it might be the trickiest part of capturing the SB interface. I would actually argue that capturing this *is* in the scope of the SB recorder, which I would define as capturing the data that flows across the SB boundary. For a FILE*, that obviously doesn't mean the pointer value, because that's going to be meaningless in subsequent runs. It has do do a "deep" capture of the FILE* so that it can later be recreated with reasonable fidelity.

In an ideal world, it would just take a snapshot of the file, and then move on, but that is of course not possible, because the FILE* may be a stream, and it would have to look into the future to capture it instantaneously. So, it will have to capture it incrementally, as it is being read. Which will require some cooperation from the lower layers of the debugger.

I'd imagine this could be done by adding a level of indirection, similar to how you've made all filesystem accesses go through the FileSystem class. Instead of FILE*. everything would deal with some proxy class, which would record it's interaction into the log in record mode and replay it from saved data in replay mode. Most of the code would still be oblivious to the fact that anything funny is going on, but it would have to be changed to deal with the proxy class instead of the raw APIs.

>> Of course, capturing *real* stdin will come with additional challenges, both for recording and testing. For recording, we'll have to decide whether to try to capture raw stdin with enough fidelity so libedit can be driven through it, or we go one level higher, and try to capture the "cooked" command stream coming out of libedit. The testing strategy will depend on decision made here. In case we capture cooked commands, the situation should be very similar to command line options, so it might be enough to test by just passing the commands to the driver via stdin (`%lldb <%t/my_commands.txt`)  instead of via command line args. In case of low-level recordings, we may need to test using something which can provide a realistic-looking fake terminal such as pexpect (yikes). But even in that case, that should be only necessary to test the handling of funny line-editing operations (such as backspaces, cursor keys, etc.), and for general functionality redirecting stdin from file should be enough.
> 
> I don't think we really differentiate between *real* stdin and a file. Even
>  when sourcing a file go through the IOHandler, so that should be pretty
>  transparent, unless you actually want to differentiate which, if I understand
>  correctly, wouldn't be needed with your proposed approach?

I am afraid you lost me there. I am not sure what you wanted to say, but yes, I am saying that (for the most part) we don't have to differentiate between a file and real (tty) stdin.

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

https://reviews.llvm.org/D55582