labath added inline comments.
================ Comment at: lldb/include/lldb/Host/Host.h:231 + /// Check whether a process is translated (Rosetta). + /// \arg process_info The info structure for the process queried. ---------------- aprantl wrote: > davide wrote: > > aprantl wrote: > > > Is this supposed to be a generic function call that, e.g, should fire for > > > debugging a process in QEMU under Linux, or is it meant to specifically > > > check for Rosetta on macOS? The comment makes it sound like it's the > > > latter and the API name hints at the former. It would be nice to clarify > > > this in the comment. > > It is meant specifically for Rosetta, but this leaves in `Host`, so I > > decided for a generic name. > > Do you have a better suggestion for the name or the comment ? Happy to go > > with it > Not awesome, but `IsMacOSRosettaProcess` or `IsMacOSRosettaTranslated`? > > A more serious question: Is Host actually the right place for this function? > What happens when I remote-debug a Rosetta process from another Mac or a > Linux machine? Is there even a correct abstraction for this use-case? > Platform maybe? This code is only used when debugging on the host. When debugging remotely, you either connect to at already running instance of debugserver, or you ask the lldb-server-platform process to launch one for you. The platform process then should use this function to find the right debugserver to launch, but using a Host function is appropriate there, because the platform process is running remotely, and so it will the the "remote Host" which is answering the question. ================ Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3424 + if (Host::IsProcessTranslated(process_info)) { + FileSpec rosetta_debugserver("/Library/Apple/usr/libexec/oah/debugserver"); + debugserver_launch_info.SetExecutableFile(rosetta_debugserver, false); ---------------- davide wrote: > labath wrote: > > I don't think this is a particularly good abstraction, as the debugserver > > path is still left here, and the path is definitely os- and arch-specific. > > It would be better if the API returned the path to the > > debugserver-to-be-used instead. > > > > Bonus points if you can also hide the `DEBUGSERVER_BASENAME` logic from > > GDBRemoteCommunication.cpp into the same API. > hmm, I wonder if this code should live in `GDBRemoteCommunication.cpp` I think it ended up there because the gdb-server launching code is used from both liblldb and lldb-server (the platform version), and GDBRemoteCommunication is the greatest common denominator. It's not ideal, but I don't think its the worst thing that we have. (At one point I'd like to create a library to house code which is shared between liblldb and lldb-server, and then it may be easier to organize things better.) Speaking of that... I think this code should live in GDBRemoteCommunication as well. In the situation where one is remote-debugging a mac via lldb-server-platform, I assume one would want the platform process to automatically spawn the right debugserver for the rosetta thingies. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82813/new/ https://reviews.llvm.org/D82813 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits