wallace updated this revision to Diff 226744.
wallace added a comment.
now reading from comm
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68968/new/
https://reviews.llvm.org/D68968
Files:
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Utility/ProcessInfo.h
lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
lldb/source/Host/linux/Host.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
lldb/source/Utility/ProcessInfo.cpp
Index: lldb/source/Utility/ProcessInfo.cpp
===================================================================
--- lldb/source/Utility/ProcessInfo.cpp
+++ lldb/source/Utility/ProcessInfo.cpp
@@ -38,11 +38,18 @@
m_pid = LLDB_INVALID_PROCESS_ID;
}
+// On systems like android, there are cases in which a process name can
+// correspond to a bundle id (e.g. com.test.app), which is not an executable
+// path.
const char *ProcessInfo::GetName() const {
+ if (!m_display_name.empty())
+ return m_display_name.c_str();
return m_executable.GetFilename().GetCString();
}
llvm::StringRef ProcessInfo::GetNameAsStringRef() const {
+ if (!m_display_name.empty())
+ return m_display_name;
return m_executable.GetFilename().GetStringRef();
}
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
@@ -1187,14 +1187,16 @@
response.PutCString("name:");
response.PutStringAsRawHex8(proc_info.GetExecutableFile().GetCString());
- response.PutChar(';');
- response.PutCString("args:");
+ response.PutCString(";args:");
response.PutStringAsRawHex8(proc_info.GetArg0());
for (auto &arg : proc_info.GetArguments()) {
response.PutChar('-');
response.PutStringAsRawHex8(arg.ref());
}
+ response.PutCString(";display_name:");
+ response.PutStringAsRawHex8(proc_info.GetDisplayName());
+
response.PutChar(';');
const ArchSpec &proc_arch = proc_info.GetArchitecture();
if (proc_arch.IsValid()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1948,6 +1948,11 @@
process_info.GetArguments().AppendArgument(arg);
is_arg0 = false;
}
+ } else if (name.equals("display_name")) {
+ StringExtractor extractor(value);
+ std::string display_name;
+ extractor.GetHexByteString(display_name);
+ process_info.SetDisplayName(display_name);
} else if (name.equals("cputype")) {
value.getAsInteger(0, cpu);
} else if (name.equals("cpusubtype")) {
Index: lldb/source/Host/linux/Host.cpp
===================================================================
--- lldb/source/Host/linux/Host.cpp
+++ lldb/source/Host/linux/Host.cpp
@@ -202,6 +202,40 @@
}
}
+static bool GetProcessNameFromComm(::pid_t pid, std::string& name) {
+ auto BufferOrError = getProcFile(pid, "comm");
+ if (!BufferOrError)
+ return false;
+
+ std::unique_ptr<llvm::MemoryBuffer> Stat = std::move(*BufferOrError);
+ llvm::StringRef Content = Stat->getBuffer();
+ if (Content.empty())
+ return false;
+
+ // comm might end with new line
+ name = Content.trim();
+ return true;
+}
+
+static void SetProcessDisplayName(::pid_t pid, ProcessInstanceInfo& process_info) {
+ if (!process_info.GetNameAsStringRef().empty())
+ return;
+ if (!process_info.GetArg0().empty()) {
+ // When /proc/<pid>/exe is not readable, then we use arg0 as display name.
+ // On Android devices this might contain the apk package name.
+ process_info.SetDisplayName(process_info.GetArg0());
+ return;
+ }
+ // If /proc/<pid>/cmdline is not readable, then it might correspond to a kernel thread
+ // or system process. We fallback to reading from /proc<pid>/stat for a display name.
+ std::string name;
+ if (GetProcessNameFromComm(pid, name)) {
+ // We follow the guideline of surrounding the name with brackets, as explained here
+ // https://unix.stackexchange.com/questions/22121/what-do-the-brackets-around-processes-mean
+ process_info.SetDisplayName("[" + name + "]");
+ }
+}
+
static bool GetProcessAndStatInfo(::pid_t pid,
ProcessInstanceInfo &process_info,
ProcessState &State, ::pid_t &tracerpid) {
@@ -213,6 +247,8 @@
GetExePathAndArch(pid, process_info);
GetProcessArgs(pid, process_info);
GetProcessEnviron(pid, process_info);
+ // Get fallback display name
+ SetProcessDisplayName(pid, process_info);
// Get User and Group IDs and get tracer pid.
if (!GetStatusInfo(pid, process_info, State, tracerpid))
Index: lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
+++ lldb/packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestPlatformClient.py
@@ -5,9 +5,11 @@
from lldbsuite.test.decorators import *
from gdbclientutils import *
+
def hexlify(string):
return binascii.hexlify(string.encode()).decode()
+
class TestPlatformClient(GDBRemoteTestBase):
def test_process_list_with_all_users(self):
@@ -16,31 +18,42 @@
class MyResponder(MockGDBServerResponder):
def __init__(self):
MockGDBServerResponder.__init__(self)
- self.currentQsProc = 0
+ self.currentProc = 0
self.all_users = False
+ def _processInfo0(self):
+ name = hexlify("/a/test_process")
+ args = "-".join(map(hexlify,
+ ["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
+ return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
+
+ def _processInfo1(self):
+ name = hexlify("/b/another_test_process")
+ # This intentionally has a badly encoded argument
+ args = "X".join(map(hexlify,
+ ["/system/bin/ls", "--help"]))
+ return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
+
+ def _processInfo2(self):
+ # a process without args but with a display_name, which can happen on android
+ display_name = hexlify("com.test.app:Core")
+ return "pid:12;ppid:3;uid:4;gid:5;euid:6;egid:7;name:;args:;display_name:" + display_name + ";"
+
def qfProcessInfo(self, packet):
- if "all_users:1" in packet:
- self.all_users = True
- name = hexlify("/a/test_process")
- args = "-".join(map(hexlify,
- ["/system/bin/sh", "-c", "/data/local/tmp/lldb-server"]))
- return "pid:10;ppid:1;uid:2;gid:3;euid:4;egid:5;name:" + name + ";args:" + args + ";"
- else:
- self.all_users = False
- return "E04"
+ self.all_users = "all_users:1" in packet
+ self.currentProc = 1
+ return self._processInfo0()
def qsProcessInfo(self):
if self.all_users:
- if self.currentQsProc == 0:
- self.currentQsProc = 1
- name = hexlify("/b/another_test_process")
- # This intentionally has a badly encoded argument
- args = "X".join(map(hexlify,
- ["/system/bin/ls", "--help"]))
- return "pid:11;ppid:2;uid:3;gid:4;euid:5;egid:6;name:" + name + ";args:" + args + ";"
- elif self.currentQsProc == 1:
- self.currentQsProc = 0
+ if self.currentProc == 1:
+ self.currentProc = 2
+ return self._processInfo1()
+ elif self.currentProc == 2:
+ self.currentProc = 3
+ return self._processInfo2()
+ else:
+ self.currentProc = 0
return "E04"
else:
return "E04"
@@ -53,15 +66,15 @@
self.server.port)
self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
self.expect("platform process list -x",
- substrs=["2 matching processes were found", "test_process", "another_test_process"])
+ substrs=["3 matching processes were found", "test_process", "another_test_process", "com.test.app:Core"])
self.expect("platform process list -xv",
substrs=[
"PID PARENT USER GROUP EFF USER EFF GROUP TRIPLE ARGUMENTS",
"10 1 2 3 4 5 /system/bin/sh -c /data/local/tmp/lldb-server",
"11 2 3 4 5 6"])
- self.expect("platform process list -xv", substrs=["/system/bin/ls"], matching=False)
+ self.expect("platform process list -xv",
+ substrs=["/system/bin/ls", "com.test.app:Core"], matching=False)
self.expect("platform process list",
- error=True,
- substrs=["error: no processes were found on the \"remote-linux\" platform"])
+ substrs=["1 matching process was found"])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/include/lldb/Utility/ProcessInfo.h
===================================================================
--- lldb/include/lldb/Utility/ProcessInfo.h
+++ lldb/include/lldb/Utility/ProcessInfo.h
@@ -73,6 +73,10 @@
void Dump(Stream &s, Platform *platform) const;
+ void SetDisplayName(llvm::StringRef display_name) { m_display_name = display_name; }
+
+ llvm::StringRef GetDisplayName() const { return m_display_name; }
+
Args &GetArguments() { return m_arguments; }
const Args &GetArguments() const { return m_arguments; }
@@ -90,6 +94,10 @@
protected:
FileSpec m_executable;
+ std::string m_display_name; // There are special programs like Android apks or
+ // kernel threads, whose executable path might not reflect accurately the name
+ // they should expose to the user and it might even be impossible to get the
+ // executable path. This field can be useful for these cases.
std::string m_arg0; // argv[0] if supported. If empty, then use m_executable.
// Not all process plug-ins support specifying an argv[0] that differs from
// the resolved platform executable (which is in m_executable)
Index: lldb/docs/lldb-gdb-remote.txt
===================================================================
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -1433,27 +1433,33 @@
// Required. The qfProcessInfo packet can be followed by a ':' and
// some key value pairs. The key value pairs in the command are:
//
-// KEY VALUE DESCRIPTION
-// =========== ======== ================================================
-// "name" ascii-hex An ASCII hex string that contains the name of
-// the process that will be matched.
-// "name_match" enum One of: "equals", "starts_with", "ends_with",
-// "contains" or "regex"
-// "pid" integer A string value containing the decimal process ID
-// "parent_pid" integer A string value containing the decimal parent
-// process ID
-// "uid" integer A string value containing the decimal user ID
-// "gid" integer A string value containing the decimal group ID
-// "euid" integer A string value containing the decimal effective user ID
-// "egid" integer A string value containing the decimal effective group ID
-// "all_users" bool A boolean value that specifies if processes should
-// be listed for all users, not just the user that the
-// platform is running as
-// "triple" string An ASCII triple string ("x86_64",
-// "x86_64-apple-macosx", "armv7-apple-ios")
-// "args" string A string value containing the process arguments
-// separated by the character '-', where each argument is
-// hex-encoded. It includes argv[0].
+// KEY VALUE DESCRIPTION
+// ============== ========= ================================================
+// "name" ascii-hex An ASCII hex string that contains the name of
+// the process that will be matched.
+// "name_match" enum One of: "equals", "starts_with", "ends_with",
+// "contains" or "regex"
+// "pid" integer A string value containing the decimal process ID
+// "parent_pid" integer A string value containing the decimal parent
+// process ID
+// "uid" integer A string value containing the decimal user ID
+// "gid" integer A string value containing the decimal group ID
+// "euid" integer A string value containing the decimal effective
+// user ID
+// "egid" integer A string value containing the decimal effective
+// group ID
+// "all_users" bool A boolean value that specifies if processes
+// should be listed for all users, not just the user
+// that the platform is running as
+// "triple" string An ASCII triple string ("x86_64",
+// "x86_64-apple-macosx", "armv7-apple-ios")
+// "args" string A string value containing the process arguments
+// separated by the character '-', where each
+// argument is hex-encoded. It includes argv[0].
+// "display_name" string An ASCII optional string for using when
+// displaying information about the process. In
+// contrast to the "name" field, this value doesn't
+// need to correspond to actual system information.
//
// The response consists of key/value pairs where the key is separated from the
// values with colons and each pair is terminated with a semi colon. For a list
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
