Page MenuHomePhabricator

Fix issue where GPR and FPR registers have overlapping byte offsets.
ClosedPublic

Authored by chaoren on Mar 28 2015, 10:53 AM.

Details

Summary

The implementation of GDBRemoteRegisterContext relies on byte offsets to cache
register values. GPR, FPR, etc. should start on different offsets. This is
correctly done in debugserver (in DNBArchImplX86_64.cpp), but not on Linux or
FreeBSD (in RegisterInfos_x86_64.h).

Diff Detail

Repository
rL LLVM

Event Timeline

chaoren retitled this revision from to Fix issue where GPR and FPR registers have overlapping byte offsets..
chaoren updated this object.
chaoren edited the test plan for this revision. (Show Details)
chaoren added a subscriber: Unknown Object (MLST).
chaoren added inline comments.Mar 28 2015, 10:55 AM
source/Plugins/Process/Utility/RegisterContextFreeBSD_x86_64.cpp
55 ↗(On Diff #22841)

This doesn't sound right, but I don't know enough about FreeBSD to dispute it.

Ping? Anyone know who would be the right person to review this?

clayborg accepted this revision.Apr 1 2015, 12:58 PM
clayborg edited edge metadata.

Looks good to me. I usually create assembler files that load all registers up with known values and then stop at a label like "stop_here" and debug the program and verify all registers are where they should be. It would be a good idea for us to start doing this for all targets we support.

This revision is now accepted and ready to land.Apr 1 2015, 12:58 PM
This revision was automatically updated to reflect the committed changes.

I usually create assembler files that load all registers up with known
values and then stop at a label like "stop_here" and debug the program and
verify all registers are where they should be.

Does this mean a test like that already exists for OS X?

No, I do this when first debugging a new Process plug-in or architecture. I wasn't sure about the assembler portability so I never checked on in.

emaste edited edge metadata.Apr 2 2015, 9:07 AM

This causes >20 tests to segfault on FreeBSD.

The debug register implementation on FreeBSD is a bit different, see rL201706. I can work around it for now by moving DR_OFFSET's definition back into RegisterContextFreeBSD_x86_64.cpp and RegisterContextLinux_x86_64.cpp.

Whoops, I apologize. I'll fix it when I get to the office. The GPR/FPR
offset change doesn't cause any problems on FreeBSD right? I just sort of
just plopped a dummy UserArea in there for FreeBSD to compile. It would be
great if I could find where the Linux one came from, and where to get the
correct FreeBSD layout.

emaste added a comment.Apr 3 2015, 1:54 PM

Whoops, I apologize. I'll fix it when I get to the office. The GPR/FPR
offset change doesn't cause any problems on FreeBSD right? I just sort of
just plopped a dummy UserArea in there for FreeBSD to compile. It would be
great if I could find where the Linux one came from, and where to get the
correct FreeBSD layout.

I'm happy to see this sort of cleanup happening and a little collateral damage is understandable. I've committed rL234048 as a temporary change to restore basic functionality and fix the buildbot; we'll want to investigate a bit further to come up with a clean fix as you describe.