Author: Martin Storsjö Date: 2022-06-22T17:16:05+03:00 New Revision: 8a64dd5b06146f073a4a326f0e24fa18e571b281
URL: https://github.com/llvm/llvm-project/commit/8a64dd5b06146f073a4a326f0e24fa18e571b281 DIFF: https://github.com/llvm/llvm-project/commit/8a64dd5b06146f073a4a326f0e24fa18e571b281.diff LOG: [lldb] Fix reading i686-windows executables with GNU environment 25c8a061c5739677d2fc0af29a8cc9520207b923 / D127048 added an option for setting the ABI to GNU. When an object file is loaded, there's only minimal verification done for the architecture spec set for it, if the object file only provides one. However, for i386 object files, the PECOFF object file plugin provides two architectures, i386-pc-windows and i686-pc-windows. This picks a totally different codepath in TargetList::CreateTargetInternal, where it's treated as a fat binary. This goes through more verifications to see if the architectures provided by the object file matches what the platform plugin supports. The PlatformWindows() constructor explicitly adds the "i386-pc-windows" and "i686-pc-windows" architectures (even when running on other architectures), which allows this "fat binary verification" to succeed for the i386 object files that provide two architectures. However, after that commit, if the object file is advertised with the different environment (either when lldb is built in a mingw environment, or if that setting is set), the fat binary validation won't accept the file any longer. Update ArchSpec::IsEqualTo with more logic for the Windows use cases; mismatching vendors is not an issue (they don't have any practical effect on Windows), and GNU and MSVC environments are compatible to the point that PlatformWindows can handle object files for both environments/ABIs. As a separate path forward, one could also consider to stop returning two architecture specs from ObjectFilePECOFF::GetModuleSpecifications for i386 files. Differential Revision: https://reviews.llvm.org/D128268 Added: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml Modified: lldb/source/Utility/ArchSpec.cpp Removed: ################################################################################ diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp index c6feacfc7c1e6..a99aed82bc886 100644 --- a/lldb/source/Utility/ArchSpec.cpp +++ b/lldb/source/Utility/ArchSpec.cpp @@ -979,7 +979,16 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool exact_match) const { const llvm::Triple::VendorType lhs_triple_vendor = lhs_triple.getVendor(); const llvm::Triple::VendorType rhs_triple_vendor = rhs_triple.getVendor(); - if (lhs_triple_vendor != rhs_triple_vendor) { + + const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS(); + const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS(); + + bool both_windows = lhs_triple.isOSWindows() && rhs_triple.isOSWindows(); + + // On Windows, the vendor field doesn't have any practical effect, but + // it is often set to either "pc" or "w64". + if ((lhs_triple_vendor != rhs_triple_vendor) && + (exact_match || !both_windows)) { const bool rhs_vendor_specified = rhs.TripleVendorWasSpecified(); const bool lhs_vendor_specified = TripleVendorWasSpecified(); // Both architectures had the vendor specified, so if they aren't equal @@ -993,8 +1002,6 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool exact_match) const { return false; } - const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS(); - const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS(); const llvm::Triple::EnvironmentType lhs_triple_env = lhs_triple.getEnvironment(); const llvm::Triple::EnvironmentType rhs_triple_env = @@ -1032,6 +1039,9 @@ bool ArchSpec::IsEqualTo(const ArchSpec &rhs, bool exact_match) const { return true; } + if (!exact_match && both_windows) + return true; // The Windows environments (MSVC vs GNU) are compatible + return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env); } diff --git a/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml b/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml new file mode 100644 index 0000000000000..ca2bac38027fa --- /dev/null +++ b/lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml @@ -0,0 +1,61 @@ +## Check that i386 executables can be opened in both ABI modes. +## I386 executables are special in the sense that the PECOFF object +## file plugin returns two architectures for them, i386 and i686, which +## causes more elaborate comparisons to be run against the Platform plugin's +## architecture list. Check that this is accepted despite potential mismatches +## in the environment part of the triple. + +# RUN: yaml2obj %s -o %t + +## Default ABI is msvc: +# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi msvc" \ +# RUN: -f %t -o "image list --triple --basename" -o exit | \ +# RUN: FileCheck -DABI=msvc -DFILENAME=%basename_t.tmp %s + +## Default ABI is gnu: +# RUN: %lldb -O "settings set plugin.object-file.pe-coff.abi gnu" \ +# RUN: -f %t -o "image list --triple --basename" -o exit | \ +# RUN: FileCheck -DABI=gnu -DFILENAME=%basename_t.tmp %s + +# CHECK-LABEL: image list --triple --basename +# CHECK-NEXT: i686-pc-windows-[[ABI]] [[FILENAME]] + +--- !COFF +OptionalHeader: + AddressOfEntryPoint: 4480 + ImageBase: 268435456 + SectionAlignment: 4096 + FileAlignment: 512 + MajorOperatingSystemVersion: 6 + MinorOperatingSystemVersion: 0 + MajorImageVersion: 0 + MinorImageVersion: 0 + MajorSubsystemVersion: 6 + MinorSubsystemVersion: 0 + Subsystem: IMAGE_SUBSYSTEM_WINDOWS_CUI + DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ] + SizeOfStackReserve: 1048576 + SizeOfStackCommit: 4096 + SizeOfHeapReserve: 1048576 + SizeOfHeapCommit: 4096 +header: + Machine: IMAGE_FILE_MACHINE_I386 + Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ] +sections: + - Name: .text + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + VirtualAddress: 4096 + VirtualSize: 64 + SectionData: DEADBEEFBAADF00D + - Name: .data + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] + VirtualAddress: 8192 + VirtualSize: 64 + SectionData: DEADBEEFBAADF00D + - Name: .debug_info + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] + VirtualAddress: 16384 + VirtualSize: 64 + SectionData: DEADBEEFBAADF00D +symbols: [] +... _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
