Page MenuHomePhabricator

[lldb] Introduce i386 support in NetBSD Process plugin
ClosedPublic

Authored by mgorny on Fri, Jan 31, 1:04 PM.

Details

Summary

Introduce support for i386 platform that is shared with amd64
in the same plugin. The concept is partially based on the Linux
implementation.

The plugin tries to reuse as much code as possible. As a result, i386
register enums are mapped into amd64 values and those are used in actual
code. The code for accessing FPU and debug registers is shared,
although general-purpose register layouts do not match between the two
kernel APIs and need to be #ifdef-ed.

This layout will also make it possible to add support for debugging
32-bit programs on amd64 with minimal added code.

In order for this to work, I had to add missing data for debug registers
on i386.

Diff Detail

Event Timeline

mgorny created this revision.Fri, Jan 31, 1:04 PM
krytarowski added inline comments.Fri, Jan 31, 1:49 PM
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

Please add uint32_t tlsbase;.

mgorny marked an inline comment as done.Fri, Jan 31, 10:24 PM
mgorny added inline comments.
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

To what purpose? It's not being used anywhere.

krytarowski added inline comments.Sat, Feb 1, 12:23 AM
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

Consistency with amd64 AND we will want to make use of it in future and present as a 'tlsbase' register or anything.

mgorny marked an inline comment as done.Sat, Feb 1, 12:47 AM
mgorny added inline comments.
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

The only reason I didn't remove it from amd64 is because I don't believe it's worth the effort. I'm against proactively adding undocumented features, especially given how much time I've spent trying to figure out what those structures were doing. We can add it when we start using it.

krytarowski added inline comments.Sat, Feb 1, 8:47 AM
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

This was discussed when we were adding amd64 fields and it was decided to add tlsbase. Please add it here too.

krytarowski added inline comments.Sat, Feb 1, 3:56 PM
lldb/source/Plugins/Process/Utility/RegisterContextNetBSD_i386.cpp
63

Also there is now ptrace(2) API ready (PT_LWPSTATUS) to read tlsbase and I wish I saw its support in LLDB sooner than later.

mgorny updated this revision to Diff 242324.Tue, Feb 4, 6:42 AM

Added tlsbase.

labath added a comment.Tue, Feb 4, 3:17 PM

This seems ok to me. I am happy with it if Kamil is.

krytarowski accepted this revision.Wed, Feb 5, 12:42 AM
This revision is now accepted and ready to land.Wed, Feb 5, 12:42 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptWed, Feb 5, 4:37 AM