Page MenuHomePhabricator

LLDB ARM Register context support
ClosedPublic

Authored by omjavaid on Mar 31 2015, 6:54 AM.

Details

Summary

This patch adds support for ARM Register contexts in LLDB.

This is a preliminary patch and is not tested on arm platform as yet. PTRACE on arm work similar to x86 so should work as it is on x86_64 with may be a few minor fixes. Some intelligent hacks are needed for thumb/arm mode switches and little/bid endian switches which I have ignored in this first patch.

There are some obvious issues like repetitive code which may go out once we refactor some redundant code for all architectures not just arm.

Complementing previous ABI patch submitted last week most of the missing pieces for arm are now ve been written.

Diff Detail

Event Timeline

omjavaid updated this revision to Diff 22951.Mar 31 2015, 6:54 AM
omjavaid retitled this revision from to LLDB ARM Register context support .
omjavaid updated this object.
omjavaid edited the test plan for this revision. (Show Details)
omjavaid added reviewers: tberghammer, rengolin, vharron.
omjavaid added a subscriber: Unknown Object (MLST).
emaste added a subscriber: emaste.Mar 31 2015, 7:53 AM
emaste added inline comments.Mar 31 2015, 8:00 AM
Host/posix/PipePosix.cpp
271 ↗(On Diff #22951)

Extra changes included by accident, I presume

Plugins/Process/POSIX/POSIXThread.cpp
199–202 ↗(On Diff #22951)

suggest keeping in alphabetical order (here and several other places)

Plugins/Process/POSIX/ProcessPOSIX.cpp
678–679 ↗(On Diff #22951)

ProcessPOSIX.cpp is used by Linux and FreeBSD so if Linux implements non-standard behaviour this will need to be changed

Plugins/Process/elf-core/ProcessElfCore.cpp
428 ↗(On Diff #22951)

32-bit arm is not LP64

Plugins/Process/elf-core/ThreadElfCore.cpp
135–137 ↗(On Diff #22951)

alpha order

157–159 ↗(On Diff #22951)

alpha order

abidh added a subscriber: abidh.Mar 31 2015, 8:01 AM

case llvm::Triple::arm is added in different order in switches. Not a biggie but good to have same order everywhere. A few other changes seem unrelated to this patch.

Host/posix/FileSystem.cpp
188 ↗(On Diff #22951)

Does not seem related.

Host/posix/PipePosix.cpp
271 ↗(On Diff #22951)

Does not seem related to this patch. Was this intentional?

Plugins/Process/elf-core/ProcessElfCore.cpp
431 ↗(On Diff #22951)

I think emaste will be best person to review this change.

tberghammer edited edge metadata.Mar 31 2015, 8:49 AM

PosixThread and ProcessPosix don't used on Linux but it is used on FreeBSD. There is no problem with adding arm handling to there but don't expect any effect from it on Linux.

Plugins/Process/POSIX/ProcessPOSIX.cpp
678–700 ↗(On Diff #22951)

AFAIK this class is not used on Linux because of the "remote debugging approach" for local debugging also. Linux uses the code in PlatformLinux and in NativeProcessLinux (unfortunately both) so please add the trap opcode there.

Plugins/Process/Utility/RegisterInfos_arm.h
178–181 ↗(On Diff #22951)

Please mark them with LLDB_REGNUM_GENERIC_ARG[1-4] (required for expression evaluation)

tberghammer added inline comments.Apr 2 2015, 6:09 AM
Plugins/Process/Utility/RegisterInfos_arm.h
185 ↗(On Diff #22951)

Are you sure r7 is FP? I haven't found it in the spec but based on some testing I think it is r11 (on a Nexus 5). Please also add an alias for the fp register the same way it is done for sp.

omjavaid updated this revision to Diff 23311.Apr 6 2015, 11:54 PM
omjavaid edited edge metadata.

I have updated the patch after incorporating suggestions.

I have changed the alpha order issues and also I have marked argument registers as suggested.

About arm frame pointer I am still not sure if R11 is the right choice but as it works on nexus 5 I have made that change. Arm FP should be r7 in thumb mode and r11 in arm mode.

I am not sure if gnu-linux even uses a frame pointer.

Regarding arm trap handling, there is still a bit of work required to handle arm/thumb mode trap in little/big endian modes. I have skipped that change for now and will work on that and submit it later.

Host/Filesystem and Host/Pipeposix changes were some issues that were only visible on my build system so I have removed them from the patch and will later submit them with better solution and convincing arguments.

Looking forward to your consideration for accepting these changes.

PS: Can someone kindly help me with getting these patches committed or getting the commit rights to do it myself once they are accepted.

Thanks!

tberghammer accepted this revision.Apr 7 2015, 3:19 AM
tberghammer edited edge metadata.

About the frame pointer on arm, I tested it with arm instruction set so this is why I seen it in r11 (I haven't known it is different with thumb). If it is located in different register based on the instruction set then we will have to find a way to handle it but it should be done later.

It looks good as a first step for getting arm32 working. There is a lot of things left before it will actually work properly but I prefer to do those changes as separate patches so we don't have 1 huge patch.

I added Greg Clayton as a reviewer (global LLDB lead) and if he approves it then I will happily help you committing it in.

This revision is now accepted and ready to land.Apr 7 2015, 3:19 AM

Friendly ping (@clayborg please take a look)

clayborg accepted this revision.Apr 13 2015, 11:12 AM
clayborg edited edge metadata.

Looks good.

tberghammer closed this revision.Apr 24 2015, 6:36 AM