[Lldb-commits] [PATCH] D39681: Implement core dump debugging for PPC64le

John Baldwin via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Nov 16 09:47:35 PST 2017


bsdjhb added inline comments.


================
Comment at: source/Plugins/Process/elf-core/elf-core-enums.h:14
+/// Core files PT_NOTE segment descriptor types
+enum {
+  NT_PRSTATUS = 1,
----------------
krytarowski wrote:
> labath wrote:
> > krytarowski wrote:
> > > alexandreyy wrote:
> > > > krytarowski wrote:
> > > > > alexandreyy wrote:
> > > > > > krytarowski wrote:
> > > > > > > No namespace here?
> > > > > > I think these constants are used for multiple OSs.
> > > > > > Am I correct?
> > > > > > Do you have a suggestion for a namespace?
> > > > > There is rumor that they came from SVR4.. but I don't have the specs to make sure.
> > > > > 
> > > > > Multiple in this case does not mean "all". This is not true for at least NetBSD.
> > > > I can remove these constants and modify the first "if" in ParseThreadContextsFromNoteSegment to:
> > > > 
> > > >   if (((note.n_type == LINUX::NT_PRSTATUS ||
> > > >           note.n_type == FREEBSD::NT_PRSTATUS) && have_prstatus) ||
> > > >         ((note.n_type == LINUX::NT_PRPSINFO ||
> > > >           note.n_type == FREEBSD::NT_PRPSINFO) && have_prpsinfo)) {
> > > >       assert(thread_data->gpregset.GetByteSize() > 0);
> > > > 
> > > > What do you think?
> > > > But NT_PRSTATUS and NT_PRPSINFO have the same value for Linux and FreeBSD .
> > > I propose to put NT_PRSTATUS, NT_PRFPREG, NT_PRPSINFO into this namespace and call it SVR4.
> > > 
> > > SmartOS uses the same values.
> > With the addition of all the bsd variants, the note parsing code has grown pretty big.
> > 
> > My plan was to refactor it a bit after this is committed. I wanted to split it into two stages: first we just determine the OS from the notes; and then we dispatch to an appropriate os-specific function to do the actual parsing (with each function just using the NT constants from its own namespace). If you agree with that direction then I propose to go with the LINUX::NT_PRSTATUS || FREEBSD::NT_PRSTATUS proposed by alexandreyy in the interim.
> Sounds good.
Yes, that is probably the right approach.  In binutils the note parsing code switches on the note name ("FreeBSD" vs "NetBSD" vs "CORE" vs "Linux", etc.) and then inside name-specific functions switches on the note type.  I've been adding support for more notes to GDB for FreeBSD and ended up splitting out a handler for "FreeBSD" notes inline with this approach.  Even notes with the same type (NT_PRSTATUS, etc.) have different layouts across OS's.


Repository:
  rL LLVM

https://reviews.llvm.org/D39681





More information about the lldb-commits mailing list