[Lldb-commits] [PATCH] D31131: [LLDB] OpenBSD support

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Mar 20 03:08:47 PDT 2017


labath added a comment.

Looks like a great start. If you want to continue and get live process debugging working as well, you should definitely sync up with kamil (i.e. don't start from ProcessFreeBSD, his process plugin should be a much better starting point).

In terms of this patch, I have a couple of comments over the supported architectures. Also, please make sure you run clang-format over your diff, as some odd formatting has crept in.

I am not going to block this change over it, but it would be great (for the sake of your own platform) if you could try to generate a tiny core file to check in, to make sure that any further lldb changes to not break you. For linux, we've gotten these down to ~30k, but we've had some problems doing the same on FreeBSD (https://reviews.llvm.org/D26617), and unfortunately I haven't yet found time to investigate this.



================
Comment at: source/Host/openbsd/Host.cpp:223
+
+#if 0
+lldb::DataBufferSP Host::GetAuxvData(lldb_private::Process *process) {
----------------
krytarowski wrote:
> Wasn't it already gone from there?
Yep, this should be deleted completely.


================
Comment at: source/Plugins/Platform/OpenBSD/PlatformOpenBSD.cpp:158
+    case 1:
+      triple.setArchName("i386");
+      break;
----------------
As far as I can tell, you're only adding x86_64 support, so you should probably remove the others.


================
Comment at: source/Plugins/Process/elf-core/ThreadElfCore.cpp:117
+      case llvm::Triple::aarch64:
+        reg_interface = new RegisterInfoPOSIX_arm64(arch);
+        break;
----------------
Have you checked that the arm64 case would work here? (at least whether the register context have roughly the same layout ?)


Repository:
  rL LLVM

https://reviews.llvm.org/D31131





More information about the lldb-commits mailing list