Page MenuHomePhabricator

ELF core: Adding parsing of the floating-point and SSE registers on x86 32/64 bit elf core files
ClosedPublic

Authored by dvlahovski on Nov 4 2016, 9:50 AM.

Details

Summary

The floating-point and SSE registers could be present in the elf-core
file in the note NT_FPREGSET for 64 bit ones, and in the note
NT_PRXFPREG for 32 bit ones.

The entire note is a binary blob matching the layout of the x87 save
area that gets generated by the FXSAVE instruction (see Intel developers
manual for more information).

This CL mainly modifies the RegisterRead function in
RegisterContextPOSIXCore_x86_64 for it to return the correct data both
for GPR and FPR/SSE registers, and return false (meaning "this register
is not available") for other registers.

I added a test to TestElfCore.py that tests reading FPR/SSE registers
both from a 32 and 64 bit elf-core file and I have inluded the source
which I used to generate the core files.

I tried to also add support for the AVX registers, because this info could
also be present in the elf-core file (note NT_X86_XSTATE - that is the result of
the newer XSAVE instruction). Parsing the contents from the file is
easy. The problem is that the ymm registers are split into two halves
and they are in different places in the note. For making this work one
would either make a "hacky" approach, because there won't be
any other way with the current state of the register contexts - they
assume that "this register is of size N and at offset M" and
don't have the notion of discontinuos registers.

Diff Detail

Repository
rL LLVM

Event Timeline

dvlahovski updated this revision to Diff 76918.Nov 4 2016, 9:50 AM
dvlahovski retitled this revision from to ELF core: Adding parsing of the floating-point and SSE registers on x86 32/64 bit elf core files.
dvlahovski updated this object.
dvlahovski added a reviewer: labath.
dvlahovski added a subscriber: lldb-commits.
emaste added a subscriber: emaste.Nov 4 2016, 1:47 PM

We need to make sure this does not regress FreeBSD core handling -- I will test as soon as I can, and add FXSAVE parsing for FreeBSD. I'll use fpr_sse.cpp to generate FreeBSD core files as well (although note that we get ~2.5MB cores with default configuration, so tweaking malloc options will be necessary).

Please document how the core files were created (e.g. compiler invocation), perhaps as a comment in the source file.

packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
107 ↗(On Diff #76918)

Curious, why are these skipped only on windows?

labath edited edge metadata.Nov 4 2016, 2:45 PM

I was very successful in creating tiny core files by avoiding linking with the standard library (you don't really need a full libc to crash :P ).
I think this file should use the same approach as well (see the other core files in this directory, and the make-core.sh script).

Ed, maybe you could start by tweaking that script to work on FreeBSD as well? Depending on how similar it ends up, we may want to keep it here (and rename the folder to just "elf-core", or we can create a separate "freebsd-core" folder instead...

emaste added a comment.Nov 4 2016, 4:22 PM

Good point. D26315 has the change to add FreeBSD support to make-core.sh -- just avoiding /bin/bash, and the core file handler check.

I think we should be able to rename this directory to elf-core, and rename the individual cores to e.g. linux-i386.core, etc.

labath added a comment.Nov 5 2016, 2:49 AM

Good point. D26315 has the change to add FreeBSD support to make-core.sh -- just avoiding /bin/bash, and the core file handler check.

Great. I am glad the _start thingy works the same way on FreeBSD as well. I think this approach has a lot of potential. We probably don't want to check in zillions of core files, even if they are tiny, but it should enable us to do at least a basic smoke test of our changes on OS/hardware combinations we don't have access to.

I think we should be able to rename this directory to elf-core, and rename the individual cores to e.g. linux-i386.core, etc.

I'll get to that on Monday, if you don't beat me to it. :)

labath added a comment.Nov 7 2016, 2:21 AM

I've renamed to folder into elf-core and put "linux" into the individual file names. I did not rename TestLinuxCore.py as I am still not sure what to do about it -- if we end up having a lot of these tests, we may want a separate TestFreeBSDCore.py, with the common code refactored into a base class.

I think you should be able to add a basic freebsd core file test, based on the existing main.c. We'll refactor this change after that (PS: Dimitar's internship is over, so I may end up taking this over).

packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
107 ↗(On Diff #76918)

My guess would be that we fail to select the correct platform instance when opening the core there, so we end up trying to debug a linux core file with PlatformWindows, or something like that. I'll need to look into that at some point.

I will generate the core files with the make-core.sh script and update the revision today or tomorrow.

packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
107 ↗(On Diff #76918)

To be honest, I copied this from the other ELF core tests without investigating why they skip windows.

dvlahovski updated this revision to Diff 77477.Nov 10 2016, 6:47 AM
dvlahovski edited edge metadata.

Generated the core files with make-core.sh

Looks great. Ed, do you want to give this a try run?

packages/Python/lldbsuite/test/functionalities/postmortem/linux-core/TestLinuxCore.py
113 ↗(On Diff #77477)

Please add "linux" to the core file names. We'll probably have freebsd versions of these as well..

source/Plugins/Process/elf-core/RegisterContextPOSIXCore_x86_64.h
49 ↗(On Diff #77477)

Could you use a unique_ptr instead? The data doesn't seem to be shared with anyone...

dvlahovski updated this revision to Diff 77626.Nov 11 2016, 9:05 AM
dvlahovski marked an inline comment as done.

Use unique_ptr instead of shared_ptr

dvlahovski marked an inline comment as done.Nov 11 2016, 9:06 AM

I applied this change on top of my WIP D26617 (which adds a FreeBSD core test). LLDB builds and my new test passes, so no objections from me. Once it's in I'll see about porting the new SSE test to FreeBSD too.

labath accepted this revision.Nov 15 2016, 1:30 AM
labath edited edge metadata.
This revision is now accepted and ready to land.Nov 15 2016, 1:30 AM
This revision was automatically updated to reflect the committed changes.