[Lldb-commits] [PATCH] D159076: [lldb] Add DynamicLoader for FreeBSD Kernel post-mortem debug facility

[Ed Maste via Phabricator via lldb-commits]

emaste added inline comments.


================
Comment at: lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp:115-116
+    // TODO: If we need to check unknow OS triple like armv7-unknown-unknown?
+    // TODO: I don't know if kFreeBSD is a type of FreeBSD and should we accept
+    // this Triple? Check the type of kernel
+    if (!triple_ref.isOSFreeBSD()) {
----------------
kFreeBSD indicates a FreeBSD kernel, typically with some other userland (e.g. Debian GNU/kFreeBSD is Debian's standard userland, with FreeBSD instead of Linux as the kernel). But these projects are mostly not active anymore and I'm not sure how we'd test.


================
Comment at: lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp:183
+  if (header.getDataEncoding() == llvm::ELF::ELFDATA2MSB) {
+    // TODO: swap byte order for big endian
+  }
----------------
Add `return false;` for now as well?


================
Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:943
+      if (header.sh_flags & SHF_ALLOC)
+	return Address(GetSectionList()->FindSectionByID(SectionIndex(I)), 0);
+    }
----------------
The indentation looks wrong on a few lines here, 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159076/new/

https://reviews.llvm.org/D159076