https://github.com/royitaqi created https://github.com/llvm/llvm-project/pull/143633
**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. >From d01f75f4b8f75676ae15730f0035cca685dc5195 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Tue, 3 Jun 2025 17:04:29 -0700 Subject: [PATCH 1/4] Set default object format. Add test. --- .../ObjectFile/Mach-O/ObjectFileMachO.cpp | 1 + .../ObjectFile/MachO/TestObjectFileMachO.cpp | 54 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 3950454b7c90e..0079672c5cbd0 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -5148,6 +5148,7 @@ void ObjectFileMachO::GetAllArchSpecs(const llvm::MachO::mach_header &header, llvm::Triple base_triple = base_arch.GetTriple(); base_triple.setOS(llvm::Triple::UnknownOS); base_triple.setOSName(llvm::StringRef()); + base_triple.setObjectFormat(llvm::Triple::MachO); if (header.filetype == MH_PRELOAD) { if (header.cputype == CPU_TYPE_ARM) { diff --git a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp index 0ef2d0b85fd36..33aaba5d1055f 100644 --- a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp +++ b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp @@ -94,4 +94,58 @@ TEST_F(ObjectFileMachOTest, IndirectSymbolsInTheSharedCache) { for (size_t i = 0; i < 10; i++) OF->ParseSymtab(symtab); } + +TEST_F(ObjectFileMachOTest, ObjectFileFormatWithoutLoadCommand) { + // 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 >From 119833ef21bb21e580280811fcf16afcf4be4bb2 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Tue, 3 Jun 2025 17:05:09 -0700 Subject: [PATCH 2/4] Fix format --- lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp index 33aaba5d1055f..aab3e996343b3 100644 --- a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp +++ b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp @@ -136,7 +136,7 @@ TEST_F(ObjectFileMachOTest, ObjectFileFormatWithoutLoadCommand) { reserved3: 0x00000000 ... )"; - + // Perform setup. llvm::Expected<TestFile> file = TestFile::fromYaml(yamldata); EXPECT_THAT_EXPECTED(file, llvm::Succeeded()); @@ -146,6 +146,7 @@ TEST_F(ObjectFileMachOTest, ObjectFileFormatWithoutLoadCommand) { 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); + ASSERT_EQ(object_file->GetArchitecture().GetTriple().getObjectFormat(), + llvm::Triple::MachO); } #endif >From 5bed1c47ee92c5a6e2cb85838ae3b0d4557d6e02 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Wed, 4 Jun 2025 15:41:26 -0700 Subject: [PATCH 3/4] Update unit test name --- lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp index aab3e996343b3..71ff866abb352 100644 --- a/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp +++ b/lldb/unittests/ObjectFile/MachO/TestObjectFileMachO.cpp @@ -95,7 +95,7 @@ TEST_F(ObjectFileMachOTest, IndirectSymbolsInTheSharedCache) { OF->ParseSymtab(symtab); } -TEST_F(ObjectFileMachOTest, ObjectFileFormatWithoutLoadCommand) { +TEST_F(ObjectFileMachOTest, ObjectFormatWithoutVersionLoadCommand) { // A Mach-O file without the load command LC_BUILD_VERSION. const char *yamldata = R"( --- !mach-o >From e303d3934fad076f433bd3bc18fc1816612a9ba8 Mon Sep 17 00:00:00 2001 From: Roy Shi <roy...@meta.com> Date: Tue, 10 Jun 2025 16:28:47 -0700 Subject: [PATCH 4/4] Different approach: use vendor in getDefaultFormat() --- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp | 1 - llvm/lib/TargetParser/Triple.cpp | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp index 0079672c5cbd0..3950454b7c90e 100644 --- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -5148,7 +5148,6 @@ void ObjectFileMachO::GetAllArchSpecs(const llvm::MachO::mach_header &header, llvm::Triple base_triple = base_arch.GetTriple(); base_triple.setOS(llvm::Triple::UnknownOS); base_triple.setOSName(llvm::StringRef()); - base_triple.setObjectFormat(llvm::Triple::MachO); if (header.filetype == MH_PRELOAD) { if (header.cputype == CPU_TYPE_ARM) { 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; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
