[lldb] [ObjectFile/ELF] Correct recognition of NetBSD images

[mgorny]

Summary

[lldb] [ObjectFile/ELF] Fix recognizing NetBSD images

Split the recognition into NetBSD executables & shared libraries
and core(5) files.

Introduce new owner type: "NetBSD-CORE", as core(5) files are not tagged
in the same way as regular NetBSD executables.

Stop using incorrectly ABI_TAG and ABI_SIZE. Introduce IDENT_TAG,
IDENT_DECSZ, IDENT_NAMESZ and PROCINFO.

The new values detect correctly the NetBSD images.

[krytarowski]

Extracted from: D32149.

[davide]

Please add a test case.

[krytarowski]

What would the test do?

[clayborg]

Probably take a ELF file that is NetBSD and obj2yaml it. The test would run yaml2obj on it and then test that things are recognized correctly via the SB interfaces (check triple is correct)?

[clayborg]

You would check in the YAML code for the NetBSD ELF file so that the test case can make it into an ELF file, run the test, verify things, and then cleanup the created ELF file.

[davide]

In D42870#996899, @clayborg wrote:

Probably take a ELF file that is NetBSD and obj2yaml it. The test would run yaml2obj on it and then test that things are recognized correctly via the SB interfaces (check triple is correct)?

The SBApi in this case is a sledgehammer. I think lldb-test or an unitest would be a better/lightweight alternative.

[krytarowski]

Is there a working example of this? I would clone an existing code for Linux or other supported OS and adapt it for NetBSD.

Please note that I'm in the process of restoration LLDB (lldb-server) so I cannot execute regular tests, at least in close time.

[davide]

In D42870#996913, @krytarowski wrote:

Is there a working example of this? I would clone an existing code for Linux or other supported OS and adapt it for NetBSD.

Please note that I'm in the process of restoration LLDB (lldb-server) so I cannot execute regular tests, at least in close time.

cc: @zturner / @labath

[jingham]

There is code in the GDBRemoteTestBase that makes targets from yaml2obj. See the createTarget method in packages/Python/lldbsuite/functionalities/gdb_remote_client/gdbclientutils.py. Actually, that's method should be pulled out of the gdb_remote_client and moved to TestBase in lldbtest.py, it doesn't appear to have any dependencies on being a gdb_remote_client test, and it seems like a generally useful feature. Note that gdb_remote_client directory also has an example of making a little ELF file (a.yaml).

It looks like the GDBRemoteTestBase class also has some mechanism for cleaning up temporary files, but I'm not sure that's necessary, since the generated files should all go into the build directory now Adrian is done with that work. I don't see why the test cases should have to do this by hand.

Anyway, if that were in lldbtest (maybe createTargetFromYaml), then you could make a dotest test by copying the packages/Python/lldbsuite/test/sample_test/TestSampleTest.py somewhere, remove the build stage, and in the test body just do something like:

target = self.createTargetFromYaml(path_to_yaml)
self.assertTrue(target.GetTriple() =="whateverItsSupposedToBe", "Got the right target")

You only need to use lldb-server to run the dotest.py based tests if you need to run the process, which you don't here. You should be able to run all the tests that don't run processes, though of course if you don't have an lldb-server you can't actually launch anything regardless of what test harness is doing the launching.

[labath]

Extending lldb-test's dumpModules() to also dump out the module's triple sounds like a reasonable thing to do (I am assuming that the Module class will do something reasonable when given an object file with no sections, like a core file -- if not we could make a separate command for dumping this out).

However, the tricky part here is the obj2yaml idea. Right now obj2yaml support for program headers is virtually non-existent. obj2yaml just ignores them, and yaml2obj only has basic support for PT_LOAD segments (and even here it assumes that their contents is backed by actual sections, which is not the case for core files). So, I don't think you can use yaml2obj here without some serious work to make it support this use case (which would be an awesome feature, but not exactly a trivial job...).

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1367 ↗

(On Diff #132682)

Let's avoid c string functions where possible. This could be setOSName(llvm::formatv("netbsd{0}.{1}.{2}", major, minor, patch).str())

[krytarowski]

I will be back to this once I will be done with debugging client-server connectivity issues (unrelated to this patch).

[mgorny]

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1359 ↗

(On Diff #132682)

Would it be crazy to rewrite this into std::div (i.e. to get both quotient and remainder in one call)?

[mgorny]

Added tests. I'm using yaml2obj for the regular executable, and binary file for the core dump (I've stripped it to initial 4k since that seems enough for lldb to recognize it).

[mgorny]

lldb/lit/Modules/ELF/netbsd-exec.yaml

1 ↗

(On Diff #187254)

I'm not sure if we should keep the whole file or try to strip this a bit.

[krytarowski]

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

1359 ↗

(On Diff #132682)

If it will be more readable OK.

[krytarowski]

lldb/lit/Modules/ELF/netbsd-core.test

3 ↗

(On Diff #187254)

I propose to keep it as netbsd$VERSION-$ARCH.core

We will want multiple core(5) files and possible with variations (FPU layout).

[mgorny]

Rename tests as requested.

[krytarowski]

lldb/lit/Modules/ELF/netbsd-core.test

3 ↗

(On Diff #187254)

We will want multiple core(5) files and possible with variations (FPU layout).

Actually such variations will be applicable for https://reviews.llvm.org/D32149 Here we just want $ARCH-$VERSION.

[mgorny]

Added comments on how test data was generated.

[mgorny]

Reduced the test .yaml to the absolute minimum.

[mgorny]

Updated to include a non-stripped core.

[mgorny]

Switched to use shorter identifiers.

[labath]

Looks good to me. Thank you for writing the tests. @davide, do you have anything to add here?

[stella.stamenova]

This broke the windows bot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/1894

lldb/lit/Modules/ELF/netbsd-core-amd64.test

+# Test whether NetBSD core dumps are recognized correctly.
+
+# Core dump generated by the following program:
+# int main() {
+#   void* v = 0;
+#   *v = 1;
+#   return 0;
+# }
+
+# RUN: lldb-test object-file %S/Inputs/netbsd-amd64.core | FileCheck %s
+# CHECK: Architecture: x86_64-unknown-netbsd
+# CHECK: Type: core file

lldb/lit/Modules/ELF/netbsd-exec-8.99.30-amd64.yaml

+# Test whether NetBSD executables are recognized correctly.
+
+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+# CHECK: Architecture: x86_64--netbsd8.99.30
+# CHECK: Type: executable
+
+--- !ELF
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+  Entry:           0x00000000002006F0
+Sections:        
+  - Name:            .note.netbsd.ident
+    Type:            SHT_NOTE
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x00000000002005A8
+    AddressAlign:    0x0000000000000004
+    Content:         0700000004000000010000004E6574425344000078B29535
+...

lldb/lit/Modules/lit.local.cfg

-config.suffixes = ['.s', '.yaml']
+config.suffixes = ['.s', '.test', '.yaml']

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

 using namespace llvm::ELF;
 
 namespace {
 
 // ELF note owner definitions
 const char *const LLDB_NT_OWNER_FREEBSD = "FreeBSD";
 const char *const LLDB_NT_OWNER_GNU = "GNU";
 const char *const LLDB_NT_OWNER_NETBSD = "NetBSD";
+const char *const LLDB_NT_OWNER_NETBSDCORE = "NetBSD-CORE";
 const char *const LLDB_NT_OWNER_OPENBSD = "OpenBSD";
 const char *const LLDB_NT_OWNER_CSR = "csr";
 const char *const LLDB_NT_OWNER_ANDROID = "Android";
 const char *const LLDB_NT_OWNER_CORE = "CORE";
 const char *const LLDB_NT_OWNER_LINUX = "LINUX";
 
 // ELF note type definitions
 const elf_word LLDB_NT_FREEBSD_ABI_TAG = 0x01;
 const elf_word LLDB_NT_FREEBSD_ABI_SIZE = 4;
 
 const elf_word LLDB_NT_GNU_ABI_TAG = 0x01;
 const elf_word LLDB_NT_GNU_ABI_SIZE = 16;
 
 const elf_word LLDB_NT_GNU_BUILD_ID_TAG = 0x03;
 
-const elf_word LLDB_NT_NETBSD_ABI_TAG = 0x01;
-const elf_word LLDB_NT_NETBSD_ABI_SIZE = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG = 1;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ = 4;
+const elf_word LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ = 7;
+const elf_word LLDB_NT_NETBSD_NT_PROCINFO = 1;
 
 // GNU ABI note OS constants
 const elf_word LLDB_NT_GNU_ABI_OS_LINUX = 0x00;
 const elf_word LLDB_NT_GNU_ABI_OS_HURD = 0x01;
 const elf_word LLDB_NT_GNU_ABI_OS_SOLARIS = 0x02;
 
 // LLDB_NT_OWNER_CORE and LLDB_NT_OWNER_LINUX note contants
 #define NT_PRSTATUS 1
else if (note.n_name == LLDB_NT_OWNER_GNU) {
         }
         break;
       }
       if (arch_spec.IsMIPS() &&
           arch_spec.GetTriple().getOS() == llvm::Triple::OSType::UnknownOS)
         // The note.n_name == LLDB_NT_OWNER_GNU is valid for Linux platform
         arch_spec.GetTriple().setOS(llvm::Triple::OSType::Linux);
     }
-    // Process NetBSD ELF notes.
+    // Process NetBSD ELF executables and shared libraries
     else if ((note.n_name == LLDB_NT_OWNER_NETBSD) &&
-             (note.n_type == LLDB_NT_NETBSD_ABI_TAG) &&
-             (note.n_descsz == LLDB_NT_NETBSD_ABI_SIZE)) {
-      // Pull out the min version info.
+             (note.n_type == LLDB_NT_NETBSD_NT_NETBSD_IDENT_TAG) &&
+             (note.n_descsz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_DESCSZ) &&
+             (note.n_namesz == LLDB_NT_NETBSD_NT_NETBSD_IDENT_NAMESZ)) {
+      // Pull out the version info.
       uint32_t version_info;
       if (data.GetU32(&offset, &version_info, 1) == nullptr) {
         error.SetErrorString("failed to read NetBSD ABI note payload");
         return error;
       }
-
+      // Convert the version info into a major/minor/patch number.
+      //     #define __NetBSD_Version__ MMmmrrpp00
+      //
+      //     M = major version
+      //     m = minor version; a minor number of 99 indicates current.
+      //     r = 0 (since NetBSD 3.0 not used)
+      //     p = patchlevel
+      const uint32_t version_major = version_info / 100000000;
+      const uint32_t version_minor = (version_info % 100000000) / 1000000;
+      const uint32_t version_patch = (version_info % 10000) / 100;
+      // Set the elf OS version to NetBSD.  Also clear the vendor.
+      arch_spec.GetTriple().setOSName(
+          llvm::formatv("netbsd{0}.{1}.{2}", version_major, version_minor,
+                        version_patch).str());
+      arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
+    }
+    // Process NetBSD ELF core(5) notes
+    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.
       arch_spec.GetTriple().setOS(llvm::Triple::OSType::NetBSD);
       arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
-
-      if (log)
-        log->Printf(
-            "ObjectFileELF::%s detected NetBSD, min version constant %" PRIu32,
-            __FUNCTION__, version_info);
     }
     // Process OpenBSD ELF notes.
     else if (note.n_name == LLDB_NT_OWNER_OPENBSD) {
       // Set the elf OS version to OpenBSD.  Also clear the vendor.
       arch_spec.GetTriple().setOS(llvm::Triple::OSType::OpenBSD);
       arch_spec.GetTriple().setVendor(llvm::Triple::VendorType::UnknownVendor);
     }
     // Process CSR kalimba notes