JDevlieghere planned changes to this revision.
JDevlieghere added a comment.

In D89600#2337868 <https://reviews.llvm.org/D89600#2337868>, @labath wrote:

> Hm... Are you sure that this will help with anything? IIUC, the serialization 
> of the VFS file map still happens in the context of the crashing process. 
> This requires walking a bunch of data structures with liberally-asserting 
> code, which will abort on any inconsistency it runs into. If *that* succeeds, 
> I'd expect that the copying of the files will succeed too.

I only have a handful of crash logs to base myself off. Currently the walking 
and the copying are intertwined, so I can't say for sure. My idea was to give 
this a try and see where that got us and make more invasive changes (like what 
you proposed) if needed. But since you sound pretty convinced that we'll need 
it anyway I will extend the patch.

> I think it would be better if we moved the vfs map construction into the 
> other process too. Maybe via something like this:
>
> - The first process opens a "log" file, which will contain the list of files 
> to be copied. For added safety the file should be opened append-only.
> - As it works, it writes file names to this file.
> - Upon a crash, the re-execed process reads this file, constructs the vfs 
> map, and copies all necessary files. It should be prepared to handle/ignore 
> any garbled entries, and the format of the file should make this easy. (Since 
> the log file must by definition contain enough information to recreate the 
> vfs map, it might be simpler to not write out the map, but just always 
> recreate it in memory when replaying.)

Yes, that would be similar to how we immediately flush GDB remote packets to 
disk. It would require adding some "immediate" mode to the FileCollector and an 
API to convert between the formats. I think the new format could be just host 
paths separated by newlines.



================
Comment at: lldb/tools/driver/Options.td:235
   HelpText<"Tells the debugger to replay a reproducer from <filename>.">;
+def reproducer_finalize: Separate<["--", "-"], "reproducer-finalize">,
+  MetaVarName<"<filename>">,
----------------
labath wrote:
> Should this be a hidden argument?
Yep


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

https://reviews.llvm.org/D89600

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

Reply via email to