labath added inline comments.
================
Comment at: lldb/source/Host/linux/Host.cpp:178-185
LLDB_LOG(log, "failed to read link exe link for {0}: {1}", pid,
Status(errno, eErrorTypePOSIX));
- return false;
+ ExePath.resize(0);
+#if defined(__ANDROID__)
+ // On android we fallback to Arg0, which commonly is an apk package name.
+ if (!process_info.GetArg0().empty()) {
+ ExePath = process_info.GetArg0();
----------------
wallace wrote:
> labath wrote:
> > Why does this fail on android exactly? Is it some seccomp thingy? I'm
> > surprised that we're allowed to read `/proc/cmdline`, but not the
> > `/proc/exe` link..
> >
> > Anyway, I don't think that pretending that "com.my.app" is an executable
> > file name is a good idea. Maybe it would be better to implement something
> > on the other end. I.e., we print out argv[0] if the file name is not
> > present?
> From what I've been seeing, /proc/cmdline is always available, even for root
> processes. On the other hand, /proc/exe is only available for the user that
> is seeing it. In fact, if you use run-as, you can see the contents of
> /proc/exe, but that would involve making too many slow calls (run-as is slow).
>
> I do think that having "com.my.app" as executable name is okay, because
> android doesn't let you interact with an apk by any other means, unless you
> root your device. When using android commands, "com.my.app" is just like any
> other process.
>
> I don't have any strong opinions though, so I'm okay with following your
> suggestion if you prefer so.
>
> Handling the android case from the host by doing exePath = argv[0] would
> look like this:
>
> - the Arg0 fallback is done inside the client when the exe path is empty
> - a special additional handling would be required for cases when lldb is
> running on Android. With the approach of this diff, that case is covered.
> - the current function, in the case of android, returns a possible empty exe
> path. For anything not android, empty exe paths are not allowed
>
>
> what do you think?
Having "com.my.app" as an executable (more like, process) name is probably
fine. However, having that as an executable _path_ (of type FileSpec!) is
streching it too far, I believe. If this function was returning some
higher-level concept of a process, which had a "name" as a separate property,
then putting argv[0] there would be fine. However, for something of type
FileSpec, I think it's better to have nothing rather than something that is
definitely _not_ a file.
> the Arg0 fallback is done inside the client when the exe path is empty
agreed, and this doesn't need to be android-specific
> a special additional handling would be required for cases when lldb is
> running on Android. With the approach of this diff, that case is covered.
I am not sure what you mean by that. Can you elaborate?
> the current function, in the case of android, returns a possible empty exe
> path. For anything not android, empty exe paths are not allowed
yes, and I think we should, like for the case of environment, just permit a
ProcessInstanceInfo result with no executable path, regardless of the OS.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68289/new/
https://reviews.llvm.org/D68289
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
