This is an archive of the discontinued LLVM Phabricator instance.

PTRACE ABI to read FXSAVE area for 32-bit inferior
ClosedPublic

Authored by abhishek.aggarwal on Nov 27 2015, 6:26 AM.

Details

Summary
  • Problem occurs when:
    • 32-bit inferiors run on x86_32 machine and the architecture doesn't have AVX feature
    • This causes FPRType to be set to eFPRTypeFXSAVE
    • PTRACE_GETFPREGS was being used to read FXSAVE area
    • For 32-bit inferiors running on x86_32 machine, PTRACE_GETFPREGS reads FSAVE area and not FXSAVE area
  • Changed ptrace API to PTRACE_GETREGSET for 32-bit inferiors
    • This reads FPR data in FXSAVE format.
    • For 64-bit inferiors, no change has been made.
  • Modified XFAIL for TestReturnValue.py
    • Earlier, this test was passing for Linux OS
    • Now, it passes for Android OS as well

Change-Id: Ieed72bc969b79516fc7b263b32493aa1e7a1a2ac
Signed-off-by: Abhishek Aggarwal <abhishek.a.aggarwal@intel.com>

Diff Detail

Event Timeline

abhishek.aggarwal retitled this revision from to PTRACE ABI to read FXSAVE area for 32-bit inferior.
abhishek.aggarwal updated this object.
tberghammer added inline comments.Nov 27 2015, 6:44 AM
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
849–850

I think we don't want this assert because of 2 reason:

  • If we are running on a machine where we can't access to any floating point registers then this assert will fire but lldb is still functional in that case
  • It is changing the internal state of the object so it will cause a different behavior in debug and in release builds what we should avoid as it can cause hard to diagnose issues

Removed assert if ptrace API fails

clayborg resigned from this revision.Nov 30 2015, 9:19 AM
clayborg removed a reviewer: clayborg.

I can't intelligently comment on low level ptrace stuff, so I will defer to others that can.

Hello Greg

Is there anyone else , except the reviewers already included for the patch, I should put as the reviewer for this patch ?

labath accepted this revision.Dec 1 2015, 3:23 AM
labath added a reviewer: labath.
labath added a subscriber: labath.

Looks good.

This revision is now accepted and ready to land.Dec 1 2015, 3:23 AM
tberghammer accepted this revision.Dec 1 2015, 3:33 AM
tberghammer added a reviewer: tberghammer.

Looks good, and as it only effects Linux you can commit it just based on the review from Pavel

abhishek.aggarwal edited edge metadata.

Added Loggings

An indication through logs in case ptrace API fails
to read FXSAVE/XSAVE area.

Hi Pavel and Tamas

I enabled logs in case ptrace API fails to read FXSAVE/XSAVE area. This will serve as an indicator to find the reason of failure if register read fails.

tberghammer added inline comments.Dec 1 2015, 5:26 AM
source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
853–854

Adding a log message is a good idea but please include the error message from ReadFPR in the log message. It will usually contain the error message returned by the ptrace call.

Comments Inlined.

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
853–854

Error message from ReadFPR along with the error number (in case ptrace API fails) are displayed during call of NativeProcessLinux::PtraceWrapper(). Hence, to avoid multiplicity, I didn't include it here. The Log message I included is an additional information regarding ptrace API failure.