labath 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
----------------
lawrence_danna wrote:
> 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.
> That won't stop the serializer from using reinterpret_cast on SBFiles that
> are passed by value to other recorder-instrumented functions.
I find that surprising. At this level, what makes SBFile different from any
other SB class that is passed around by value? I'd expect that the reproducer
should be able to copy the (empty) SBFile object just like every other class
(once we actually get it to construct an empty SBFile from a FileSP)
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
