lawrence_danna marked 4 inline comments as done. lawrence_danna added inline comments.
================ Comment at: lldb/source/Utility/ReproducerInstrumentation.cpp:38 +template <> lldb::SBFile Deserializer::Deserialize<lldb::SBFile>() { + //@JDevlieghere I'm pretty sure this is not the right thing to ---------------- JDevlieghere wrote: > labath wrote: > > lawrence_danna wrote: > > > @JDevlieghere advice please. > > This is not totally surprising as SBDebugger::SetInputFile ignores the > > input SBFile when it's in replay mode. However, it still doesn't seem like > > the right fix. I am guessing that something special needs to happen in the > > record/replay logic of the SBFile constructor, but off-hand, it's not fully > > clear to me what would that be. > > > > I think ideally, we'd have the reproducer shadowing that's currently done > > in SBDebugger::SetInputFileHandle kick in earlier (in the SBFile > > constructor) so that it covers all SBFiles, and not just those that later > > become the debugger input handles, but I don't think it should be on you to > > make all of that work. Maybe the appropriate fix would be to just make sure > > the SBFile constructor creates empty objects and acknowledge that > > reproducers won't work for all SBFiles until someone actually implements > > the appropriate support for them. > Thanks for the great explanation Pavel, I share your thoughts on this. So to > make this work you'll want to remove the `LLDB_REGISTER_CONSTRUCTOR` for > SBFile that take a file descriptor or a `FILE*` (which looks like it's > currently missing?) and have custom `doit` implementation (similar to what I > did in SBDebugger) that just returns and empty SBFile. custom doits for the constructor aren't enough. That won't stop the serializer from using reinterpret_cast on SBFiles that are passed by value to other recorder-instrumented functions. I've updated the patch with a somewhat more reasonable solution that doesn't mess up the library dependencies. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68434/new/ https://reviews.llvm.org/D68434 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits