[lldb][Android] Fix platform process list regression #164333
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ses implementation
|
@llvm/pr-subscribers-lldb Author: Chad Smith (cs01) ChangesSummaryAfter #160931, Bug DescriptionAfter the implementation of FindProcesses in PlatformAndroid (#160931), Additionally, results were inconsistent between different query modes:
Root CauseThe original implementation used a simple
Why Android is Different from Regular LinuxOn regular Linux, process enumeration reads Android breaks this assumption due to the Zygote process model:
SolutionUse Remote Android:
These all work now
Test ResultsBefore this fix:After this fix:Test PlanWith an Android device/emulator connected:
adb push lldb-server /data/local/tmp/
adb shell chmod +x /data/local/tmp/lldb-server
adb shell /data/local/tmp/lldb-server platform --listen 127.0.0.1:9500 --server
ImpactRestores Fixes #164192 Full diff: https://github.com/llvm/llvm-project/pull/164333.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 57d88f615e2b3..4b93cfa9b3797 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -10,10 +10,13 @@
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Section.h"
#include "lldb/Host/HostInfo.h"
+#include "lldb/Utility/DataExtractor.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/UriParser.h"
#include "lldb/ValueObject/ValueObject.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/Support/Base64.h"
#include "AdbClient.h"
#include "PlatformAndroid.h"
@@ -571,40 +574,98 @@ void PlatformAndroid::PopulateProcessCommandLine(
process_info.SetArguments(process_args, false);
}
-// Helper function to populate architecture from /proc/[pid]/exe
+// Helper function to populate architecture from /proc/[pid]/exe by reading ELF
+// header
void PlatformAndroid::PopulateProcessArchitecture(
lldb::pid_t pid, ProcessInstanceInfo &process_info) {
- // Read /proc/[pid]/exe to get executable path for architecture detection
+ // Read the first 20 bytes of /proc/[pid]/exe to parse the ELF header
+ // We need the ELF identification (16 bytes) plus e_machine field (2 bytes at
+ // offset 18)
Status error;
- AdbClientUP exe_adb = GetAdbClient(error);
+ AdbClientUP elf_adb = GetAdbClient(error);
if (error.Fail())
return;
- std::string exe_output;
- StreamString exe_cmd;
- exe_cmd.Printf("readlink /proc/%llu/exe 2>/dev/null",
+ std::string elf_header_base64;
+ StreamString elf_cmd;
+ // Use dd to read just the ELF header (first 20 bytes is enough for e_machine)
+ // Output as base64 to avoid parsing issues with binary data
+ elf_cmd.Printf("dd if=/proc/%llu/exe bs=20 count=1 2>/dev/null | base64",
static_cast<unsigned long long>(pid));
- Status exe_error = exe_adb->Shell(exe_cmd.GetData(), seconds(5), &exe_output);
+ Status elf_error =
+ elf_adb->Shell(elf_cmd.GetData(), seconds(5), &elf_header_base64);
- if (exe_error.Fail() || exe_output.empty())
+ if (elf_error.Fail() || elf_header_base64.empty())
return;
- exe_output = llvm::StringRef(exe_output).trim().str();
+ // Decode base64 using LLVM's base64 decoder
+ std::vector<char> header_bytes;
+ llvm::Error decode_error = llvm::decodeBase64(
+ llvm::StringRef(elf_header_base64).trim(), header_bytes);
+
+ Log *log = GetLog(LLDBLog::Platform);
+ if (decode_error) {
+ LLDB_LOGF(log, "PlatformAndroid::%s base64 decode failed for PID %llu: %s",
+ __FUNCTION__, static_cast<unsigned long long>(pid),
+ llvm::toString(std::move(decode_error)).c_str());
+ return;
+ }
+
+ LLDB_LOGF(log, "PlatformAndroid::%s decoded %zu bytes for PID %llu",
+ __FUNCTION__, header_bytes.size(),
+ static_cast<unsigned long long>(pid));
+
+ // Need at least 20 bytes for ELF header check
+ if (header_bytes.size() < 20) {
+ LLDB_LOGF(log, "PlatformAndroid::%s insufficient bytes (%zu < 20)",
+ __FUNCTION__, header_bytes.size());
+ return;
+ }
+
+ // Use DataExtractor to parse ELF header
+ DataExtractor extractor(header_bytes.data(), header_bytes.size(),
+ eByteOrderLittle, 4);
+ lldb::offset_t offset = 0;
+
+ // Verify ELF magic number (0x7f 'E' 'L' 'F')
+ if (extractor.GetU8(&offset) != 0x7f || extractor.GetU8(&offset) != 'E' ||
+ extractor.GetU8(&offset) != 'L' || extractor.GetU8(&offset) != 'F') {
+ LLDB_LOGF(log,
+ "PlatformAndroid::%s invalid ELF magic at offset 0 for PID %llu",
+ __FUNCTION__, static_cast<unsigned long long>(pid));
+ return;
+ }
+
+ // Get ELF class (32-bit vs 64-bit) from e_ident[EI_CLASS]
+ bool is_64bit = (extractor.GetU8(&offset) == llvm::ELF::ELFCLASS64);
+
+ // e_machine is at offset 18 in both 32-bit and 64-bit ELF headers
+ offset = offsetof(llvm::ELF::Elf32_Ehdr, e_machine);
+ uint16_t e_machine = extractor.GetU16(&offset);
+
+ LLDB_LOGF(log, "PlatformAndroid::%s ELF class=%s e_machine=0x%x for PID %llu",
+ __FUNCTION__, is_64bit ? "64-bit" : "32-bit", e_machine,
+ static_cast<unsigned long long>(pid));
- // Determine architecture from exe path
ArchSpec arch;
- if (exe_output.find("64") != std::string::npos ||
- exe_output.find("arm64") != std::string::npos ||
- exe_output.find("aarch64") != std::string::npos) {
+ switch (e_machine) {
+ case llvm::ELF::EM_ARM:
+ arch.SetTriple("armv7-unknown-linux-android");
+ break;
+ case llvm::ELF::EM_AARCH64:
arch.SetTriple("aarch64-unknown-linux-android");
- } else if (exe_output.find("x86_64") != std::string::npos) {
- arch.SetTriple("x86_64-unknown-linux-android");
- } else if (exe_output.find("x86") != std::string::npos ||
- exe_output.find("i686") != std::string::npos) {
+ break;
+ case llvm::ELF::EM_386:
arch.SetTriple("i686-unknown-linux-android");
- } else {
- // Default to armv7 for 32-bit ARM (most common on Android)
- arch.SetTriple("armv7-unknown-linux-android");
+ break;
+ case llvm::ELF::EM_X86_64:
+ arch.SetTriple("x86_64-unknown-linux-android");
+ break;
+ default:
+ LLDB_LOGF(log, "PlatformAndroid::%s unknown e_machine=0x%x", __FUNCTION__,
+ e_machine);
+ arch.SetTriple("unknown-unknown-linux-android");
+ break;
}
if (arch.IsValid())
@@ -623,26 +684,22 @@ PlatformAndroid::FindProcesses(const ProcessInstanceInfoMatch &match_info,
if (IsHost())
return PlatformLinux::FindProcesses(match_info, proc_infos);
- // Remote Android platform: implement process name lookup using 'pidof' over
- // adb.
+ // Remote Android: Always use 'ps' to get process list with cmdline
+ // information (which contains Android package names). This ensures consistent
+ // naming for all queries (by name, PID, arch, user, etc.) - Android apps
+ // will show their package names instead of "app_process64".
- // LLDB stores the search name in GetExecutableFile() (even though it's
- // actually a process name like "com.android.chrome" rather than an
- // executable path). If no search name is provided, we can't use
- // 'pidof', so return early with no results.
const ProcessInstanceInfo &match_process_info = match_info.GetProcessInfo();
- if (!match_process_info.GetExecutableFile() ||
- match_info.GetNameMatchType() == NameMatch::Ignore) {
- return 0;
- }
+ std::string process_name;
+ NameMatch name_match_type = NameMatch::Ignore;
- // Extract the process name to search for (typically an Android package name
- // like "com.example.app" or binary name like "app_process64")
- std::string process_name = match_process_info.GetExecutableFile().GetPath();
- if (process_name.empty())
- return 0;
+ // If there's a name to match, extract it
+ if (match_process_info.GetExecutableFile()) {
+ process_name = match_process_info.GetExecutableFile().GetPath();
+ name_match_type = match_info.GetNameMatchType();
+ }
- // Use adb to find the process by name
+ // Use adb to get process list
Status error;
AdbClientUP adb(GetAdbClient(error));
if (error.Fail()) {
@@ -652,44 +709,39 @@ PlatformAndroid::FindProcesses(const ProcessInstanceInfoMatch &match_info,
return 0;
}
- // Use 'pidof' command to get PIDs for the process name.
- // Quote the process name to handle special characters (spaces, etc.)
- std::string pidof_output;
- StreamString command;
- command.Printf("pidof '%s'", process_name.c_str());
- error = adb->Shell(command.GetData(), seconds(5), &pidof_output);
+ std::string ps_output;
+ error = adb->Shell("ps -A -o PID,ARGS", seconds(5), &ps_output);
if (error.Fail()) {
Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOG(log, "PlatformAndroid::{} 'pidof {}' failed: {}", __FUNCTION__,
- process_name.c_str(), error.AsCString());
- return 0;
- }
-
- // Parse PIDs from pidof output.
- // Note: pidof can return multiple PIDs (space-separated) if multiple
- // instances of the same executable are running.
- pidof_output = llvm::StringRef(pidof_output).trim().str();
- if (pidof_output.empty()) {
- Log *log = GetLog(LLDBLog::Platform);
- LLDB_LOGF(log, "PlatformAndroid::%s no process found with name '%s'",
- __FUNCTION__, process_name.c_str());
+ LLDB_LOG(log, "PlatformAndroid::{} 'ps -A' failed: {}", __FUNCTION__,
+ error.AsCString());
return 0;
}
- // Split the output by whitespace to handle multiple PIDs
- llvm::SmallVector<llvm::StringRef, 8> pid_strings;
- llvm::StringRef(pidof_output).split(pid_strings, ' ', -1, false);
-
Log *log = GetLog(LLDBLog::Platform);
- // Process each PID and gather information
+ llvm::SmallVector<llvm::StringRef, 256> lines;
+ llvm::StringRef(ps_output).split(lines, '\n', -1, false);
+
uint32_t num_matches = 0;
- for (llvm::StringRef pid_str : pid_strings) {
- pid_str = pid_str.trim();
- if (pid_str.empty())
+
+ for (llvm::StringRef line : lines) {
+ line = line.trim();
+ if (line.empty())
continue;
+ if (line.starts_with("PID"))
+ continue;
+
+ // Parse PID (first whitespace-separated field)
+ auto space_pos = line.find(' ');
+ if (space_pos == llvm::StringRef::npos)
+ continue;
+
+ llvm::StringRef pid_str = line.substr(0, space_pos);
+ llvm::StringRef cmdline = line.substr(space_pos + 1).trim();
+
lldb::pid_t pid;
if (!llvm::to_integer(pid_str, pid)) {
LLDB_LOGF(log, "PlatformAndroid::%s failed to parse PID from: '%s'",
@@ -697,23 +749,57 @@ PlatformAndroid::FindProcesses(const ProcessInstanceInfoMatch &match_info,
continue;
}
+ bool name_matches = true;
+ if (name_match_type != NameMatch::Ignore && !process_name.empty()) {
+ name_matches = false;
+ switch (name_match_type) {
+ case NameMatch::Equals:
+ name_matches = (cmdline == process_name);
+ break;
+ case NameMatch::StartsWith:
+ name_matches = cmdline.starts_with(process_name);
+ break;
+ case NameMatch::EndsWith:
+ name_matches = cmdline.ends_with(process_name);
+ break;
+ case NameMatch::Contains:
+ name_matches = cmdline.contains(process_name);
+ break;
+ case NameMatch::RegularExpression: {
+ llvm::Regex regex(process_name);
+ name_matches = regex.match(cmdline);
+ break;
+ }
+ default:
+ name_matches = true;
+ break;
+ }
+ }
+
+ if (!name_matches)
+ continue;
+
ProcessInstanceInfo process_info;
process_info.SetProcessID(pid);
- process_info.GetExecutableFile().SetFile(process_name,
- FileSpec::Style::posix);
- // Populate additional process information
+ // Set the executable name from cmdline (first token, typically)
+ llvm::StringRef exe_name = cmdline;
+ auto first_space = cmdline.find(' ');
+ if (first_space != llvm::StringRef::npos)
+ exe_name = cmdline.substr(0, first_space);
+
+ process_info.GetExecutableFile().SetFile(exe_name, FileSpec::Style::posix);
+
PopulateProcessStatusInfo(pid, process_info);
PopulateProcessCommandLine(pid, process_info);
PopulateProcessArchitecture(pid, process_info);
- // Check if this process matches the criteria
if (match_info.Matches(process_info)) {
proc_infos.push_back(process_info);
num_matches++;
LLDB_LOGF(log, "PlatformAndroid::%s found process '%s' with PID %llu",
- __FUNCTION__, process_name.c_str(),
+ __FUNCTION__, exe_name.str().c_str(),
static_cast<unsigned long long>(pid));
}
}
@@ -721,6 +807,71 @@ PlatformAndroid::FindProcesses(const ProcessInstanceInfoMatch &match_info,
return num_matches;
}
+bool PlatformAndroid::GetProcessInfo(lldb::pid_t pid,
+ ProcessInstanceInfo &proc_info) {
+ // On native Android or when remote, use ps to get process info with
+ // cmdline (which contains Android package names).
+ if (IsHost() || pid == LLDB_INVALID_PROCESS_ID)
+ return PlatformLinux::GetProcessInfo(pid, proc_info);
+
+ // Use ps to get process info for this specific PID
+ Status error;
+ AdbClientUP adb(GetAdbClient(error));
+ if (error.Fail())
+ return false;
+
+ StreamString cmd;
+ cmd.Printf("ps -A -o PID,ARGS -p %llu", static_cast<unsigned long long>(pid));
+
+ std::string ps_output;
+ error = adb->Shell(cmd.GetData(), seconds(5), &ps_output);
+ if (error.Fail())
+ return false;
+
+ // Parse ps output - should be 2 lines: header + process
+ llvm::SmallVector<llvm::StringRef, 2> lines;
+ llvm::StringRef(ps_output).split(lines, '\n', -1, false);
+
+ for (llvm::StringRef line : lines) {
+ line = line.trim();
+ if (line.empty() || line.starts_with("PID"))
+ continue;
+
+ // Parse PID and cmdline
+ auto space_pos = line.find(' ');
+ if (space_pos == llvm::StringRef::npos)
+ continue;
+
+ llvm::StringRef pid_str = line.substr(0, space_pos);
+ llvm::StringRef cmdline = line.substr(space_pos + 1).trim();
+
+ lldb::pid_t parsed_pid;
+ if (!llvm::to_integer(pid_str, parsed_pid) || parsed_pid != pid)
+ continue;
+
+ // Found our process, populate info
+ proc_info.Clear();
+ proc_info.SetProcessID(pid);
+
+ // Set executable name from cmdline (first token)
+ llvm::StringRef exe_name = cmdline;
+ auto first_space = cmdline.find(' ');
+ if (first_space != llvm::StringRef::npos)
+ exe_name = cmdline.substr(0, first_space);
+
+ proc_info.GetExecutableFile().SetFile(exe_name, FileSpec::Style::posix);
+
+ // Populate additional info
+ PopulateProcessStatusInfo(pid, proc_info);
+ PopulateProcessCommandLine(pid, proc_info);
+ PopulateProcessArchitecture(pid, proc_info);
+
+ return true;
+ }
+
+ return false;
+}
+
std::unique_ptr<AdbSyncService> PlatformAndroid::GetSyncService(Status &error) {
auto sync_service = std::make_unique<AdbSyncService>(m_device_id);
error = sync_service->SetupSyncConnection();
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
index e771c6ae97d4d..fe28913b417b7 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.h
@@ -60,7 +60,9 @@ class PlatformAndroid : public platform_linux::PlatformLinux {
uint32_t GetDefaultMemoryCacheLineSize() override;
uint32_t FindProcesses(const ProcessInstanceInfoMatch &match_info,
- ProcessInstanceInfoList &proc_infos) override;
+ ProcessInstanceInfoList &process_infos) override;
+
+ bool GetProcessInfo(lldb::pid_t pid, ProcessInstanceInfo &proc_info) override;
protected:
const char *GetCacheHostname() override;
|
| exe_name = cmdline.substr(0, first_space); | ||
|
|
||
| process_info.GetExecutableFile().SetFile(exe_name, FileSpec::Style::posix); | ||
|
|
There was a problem hiding this comment.
This runs multiple adb commands for each pid on the Android device, and an unfiltered query returns around 1000 pids. Empirically, this function (called via Python API or platform process list) takes over 1 minute to finish even on a local device with USB-3.2 connection.
Maybe we should batch the call for all processes into one, and then parse the response?
There was a problem hiding this comment.
I noticed it was slow too. However it did not seem slower than before. But I like the idea of batching the calls. I can do that tomorrow.
| command.Printf("pidof '%s'", process_name.c_str()); | ||
| error = adb->Shell(command.GetData(), seconds(5), &pidof_output); | ||
| std::string ps_output; | ||
| error = adb->Shell("ps -A -o PID,ARGS", seconds(5), &ps_output); |
There was a problem hiding this comment.
I don't see the point of querying all processes, when the Android security model only allows attaching to the processes running as the same user as lldb-server. Shouldn't we list only those processes that we can debug?
There was a problem hiding this comment.
I believe the full list of potentially non-debuggable processes is shown on non-Android platforms too. I can check tomorrow. I would prefer to follow whatever convention lldb has for this in general.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
After #160931,
platform process listwithout arguments returned no processes on remote Android, breaking the ability to enumerate processes. This was a regression.Background
After the implementation of FindProcesses in PlatformAndroid (#160931),
platform process listwithout arguments fails to return any processes:Additionally, results were inconsistent between different query modes:
platform process list -n com.example.hellojnireturned processes without triple infoplatform process list -p 5276returned correct triple infoplatform process list(all processes) returned nothingRoot Cause
The original implementation used a simple
MatchAllProcesses()check to decide when to fall back to GDB Remote Protocol. This logic was too narrow and missed important cases like:-s,-e,-c,-rflags)Why Android is Different from Regular Linux
On regular Linux, process enumeration reads
/proc/[pid]/exe, which is a symlink to the actual binary (e.g.,/usr/bin/firefox,/bin/bash). This gives you the canonical process name directly. The/proc/[pid]/cmdlinecontains arguments which can be misleading (e.g.,python script.py- do you want "python" or "script.py"?), so Linux platforms use the exe symlink as the reliable source.Android breaks this assumption due to the Zygote process model:
/proc/[pid]/exepoints toapp_process64for all Android apps (not the actual app)com.example.hellojni) only appears in/proc/[pid]/cmdlineapp_process64), not the package name/proc/[pid]/cmdlineSolution
Use
ps -A -o PID,ARGSto get process list with cmdline information for all remote Android queries (bothFindProcessesandGetProcessInfo.GetProcessInfowas added in this PR).Remote Android:
ps -Ato list all processes with their command lines/proc/[pid]/exefor architectureThese all work now
platform process list -n com.example.appplatform process list -s com.exampleplatform process list -c exampleplatform process list -r "com\..*\.app"process attach -n com.example.appTest Results
Before this fix:
After this fix:
Test Plan
With an Android device/emulator connected:
platform process listreturns all processes with triple informationplatform process list -n com.example.appfinds Android apps by package nameprocess attach -n com.example.appsuccessfully attaches to Android appsImpact
Restores
platform process liston Android with architecture information and package name lookup. All name matching modes now work correctly.Fixes #164192