[Lldb-commits] [PATCH] D32149: Correct handling NetBSD core(5) files with threads

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.


================
Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1374
+    else if ((note.n_name == LLDB_NT_OWNER_NETBSDCORE) &&
+             (note.n_type == LLDB_NT_NETBSD_NT_PROCINFO)) {
       // Set the elf OS version to NetBSD.  Also clear the vendor.
----------------
kettenis wrote:
> By making this change you risk losing recognition of "normal" NetBSD ELF files, i.e. executables and shared libraries.
Hmm, do you mean these notes:

```
$ readelf -n /bin/cat                                                                                                                                 

Displaying notes found at file offset 0x00000214 with length 0x00000018:
  Owner                 Data size	Description
  NetBSD		0x00000004	IDENT 799005900 (7.99.59)

Displaying notes found at file offset 0x0000022c with length 0x00000014:
  Owner                 Data size	Description
  NetBSD		0x00000004	PaX <>
```


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:563
+
+/// Generic (Linux, ...) specific Thread context from PT_NOTE segment
+/// 1) A Thread Context in a core file usually described by 3 NOTE entries.
----------------
kettenis wrote:
> A bit strange to call something both generic and specific...
> 
> I think some of these note types originated on SVR4/Solaris, but I'm not sure how much Linux deviated from that design through the years.
Can we call it: Type1 core (as of now: NetBSD) and Type2 core (Linux, Android, FreeBSD, OpenBSD)?

I don't care which would be 1 and which 2.


================
Comment at: source/Plugins/Process/elf-core/ProcessElfCore.cpp:815
+            return Error("Error parsing NetBSD core(5) notes: Cannot convert "
+                         "LWP ID to integer");
+        } else if (note.n_type == NETBSD::AMD64::NT_FPREGS) {
----------------
kettenis wrote:
> Wouldn't it make sense to move the parsing of the LWP ID before the switch. Otherwise you'll have to duplicate the code for every NetBSD architecture you'll add.
Sounds good.


Repository:
  rL LLVM

https://reviews.llvm.org/D32149