[llvm-readobj][Object][COFF] Include COFF import file machine type in format string. #78366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cjacek
requested review from
compnerd,
mstorsjo,
MaskRay,
jh7370 and
efriedma-quic
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Jacek Caban (cjacek) ChangesWe currently don't print import library machine at all. It would would be useful for ARM64EC importlib llvm-lib and llvm-dlltool tests. ARM64EC should use a mixture of ARM64 and ARM64EC files (both for ARM64X and pure ARM64EC). This PR makes COFFImportFile similar to COFFObjectFile in that regard. Full diff: https://github.com/llvm/llvm-project/pull/78366.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h
index 0fb65fabdbcad5..edc836ff0348cb 100644
--- a/llvm/include/llvm/Object/COFFImportFile.h
+++ b/llvm/include/llvm/Object/COFFImportFile.h
@@ -65,6 +65,8 @@ class COFFImportFile : public SymbolicFile {
uint16_t getMachine() const { return getCOFFImportHeader()->Machine; }
+ StringRef getFileFormatName() const;
+
private:
bool isData() const {
return getCOFFImportHeader()->getType() == COFF::IMPORT_DATA;
diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp
index eeb13ffe9c11b0..2cb81b81b143ea 100644
--- a/llvm/lib/Object/COFFImportFile.cpp
+++ b/llvm/lib/Object/COFFImportFile.cpp
@@ -33,6 +33,25 @@ using namespace llvm;
namespace llvm {
namespace object {
+StringRef COFFImportFile::getFileFormatName() const {
+ switch(getMachine()) {
+ case COFF::IMAGE_FILE_MACHINE_I386:
+ return "COFF-import-file-i386";
+ case COFF::IMAGE_FILE_MACHINE_AMD64:
+ return "COFF-import-file-x86-64";
+ case COFF::IMAGE_FILE_MACHINE_ARMNT:
+ return "COFF-import-file-ARM";
+ case COFF::IMAGE_FILE_MACHINE_ARM64:
+ return "COFF-import-file-ARM64";
+ case COFF::IMAGE_FILE_MACHINE_ARM64EC:
+ return "COFF-import-file-ARM64EC";
+ case COFF::IMAGE_FILE_MACHINE_ARM64X:
+ return "COFF-import-file-ARM64X";
+ default:
+ return "COFF-import-file-<unknown arch>";
+ }
+}
+
static uint16_t getImgRelRelocation(MachineTypes Machine) {
switch (Machine) {
default:
diff --git a/llvm/test/tools/llvm-dlltool/coff-exports.def b/llvm/test/tools/llvm-dlltool/coff-exports.def
index 91a7e27ea4720f..1a086d0998865a 100644
--- a/llvm/test/tools/llvm-dlltool/coff-exports.def
+++ b/llvm/test/tools/llvm-dlltool/coff-exports.def
@@ -1,8 +1,8 @@
; RUN: llvm-dlltool -m i386:x86-64 --input-def %s --output-lib %t.a
-; RUN: llvm-readobj %t.a | FileCheck %s
+; RUN: llvm-readobj %t.a | FileCheck %s -check-prefixes=CHECK,CHECK-X64
; RUN: llvm-nm --print-armap %t.a | FileCheck --check-prefix=SYMTAB %s
; RUN: llvm-dlltool -m arm64 --input-def %s --output-lib %t.a
-; RUN: llvm-readobj %t.a | FileCheck %s
+; RUN: llvm-readobj %t.a | FileCheck %s -check-prefixes=CHECK,CHECK-ARM64
; RUN: llvm-nm --print-armap %t.a | FileCheck --check-prefix=SYMTAB %s
LIBRARY test.dll
@@ -13,7 +13,8 @@ TestFunction3 ; This is a comment
AnotherFunction
; CHECK: File: test.dll
-; CHECK: Format: COFF-import-file
+; CHECK-X64: Format: COFF-import-file-x86-64
+; CHECK-ARM64: Format: COFF-import-file-ARM64
; CHECK: Type: code
; CHECK: Name type: name
; CHECK-NEXT: Symbol: __imp_TestFunction1
diff --git a/llvm/test/tools/llvm-lib/arm64ec-implib.test b/llvm/test/tools/llvm-lib/arm64ec-implib.test
index ee8b134d06b166..3c74b4bf660765 100644
--- a/llvm/test/tools/llvm-lib/arm64ec-implib.test
+++ b/llvm/test/tools/llvm-lib/arm64ec-implib.test
@@ -26,19 +26,19 @@ READOBJ-NEXT: Arch: aarch64
READOBJ-NEXT: AddressSize: 64bit
READOBJ-EMPTY:
READOBJ-NEXT: File: test.lib(test.dll)
-READOBJ-NEXT: Format: COFF-ARM64
+READOBJ-NEXT: Format: COFF-ARM64EC
READOBJ-NEXT: Arch: aarch64
READOBJ-NEXT: AddressSize: 64bit
READOBJ-EMPTY:
READOBJ-NEXT: File: test.dll
-READOBJ-NEXT: Format: COFF-import-file
+READOBJ-NEXT: Format: COFF-import-file-ARM64EC
READOBJ-NEXT: Type: code
READOBJ-NEXT: Name type: name
READOBJ-NEXT: Symbol: __imp_funcexp
READOBJ-NEXT: Symbol: funcexp
READOBJ-EMPTY:
READOBJ-NEXT: File: test.dll
-READOBJ-NEXT: Format: COFF-import-file
+READOBJ-NEXT: Format: COFF-import-file-ARM64EC
READOBJ-NEXT: Type: data
READOBJ-NEXT: Name type: name
READOBJ-NEXT: Symbol: __imp_dataexp
diff --git a/llvm/test/tools/llvm-readobj/COFF/exports-implib.test b/llvm/test/tools/llvm-readobj/COFF/exports-implib.test
index 75d0809ecbb2d0..ee567b2977a368 100644
--- a/llvm/test/tools/llvm-readobj/COFF/exports-implib.test
+++ b/llvm/test/tools/llvm-readobj/COFF/exports-implib.test
@@ -1,25 +1,25 @@
RUN: llvm-readobj --coff-exports %p/Inputs/library.lib | FileCheck %s
CHECK: File: library.dll
-CHECK: Format: COFF-import-file
+CHECK: Format: COFF-import-file-i386
CHECK: Type: const
CHECK: Name type: undecorate
CHECK: Symbol: __imp__constant
CHECK: File: library.dll
-CHECK: Format: COFF-import-file
+CHECK: Format: COFF-import-file-i386
CHECK: Type: data
CHECK: Name type: noprefix
CHECK: Symbol: __imp__data
CHECK: File: library.dll
-CHECK: Format: COFF-import-file
+CHECK: Format: COFF-import-file-i386
CHECK: Type: code
CHECK: Name type: name
CHECK: Symbol: __imp__function
CHECK: File: library.dll
-CHECK: Format: COFF-import-file
+CHECK: Format: COFF-import-file-i386
CHECK: Type: code
CHECK: Name type: ordinal
CHECK: Symbol: __imp__ordinal
diff --git a/llvm/test/tools/llvm-readobj/COFF/file-headers.test b/llvm/test/tools/llvm-readobj/COFF/file-headers.test
index 6e9ca67c2a6545..b83a6cf5b972b3 100644
--- a/llvm/test/tools/llvm-readobj/COFF/file-headers.test
+++ b/llvm/test/tools/llvm-readobj/COFF/file-headers.test
@@ -320,7 +320,7 @@ symbols:
# RUN: llvm-readobj -h %p/Inputs/magic.coff-importlib \
# RUN: | FileCheck %s --strict-whitespace --match-full-lines --check-prefix IMPORTLIB
-# IMPORTLIB:Format: COFF-import-file
+# IMPORTLIB:Format: COFF-import-file-i386
# IMPORTLIB-NEXT:Type: code
# IMPORTLIB-NEXT:Name type: noprefix
# IMPORTLIB-NEXT:Symbol: __imp__func
diff --git a/llvm/tools/llvm-readobj/COFFImportDumper.cpp b/llvm/tools/llvm-readobj/COFFImportDumper.cpp
index c9d5e82263db11..8aedc310ae3a9f 100644
--- a/llvm/tools/llvm-readobj/COFFImportDumper.cpp
+++ b/llvm/tools/llvm-readobj/COFFImportDumper.cpp
@@ -23,7 +23,7 @@ namespace llvm {
void dumpCOFFImportFile(const COFFImportFile *File, ScopedPrinter &Writer) {
Writer.startLine() << '\n';
Writer.printString("File", File->getFileName());
- Writer.printString("Format", "COFF-import-file");
+ Writer.printString("Format", File->getFileFormatName());
const coff_import_header *H = File->getCOFFImportHeader();
switch (H->getType()) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
mstorsjo
approved these changes
Jan 17, 2024
There was a problem hiding this comment.
The change LGTM, thanks (I've also noticed that this is missing, and/or been led to believe that these object files really are architectureless). When inspecting most import libraries, the header/trailer object files still are regular object files, where one is able to see the intended architecture, but this definitely helps and makes things clearer and more testable.
There's a typo in the subject though, s/maching/machine/.
jh7370
approved these changes
Jan 17, 2024
| @@ -1,8 +1,8 @@ | |||
| ; RUN: llvm-dlltool -m i386:x86-64 --input-def %s --output-lib %t.a | |||
| ; RUN: llvm-readobj %t.a | FileCheck %s | |||
| ; RUN: llvm-readobj %t.a | FileCheck %s -check-prefixes=CHECK,CHECK-X64 | |||
There was a problem hiding this comment.
Nit: prefer double dashes, since that's what this test already uses.
|
Comments fixed in the new version. Thanks for reviews! |
cjacek
changed the title
ampandey-1995
pushed a commit
to ampandey-1995/llvm-project
that referenced
this pull request
Jan 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We currently don't print import library machine at all. It would would be useful for ARM64EC importlib llvm-lib and llvm-dlltool tests. ARM64EC should use a mixture of ARM64 and ARM64EC files (both for ARM64X and pure ARM64EC). This PR makes COFFImportFile similar to COFFObjectFile in that regard.