Skip to content

Commit 87d5ae7

Browse files
authored
implement a subset of match filters for the qfProcessInfo and qsProcessInfo packets (#168)
## Purpose Properly implement a subset of match filters for the `qfProcessInfo` and `qsProcessInfo` packets. fixes #167 fixes #171 ## Overview * Fix the `Handle_qfProcessInfo` implementation to parse the incoming string; it was mistakenly using the full argument string `args` instead of the individual argument string `arg`. * Decode the hex-encoded process name string to match against in `Handle_qfProcessInfo`. It was incorrectly not decoded before. * Fix `PlatformSessionImplBase::updateProcesses` to clear the process list before enumerating/matching. * Without this fix, the process list keeps growing with redundant entries. * Implement `PlatformSessionImplBase::processMatch` to perform the actual matching against the values provided with the `qfProcessInfo` packet. They were previously ignored (and noted in a TODO). * When matching against the process name, try matching against both the process name in the `ProcessInfo` structure and the base thread name for the process. * This matching is necessary on Linux and Android because the process name is the fully qualified path name to the executable, but the match name is just the base name (e.g. "/home/user/a.out" vs "a.out") which matches the primary thread name. * This change was the most targeted fix that didn't disrupt other behavior and break other test cases that rely on `ProcessInfo.name` being the fully qualified executable file name. * Add TODOs for the remaining missing functionality. The test coverage on this code path appears to be incomplete. * Enable the `TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name` test case on Linux and Android. ## Background The `qfProcessInfo` and `qsProcessInfo` packets are and LLDB platform extension and are used together to enumerate processes on the target. They are documented [here](https://lldb.llvm.org/resources/lldbgdbremote.html#qfprocessinfo-qsprocessinfo-platform-extension). ## Validation Ranually run the `test_attach_to_process_by_name` test case on Android and Linux to confirm it passes.
1 parent 2e7f67a commit 87d5ae7

File tree

6 files changed

+69
-9
lines changed

6 files changed

+69
-9
lines changed

Headers/DebugServer2/GDBRemote/PlatformSessionImpl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,10 @@ class PlatformSessionImplBase : public DummySessionDelegateImpl {
4949

5050
private:
5151
void updateProcesses(ProcessInfoMatch const &match) const;
52+
static bool processMatch(ProcessInfoMatch const &match,
53+
ds2::ProcessInfo const &info);
54+
static bool nameMatch(ProcessInfoMatch const &match,
55+
std::string const &name);
5256
};
5357

5458
using PlatformSessionImpl =

Sources/GDBRemote/PlatformSessionImpl.cpp

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,72 @@ ErrorCode PlatformSessionImplBase::onLaunchDebugServer(Session &session,
139139

140140
void PlatformSessionImplBase::updateProcesses(
141141
ProcessInfoMatch const &match) const {
142-
// TODO(fjricci) we should only add processes that match "match"
142+
_processIterationState.vals.clear();
143143
Platform::EnumerateProcesses(
144144
true, UserId(), [&](ds2::ProcessInfo const &info) {
145-
_processIterationState.vals.push_back(info.pid);
145+
if (processMatch(match, info))
146+
_processIterationState.vals.push_back(info.pid);
146147
});
147148

148149
_processIterationState.it = _processIterationState.vals.begin();
149150
}
151+
152+
bool PlatformSessionImplBase::processMatch(ProcessInfoMatch const &match,
153+
ds2::ProcessInfo const &info) {
154+
// Allow matching against the process info name, which is the full executable
155+
// file path on Linux, or the name of the primary thread.
156+
if (!match.name.empty() &&
157+
!nameMatch(match, info.name) &&
158+
!nameMatch(match, Platform::GetThreadName(info.pid, info.pid)))
159+
return false;
160+
161+
if (match.pid != 0 && info.pid != match.pid)
162+
return false;
163+
164+
#if !defined(OS_WIN32)
165+
if (match.parentPid != 0 && info.parentPid != match.parentPid)
166+
return false;
167+
168+
if (match.effectiveUid != 0 && info.effectiveUid != match.effectiveUid)
169+
return false;
170+
171+
if (match.effectiveGid != 0 && info.effectiveGid != match.effectiveGid)
172+
return false;
173+
#endif
174+
175+
if (match.realGid != 0 && info.realGid != match.realGid)
176+
return false;
177+
178+
if (match.realUid != 0 && info.realUid != match.realUid)
179+
return false;
180+
181+
// TODO(andrurogerz): account for match.triple
182+
return true;
183+
}
184+
185+
bool PlatformSessionImplBase::nameMatch(ProcessInfoMatch const &match,
186+
std::string const &name) {
187+
if (match.nameMatch == "equals")
188+
return name.compare(match.name) == 0;
189+
190+
if (match.nameMatch == "starts_with")
191+
return name.size() >= match.name.size() &&
192+
name.compare(0, match.name.size(), match.name) == 0;
193+
194+
if (match.nameMatch == "ends_with")
195+
return name.size() >= match.name.size() &&
196+
name.compare(name.size() - match.name.size(),
197+
match.name.size(),
198+
match.name) == 0;
199+
200+
if (match.nameMatch == "contains")
201+
return name.rfind(match.name) != std::string::npos;
202+
203+
if (match.nameMatch == "regex")
204+
// TODO(andrurogerz): match against "regex"
205+
DS2LOG(Error, "name_match:regex is not currently supported");
206+
207+
return true;
208+
}
150209
} // namespace GDBRemote
151210
} // namespace ds2

Sources/GDBRemote/Session.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2495,15 +2495,15 @@ void Session::Handle_qfProcessInfo(ProtocolInterpreter::Handler const &,
24952495

24962496
ParseList(args, ';', [&](std::string const &arg) {
24972497
std::string key, value;
2498-
size_t colon = args.find(':');
2498+
size_t colon = arg.find(':');
24992499
if (colon == std::string::npos)
25002500
return;
25012501

2502-
key = args.substr(0, colon);
2503-
value = args.substr(colon + 1);
2502+
key = arg.substr(0, colon);
2503+
value = arg.substr(colon + 1);
25042504

25052505
if (key == "name") {
2506-
match.name = value;
2506+
match.name = HexToString(value);
25072507
} else if (key == "name_match") {
25082508
match.nameMatch = value;
25092509
} else if (key == "pid") {

Support/Testing/Excluded/ds2/android-x86_64.excluded

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ TestNonStop.LldbGdbServerTestCase.test_vCont_then_partial_stop_run_both_llgs
108108
TestNonStop.LldbGdbServerTestCase.test_vCont_then_stop_llgs
109109
TestNonStop.LldbGdbServerTestCase.test_vCtrlC_llgs
110110
TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_id_correct_executable_offset
111-
TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name
112111
TestRegistersIterator.RegistersIteratorTestCase.test_iter_registers_dwarf
113112
TestRegistersIterator.RegistersIteratorTestCase.test_iter_registers_dwo
114113
TestReportData.AsanTestReportDataCase.test_dwo

Support/Testing/Excluded/ds2/linux-i686.excluded

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ TestNoreturnUnwind.NoreturnUnwind.test_dwo
169169
TestPlatformCommand.PlatformCommandTestCase.test_shell
170170
TestPrintfAfterUp.Radar9531204TestCase.test_expr_commands_dwarf
171171
TestPrintfAfterUp.Radar9531204TestCase.test_expr_commands_dwo
172-
TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name
173172
TestRegistersIterator.RegistersIteratorTestCase.test_iter_registers_dwarf
174173
TestRegistersIterator.RegistersIteratorTestCase.test_iter_registers_dwo
175174
TestReturnValue.ReturnValueTestCase.test_with_python_dwarf

Support/Testing/Excluded/ds2/linux-x86_64.excluded

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,6 @@ TestNoreturnUnwind.NoreturnUnwind.test_dwarf
164164
TestNoreturnUnwind.NoreturnUnwind.test_dwo
165165
TestPrintfAfterUp.Radar9531204TestCase.test_expr_commands_dwarf
166166
TestPrintfAfterUp.Radar9531204TestCase.test_expr_commands_dwo
167-
TestProcessAttach.ProcessAttachTestCase.test_attach_to_process_by_name
168167
TestProcessSaveCoreMinidump.ProcessSaveCoreMinidumpTestCase.test_save_linux_mini_dump_default_options_dwarf
169168
TestProcessSaveCoreMinidump.ProcessSaveCoreMinidumpTestCase.test_save_linux_mini_dump_default_options_dwarf
170169
TestProcessSaveCoreMinidump.ProcessSaveCoreMinidumpTestCase.test_save_linux_mini_dump_default_options_dwo

0 commit comments

Comments
 (0)