[Lldb-commits] [lldb] [llvm] [lldb] Fix object format of some mach-o files by using vendor info in `getDefaultFormat()` (PR #143633)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: None (royitaqi)

<details>
<summary>Changes</summary>

**Context**: See previous attempt #<!-- -->142704

**TL;DR**: Some mach-o files (including some of those in lldb tests) are mistakenly reported as `ELF` (as in their `Triple`) by `ObjectFileMachO`. The reason is that those files don't have load commands like `LC_BUILD_VERSION` or `LC_VERSION_MIN_*` (which would cause the `Triple` code to set the correct `MachO` object format). Previously, #<!-- -->142704 tries to fix this by setting the correct object format `ObjectFileMachO::GetAllArchSpecs`. However, it seems such explicit assignment cause the `Triple` to report "-macho" in its string form, causing change in UI and a test failure which depends on the UI.

So we are dealing with the following constraints:
1. The UI cannot be changed. IIUC, this means the fix cannot set the object format explicitly. I.e. the fix has to be in `Triple`'s `getDefaultFormat()`, changing the returned default format based on the `Triple`'s other fields.
2. In `getDefaultFormat()`, at least in the case that I tested (see added unit test), the `OS` is not set.
3. The `Vendor` is set to `Apple. This is because `ObjectFileMachO::GetAllArchSpecs()` calls `ArchSpec::SetArchitecture()` ([code](https://github.com/llvm/llvm-project/blob/1bf4702d2bbaad522886dfbab913a8dd6efe3b85/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp#L5059)), which sets the vendor ([code](https://github.com/llvm/llvm-project/blob/1bf4702d2bbaad522886dfbab913a8dd6efe3b85/lldb/source/Utility/ArchSpec.cpp#L892)).
4. Since the above code will always set the arch type to `eArchTypeMachO`, which will always lead to the vendor to be set as `Apple`, it seems to be a consistent way to detect mach-o object format.

---
Full diff: https://github.com/llvm/llvm-project/pull/143633.diff


2 Files Affected:

- (modified) lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp (+55) 
- (modified) llvm/lib/TargetParser/Triple.cpp (+3) 


``````````diff
diff --git a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
index 0ef2d0b85fd36..71ff866abb352 100644
--- a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
+++ b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp
@@ -94,4 +94,59 @@ TEST_F(ObjectFileMachOTest, IndirectSymbolsInTheSharedCache) {
   for (size_t i = 0; i < 10; i++)
     OF->ParseSymtab(symtab);
 }
+
+TEST_F(ObjectFileMachOTest, ObjectFormatWithoutVersionLoadCommand) {
+  // A Mach-O file without the load command LC_BUILD_VERSION.
+  const char *yamldata = R"(
+--- !mach-o
+FileHeader:
+  magic:           0xFEEDFACF
+  cputype:         0x0100000C
+  cpusubtype:      0x00000000
+  filetype:        0x00000001
+  ncmds:           1
+  sizeofcmds:      152
+  flags:           0x00002000
+  reserved:        0x00000000
+LoadCommands:
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         152
+    segname:         __TEXT
+    vmaddr:          0
+    vmsize:          4
+    fileoff:         184
+    filesize:        4
+    maxprot:         7
+    initprot:        7
+    nsects:          1
+    flags:           0
+    Sections:
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000000000000
+        content:         'AABBCCDD'
+        size:            4
+        offset:          184
+        align:           0
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+...
+)";
+
+  // Perform setup.
+  llvm::Expected<TestFile> file = TestFile::fromYaml(yamldata);
+  EXPECT_THAT_EXPECTED(file, llvm::Succeeded());
+  auto module_sp = std::make_shared<Module>(file->moduleSpec());
+  ASSERT_NE(module_sp, nullptr);
+  auto object_file = module_sp->GetObjectFile();
+  ASSERT_NE(object_file, nullptr);
+
+  // Verify that the object file is recognized as Mach-O.
+  ASSERT_EQ(object_file->GetArchitecture().GetTriple().getObjectFormat(),
+            llvm::Triple::MachO);
+}
 #endif
diff --git a/llvm/lib/TargetParser/Triple.cpp b/llvm/lib/TargetParser/Triple.cpp
index 6a559ff023caa..19ccd44322314 100644
--- a/llvm/lib/TargetParser/Triple.cpp
+++ b/llvm/lib/TargetParser/Triple.cpp
@@ -933,6 +933,9 @@ static Triple::ObjectFormatType getDefaultFormat(const Triple &T) {
     case Triple::Win32:
     case Triple::UEFI:
       return Triple::COFF;
+    case Triple::UnknownOS:
+      return T.getVendor() == Triple::Apple ? Triple::MachO : Triple::ELF;
+      // Intentional leak into the default case for additional logic.
     default:
       return T.isOSDarwin() ? Triple::MachO : Triple::ELF;
     }

``````````

</details>


https://github.com/llvm/llvm-project/pull/143633