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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
