[Lldb-commits] [PATCH] D82813: [Apple Silicon] Rewrite part of the Rosetta support to be confined in Apple specific files

Wed Jul 1 09:42:43 PDT 2020

aprantl 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.
----------------
labath wrote:
> 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.
> 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.

Thanks, that makes perfect sense!

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

https://reviews.llvm.org/D82813