This is an archive of the discontinued LLVM Phabricator instance.

[lldb] [Process/FreeBSDRemote] Access FPR via RegisterInfo offsets
ClosedPublic

Authored by mgorny on Nov 11 2020, 3:56 AM.

Details

Summary

Use offset-based method to access base x87 FPU registers, using offsets
relative to the position of 'struct FPR', as determined by the location
of first register in it (fctrl). Change m_fpr to use a fixed-size array
matching FXSAVE size (512 bytes). Add unit tests for verifying
RegisterInfo offsets and sizes against the FXSAVE layout.

Diff Detail

Event Timeline

mgorny requested review of this revision.Nov 11 2020, 3:56 AM
mgorny created this revision.
mgorny updated this revision to Diff 304479.Nov 11 2020, 4:31 AM

Rename ASSERT_FPR to ASSERT_OFF since it's a more generic offset-based assert.

mgorny updated this revision to Diff 304861.Nov 12 2020, 9:01 AM

Update to match changes in GPR patch.

labath added inline comments.Nov 13 2020, 2:06 AM
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
563–570

what if m_gpr and m_fpr were actually a single concatenated buffer? Could we then ditch GetFPROffset and just have a single memcpy here?

mgorny added inline comments.Nov 13 2020, 2:23 AM
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
563–570

We'd need GetFPROffset() for ptrace calls then ;-). I've got an another idea ready, just need to wait for some free RAM to test it. That said, we might reach the point where combining all these diffs is easier than reviewing them separately.

labath added inline comments.Nov 13 2020, 2:45 AM
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
563–570

We'd need GetFPROffset() for ptrace calls then ;-)

To get the address of the regset buffer, right?

I'd imagine that could be handled by some function like GetFPRBuffer() (similar one exists in linux), or even GetBuffer(regset). I'd imagine that would be useful for the generic caching infrastructure as well.

mgorny updated this revision to Diff 305087.Nov 13 2020, 3:50 AM

Rebased.

labath accepted this revision.Nov 13 2020, 6:20 AM
This revision is now accepted and ready to land.Nov 13 2020, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 16 2020, 4:03 AM