Page MenuHomePhabricator

lldb gdb inspired use of ptrace on arm64
ClosedPublic

Authored by pawelo on Aug 5 2014, 9:58 PM.

Details

Reviewers
tfiala
Summary

I looked how it is done in gdb, and found that this piece of code must be rewritten.

Diff Detail

Event Timeline

pawelo updated this revision to Diff 12220.Aug 5 2014, 9:58 PM
pawelo retitled this revision from to lldb gdb inspired use of ptrace on arm64.
pawelo updated this object.
pawelo edited the test plan for this revision. (Show Details)
pawelo set the repository for this revision to rL LLVM.
pawelo added a subscriber: Unknown Object (MLST).
pawelo updated this revision to Diff 12583.Aug 16 2014, 10:56 AM
pawelo set the repository for this revision to rL LLVM.

improved version that covers ProcessMonitor too

tfiala added a subscriber: tfiala.Aug 18 2014, 8:08 AM

Thanks Paul. I'll have a look today.

Looking at this now.

tfiala edited edge metadata.Aug 19 2014, 9:57 AM

Hey Paul,

I have just a few questions that all boil down to checking if we need a guard #ifdef around a particular pattern in the code above. (See code-inlined comments).

Aside from that, LGTM.

Tested:
Ubuntu 14.04 x86_64, clang-3.5-built lldb, all tests passed.

source/Plugins/Process/Linux/ProcessMonitor.cpp
581

Do we need an analog of #ifdef PT_GETREGS (as the x86 side has) here?

621

The x86 code is guarded by a #ifdef PT_GETFPREGS. Do we have/need an analog for that here?

690

The x86-based call was guarded by an ifdef on the PT_SETREGS define. Do we need an analog here for PTRACE_SETREGSET?

730

Do we need an analog of #ifdef PT_SETFPREGS here?

All those PT_* ifdefs were related to ARM64, so I suppose they can be safely removed.

Ok - that sounds good. Would you like to pull those out and put it in this
patch, since they seem to be atomically related?

pawelo updated this revision to Diff 12679.Aug 19 2014, 1:23 PM
pawelo edited edge metadata.
pawelo set the repository for this revision to rL LLVM.

Improved version (I hope I did it well).

Hey Paul - I'll be looking to get your patches in today (I was OOO
yesterday).

Ok - those were the changes I was looking for. Thanks for cleaning up those other #ifdefs, Paul. Testing now.

tfiala accepted this revision.Aug 21 2014, 9:41 AM
tfiala edited edge metadata.

LGTM.

Tested:
Ubuntu 14.04 x86_64: clang-3.5-built lldb. All local tests passed.

This revision is now accepted and ready to land.Aug 21 2014, 9:41 AM
tfiala closed this revision.Aug 21 2014, 9:43 AM

svn commit
Sending source/Plugins/Process/Linux/NativeProcessLinux.cpp
Sending source/Plugins/Process/Linux/ProcessMonitor.cpp
Transmitting file data ..
Committed revision 216185.