[arch] Arch_DebuggerIsAttachedPosix: use TracerPid in /proc/self/status #3014
Conversation
|
Filed as internal issue #USD-9478 |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The old implemenation worked by trying to attach a ptrace to the current process, and if it failed, then we assumed it was being debugged. The problem is that we have no easy way to check if an EPERM result from ptrace is because it's being debugged, or an actual permissions error - and permissions checking process is somewhat complex - see the "Ptrace access mode checking" section here: https://man7.org/linux/man-pages/man2/ptrace.2.html On a basic Ubuntu-22.04 test system, ptrace would return EPERM even when the process was not being debugged, resulting in a "false positive". The new TracerPid method was more accurate in this case. Checking the /proc/self/status for TracerPid is also the same method that gperftools and even gdb itself use to check if a process is being debugged: - https://github.com/gperftools/gperftools/blob/02adc8ceab39bbeac1f65e10bde577e1753094fa/src/heap-checker.cc#L183 - https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/nat/linux-procfs.c;h=b17e3120792e0e0790271898212b69b0577847cc;hb=HEAD#l68 - https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/testsuite/gdb.threads/siginfo-threads.c;h=22c6038206ba77fc3432da5ee30284c80641f305;hb=HEAD#l373
pmolodo
force-pushed
the
pr/ArchDebuggerLinux
branch
from
ed2b39b
to
e6aeb47
Compare
|
I'm pulling this one in today. |
| // Do nothing | ||
| if (line.compare(0, field.size(), field) == 0 && line[field.size()] == ':') { | ||
| // We found our "field:" line | ||
| return line.substr(field.size() + 2); |
There was a problem hiding this comment.
This should be + 1 no? We want the text after the colon?
There was a problem hiding this comment.
Ah - good catch. I think the what happened was I initially thought that what followed the : was (possibly multiple) space characters - ie, "Field: value", and decided not to deal with whitespace stripping, added this comment:
// Note that the returned string will generally include leading whitespace
But I then realized what followed was always a tab character - ie, "Field:\tvalue", and I modified the code to just skip the tab character... but forgot to add an explanatory comment (or remove the old comment).
Probably what you've proposed (locale-independent whitespace stripping) is more robust, though.
| static | ||
| int Arch_ReadTracerPid() { | ||
| try { | ||
| return std::stoi(Arch_ReadProcStatusField("TracerPid")); |
There was a problem hiding this comment.
I think I'm going to rework this bit to do manual whitespace skipping and std::from_chars() to make it locale-independent.
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description of Change(s)
The old implemenation worked by trying to attach a ptrace to the current process, and if it failed, then we assumed it was being debugged.
The problem is that we have no easy way to check if an
EPERMresult from ptrace is because it's being debugged, or an actual permissions error - and permissions checking process is somewhat complex - see the "Ptrace access mode checking" section here:https://man7.org/linux/man-pages/man2/ptrace.2.html
On a basic Ubuntu-22.04 test system, ptrace would return
EPERMeven when the process was not being debugged, resulting in a "false positive". The newTracerPidmethod was more accurate in this case.Checking the
/proc/self/statusforTracerPidis also the same method that gperftools and even gdb itself use to check if a process is being debugged:Fixes Issue(s)
ArchDebuggerIsAttachedwould return a false positive on some Linux distros (ie, Ubuntu-22.04)