This is an archive of the discontinued LLVM Phabricator instance.

Introduce support for Debug Registers in RegisterContextNetBSD_x86_64
ClosedPublic

Authored by krytarowski on Feb 23 2017, 12:38 AM.

Details

Summary

NetBSD 7.99.62 introduced Debug Registers interface similar to the FreeBSD one.
This interface will land NetBSD-8.0.

Introduce support for this interface in Register Context NetBSD x86_64 unconditionally as older versions of NetBSD will not be supported.

This change allows to reduce diff with other ports and remove local copy of the RegisterInfos_x86_64.h content.

NetBSD Register Context for 32-bit x86 support will be added later.

Sponsored by <The NetBSD Foundation>

Diff Detail

Repository
rL LLVM

Event Timeline

krytarowski created this revision.Feb 23 2017, 12:38 AM
krytarowski retitled this revision from Introduce support for Debug Registers RegisterContextNetBSD_x86_64 to Introduce support for Debug Registers in RegisterContextNetBSD_x86_64.Feb 23 2017, 12:55 AM
labath accepted this revision.Feb 23 2017, 2:10 AM

Good stuff, just remove the unused function.

source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
87

It looks like you copied this from freebsd code, but it does not appear to be used. Can this function be removed?

This revision is now accepted and ready to land.Feb 23 2017, 2:10 AM
clayborg added inline comments.Feb 23 2017, 11:50 AM
source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
51–56

Would it be nicer to have a union in here? Something like:

struct DebugRegs
{
  uint64_t debug_addr1;
  uint64_t debug_addr2;
  uint64_t debug_addr3;
  uint64_t reserved1;
  uint64_t reserved2;
  uint64_t debug_status;
  uint64_t debug_control;
};
struct DBG {
  union {
    uint64_t dr[16];
    DebugRegs regs;
  };
};
krytarowski added inline comments.Feb 23 2017, 2:00 PM
source/Plugins/Process/Utility/RegisterContextNetBSD_x86_64.cpp
51–56

I prefer to Follow Linux and FreeBSD here. I don't see similar union in other ports.

76

I checked Linux code and perhaps this define should look this way:

#define DR_OFFSET(reg_index)                                                   \
  (LLVM_EXTENSION offsetof(UserArea, dbg) +                                    \
LLVM_EXTENSION offsetof(DBG, dr[reg_index]))
clayborg accepted this revision.Feb 23 2017, 2:08 PM

That is fine, just wanted to check.

  • remove unused function GetSharedRegisterInfoVector
  • fix DR_OFFSET
krytarowski closed this revision.Feb 23 2017, 6:05 PM