mgorny added a comment. Thanks for the review. I'm going to update the tests as you suggested before pushing.
================ Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:287 + return Extension::multiprocess | Extension::fork | Extension::vfork | + Extension::pass_signals | Extension::auxv | Extension::libraries_svr4; } ---------------- labath wrote: > mgorny wrote: > > labath wrote: > > > Technically, `libraries_svr4` functionality is implemented in the base > > > class, so it would be more correct to implement this as `| > > > NativeProcessELF::Factory::GetSupportedExtensions()` (this would be a new > > > class, which exists only to override `GetSupportedExtensions`). > > Doesn't it require some extra logic in the `NativeProcess*` class though, > > like memory maps or something like that? If not, I sure can move it, I > > guess. > Hmm.. that's a good question. Fortunately, we have a unit test for this > class, so it should be easy to answer it. It seems it does require reading an > auxiliary vector (and memory). However, I don't think that disqualifies it > from this (svr4 support is the only reason for this class existence -- if a > plugin did not support auxv reading, it would not inherit from it). OTOH, I > can sort of see how it makes sense to implement this here, so I'm not going > to push for that.. Now that I think of it, I'm pretty sure Kamil told me once that NetBSD used not to support SVR4 libs properly (dyld stuff, I guess). I suppose it makes more sense to enable it per platform. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101241/new/ https://reviews.llvm.org/D101241 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
