mstorsjo created this revision.
mstorsjo added reviewers: labath, DavidSpickett, omjavaid, alvinhochun.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.
25c8a061c5739677d2fc0af29a8cc9520207b923
<https://reviews.llvm.org/rG25c8a061c5739677d2fc0af29a8cc9520207b923> / D127048
<https://reviews.llvm.org/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 this commit, if the object file is advertised with
the different environment (either when 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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D128268
Files:
lldb/source/Utility/ArchSpec.cpp
lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
Index: lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
===================================================================
--- /dev/null
+++ lldb/test/Shell/ObjectFile/PECOFF/settings-abi-i686.yaml
@@ -0,0 +1,54 @@
+# 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: []
+...
Index: lldb/source/Utility/ArchSpec.cpp
===================================================================
--- lldb/source/Utility/ArchSpec.cpp
+++ lldb/source/Utility/ArchSpec.cpp
@@ -979,22 +979,29 @@
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 bool rhs_vendor_specified = rhs.TripleVendorWasSpecified();
- const bool lhs_vendor_specified = TripleVendorWasSpecified();
- // Both architectures had the vendor specified, so if they aren't equal
- // then we return false
- if (rhs_vendor_specified && lhs_vendor_specified)
- return false;
-
- // Only fail if both vendor types are not unknown
- if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
- rhs_triple_vendor != llvm::Triple::UnknownVendor)
- return false;
- }
const llvm::Triple::OSType lhs_triple_os = lhs_triple.getOS();
const llvm::Triple::OSType rhs_triple_os = rhs_triple.getOS();
+
+ if (lhs_triple_vendor != rhs_triple_vendor) {
+ // On Windows, the vendor field doesn't have any practical effect, but
+ // it is often set to either "pc" or "w64".
+ if (exact_match || lhs_triple_os != llvm::Triple::Win32 ||
+ rhs_triple_os != llvm::Triple::Win32) {
+ 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
+ // then we return false
+ if (rhs_vendor_specified && lhs_vendor_specified)
+ return false;
+
+ // Only fail if both vendor types are not unknown
+ if (lhs_triple_vendor != llvm::Triple::UnknownVendor &&
+ rhs_triple_vendor != llvm::Triple::UnknownVendor)
+ return false;
+ }
+ }
+
const llvm::Triple::EnvironmentType lhs_triple_env =
lhs_triple.getEnvironment();
const llvm::Triple::EnvironmentType rhs_triple_env =
@@ -1032,6 +1039,10 @@
return true;
}
+ if (!exact_match && lhs_triple_os == llvm::Triple::Win32 &&
+ rhs_triple_os == llvm::Triple::Win32)
+ return true; // The Windows environments (MSVC vs GNU) are compatible
+
return IsCompatibleEnvironment(lhs_triple_env, rhs_triple_env);
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
