[Object] Recognize ARM64EC binaries in COFFObjectFile::getMachine.

[jacek]

Summary

ARM64EC/ARM64X binaries use ARM64 or AMD64 machine types, but provide additional CHPE metadata that may be used to distinguish them from pure ARM64/AMD64 binaries.

[efriedma]

Is it possible to write tests using yaml2obj instead of lld-link?

[jhenderson]

In D149091#4293527, @efriedma wrote:

Is it possible to write tests using yaml2obj instead of lld-link?

+1 to this if at all possible: the code under test isn't LLD specific (I assume, since it's in the LLVM header area), so sticking the testing in lld seems wrong. It might require some extra work in yaml2obj to get it to support new things, but I don't think that's a bad thing.

lld/test/COFF/arm64ec.test

43 ↗

(On Diff #516509)

Nit: delete extra blank line.

[jacek]

Thanks for reviews. The new version uses yaml2obj instead of lld-link. Previous tests will be useful for testing lld-link itself, but I will send it later, probably together with code range generation support.

yaml-based test is a bit less similar to real-world binaries. Linker should align code boundaries to page size, but doing that with yaml SectionData makes it hard to follow. I just used much smaller alignment, it shouldn't matter for disassembler anyway.

CHPE and load config are quite hard to read. I guess we could have some helper syntax for that in yaml2obj, but it's not clear to me how it should look like. Unlike other things like PE headers, load config and CHPE metadata are part of .data/.rdata sections (so it would need to alter SectionData content) and we'd need some way of referencing it (unless we'd still hard-code RVAs). I will investigate if further if you prefer.

[efriedma]

I guess we could have some helper syntax for that in yaml2obj, but it's not clear to me how it should look like.

Someone can always use llvm-readobj or dumpbin to help understand the fine details if necessary, so it's not a big deal if it's only a few tests (particularly if they have comments explaining the tricky bits).

We could invent a new kind of "SectionData" that allows structured data, instead of just a string, I guess. We don't want to get too fancy here; even just representing the string as an array of 32-bit integers would be sufficient to annotate various bits using comments.

Adding a mechanism for relocations to represent RVAs seems overkill. yaml2obj is intentionally kept simple.

[mstorsjo]

Just a sidenote; I think @jacek should go get himself commit access at this point.

[jacek]

The new version uses yaml2obj StructuredData from D149439 and D149440.

[jhenderson]

The new yaml2obj-based test certainly looks reasonable to me. However, it looks like there are three code paths now, and the test only tests one of them. The case where CHPEMetadata is false/nullptr/whatever is possibly covered by other tests(?), but I think you also need to test the other part of the switch you've added? (I don't know anything really about the code area to give any particular guidance about it though).

[jacek]

Yes, existing tests should cover missing CHPE metadata. I added a test for ARM64X.

[jhenderson]

llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml

3

Nits:

1. Prefer double-dash options for FileCheck (--check-prefix), for consistency with the llvm-readobj command you've added.
2. DISASM is a more common abbreviation than DISAS, so I suggest using that.

29

Rather than having two identical(?)-barring-one-field input YAMLs between the two tests, can you use the yaml2obj -D feature to enable sharing of the two? I don't know if that feature is specific to the ELF side or not, but if it is usable, you could have both tests in the same file, and just create two input files from the same input block, which differ only by the Machine type. This would also allow you to minimise the duplication in the FileCheck CHECK patterns.

A good example of what I mean can be seen in this test: https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/file-header-machine-types.test.

[jacek]

Rebased, using a single file and DISASM and double-dash for disassembler output checks.

[jhenderson]

LGTM, with nits.

llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml

1

Nit: newer tools tests use ## for comments, to distinguish them from lit and FileCheck directives.

5

I would consider reordering your CHECK/RUN lines for a bit of improved readability/comparability, since you're partly sharing them (I also added some extra whitespace to make things nicely line up):

# RUN: yaml2obj %s -o %t -DMACHINE=IMAGE_FILE_MACHINE_AMD64
# RUN: llvm-objdump -d %t | FileCheck --check-prefixes=DISASM,ARM64EC %s
# RUN: llvm-readobj --coff-load-config %t | FileCheck --check-prefix=CODEMAP %s

# Check that ARM64 image file with CHPE data is recognized as ARM64X.
# RUN: yaml2obj %s -o %t -DMACHINE=IMAGE_FILE_MACHINE_ARM64
# RUN: llvm-objdump -d %t | FileCheck --check-prefixes=DISASM,ARM64X %s
# RUN: llvm-readobj --coff-load-config %t | FileCheck --check-prefix=CODEMAP %s

# ARM64EC: file format coff-arm64ec
# ARM64X:  file format coff-arm64x

# DISASM: ...

[jhenderson]

NB: It wouldn't hurt to get an approval from someone more familiar with COFF/ARM64EC etc.

[jacek]

Reordered and using '##'. Thanks for reviews.

[mstorsjo]

LGTM. And I didn't know about the yaml2obj option -D, that's nice!

[jhenderson]

LGTM again.

[antmo]

Hi, I think this broke lldb-aarch64-windows : https://lab.llvm.org/buildbot/#/builders/219/builds/4366
Could you please take a look at it?

[jacek]

In D149091#4532352, @antmo wrote:

Hi, I think this broke lldb-aarch64-windows : https://lab.llvm.org/buildbot/#/builders/219/builds/4366
Could you please take a look at it?

I think I can see what's going on, sorry about that. Now that COFFObjectFile recognizes ARM64X, it reports that for system DLLs and lldb doesn't know what to do about them. I think that something like the attached patch should help, but I will need a bit to verify.

patch.diff1 KBDownload

[jacek]

I created D156268 with a fix for lldb.

llvm/include/llvm/Object/COFF.h

uintptr_t getSymbolTable() const {
     if (SymbolTable16)
       return reinterpret_cast<uintptr_t>(SymbolTable16);
     if (SymbolTable32)
       return reinterpret_cast<uintptr_t>(SymbolTable32);
     return uintptr_t(0);
   }
 
   uint16_t getMachine() const {
-    if (COFFHeader)
+    if (COFFHeader) {
+      if (CHPEMetadata) {
+        switch (COFFHeader->Machine) {
+        case COFF::IMAGE_FILE_MACHINE_AMD64:
+          return COFF::IMAGE_FILE_MACHINE_ARM64EC;
+        case COFF::IMAGE_FILE_MACHINE_ARM64:
+          return COFF::IMAGE_FILE_MACHINE_ARM64X;
+        }
+      }
       return COFFHeader->Machine;
+    }
     if (COFFBigObjHeader)
       return COFFBigObjHeader->Machine;
     llvm_unreachable("no COFF header!");
   }
 
   uint16_t getSizeOfOptionalHeader() const {
     if (COFFHeader)
       return COFFHeader->isImportLibrary() ? 0

llvm/test/tools/llvm-objdump/COFF/arm64ec.yaml

+# Check that AMD64 image file with CHPE data is recognized as ARM64EC.
+# RUN: yaml2obj %s -o %t -DMACHINE=IMAGE_FILE_MACHINE_AMD64
+# RUN: llvm-objdump -d %t | FileCheck --check-prefixes=DISASM,ARM64EC %s
+# RUN: llvm-readobj --coff-load-config %t | FileCheck --check-prefix=CODEMAP %s
+# ARM64EC: file format coff-arm64ec
+
+# Check that ARM64 image file with CHPE data is recognized as ARM64X.
+# RUN: yaml2obj %s -o %t -DMACHINE=IMAGE_FILE_MACHINE_ARM64
+# RUN: llvm-objdump -d %t | FileCheck --check-prefixes=DISASM,ARM64X %s
+# RUN: llvm-readobj --coff-load-config %t | FileCheck --check-prefix=CODEMAP %s
+# ARM64X: file format coff-arm64x
+
+# DISASM:      180001000: 52800040     mov     w0, #0x2
+# DISASM-NEXT: 180001004: d65f03c0     ret
+# DISASM-NEXT: ...
+# DISASM:      180002020: 528000a0     mov     w0, #0x5
+# DISASM-NEXT: 180002024: d65f03c0     ret
+
+# CODEMAP:      CodeMap [
+# CODEMAP-NEXT:   0x1000 - 0x1008  ARM64EC
+# CODEMAP-NEXT:   0x1020 - 0x2007  X64
+# CODEMAP-NEXT:   0x2020 - 0x2028  ARM64EC
+# CODEMAP-NEXT: ]
+
+--- !COFF
+OptionalHeader:
+  ImageBase:        0x180000000
+  SectionAlignment: 4096
+  FileAlignment:    512
+  DLLCharacteristics: [ ]
+  LoadConfigTable:
+    RelativeVirtualAddress: 0x3000
+    Size:                   320
+header:
+  Machine:         [[MACHINE]]
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE, IMAGE_FILE_DLL ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x1000
+    VirtualSize:     38
+    SectionData:     40008052C0035FD6000000000000000000000000000000000000000000000000B803000000C3
+  - Name:            .test
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x2000
+    VirtualSize:     40
+    SectionData:     B806000000C3CC00000000000000000000000000000000000000000000000000A0008052C0035FD6
+  - Name:            .rdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  0x3000
+    VirtualSize:     328
+    StructuredData:
+      - LoadConfig:
+          CHPEMetadataPointer: 0x180004000
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    VirtualAddress:  0x4000
+    VirtualSize:     112
+    StructuredData:
+      - UInt32: 1       # Version
+      - UInt32: 0x4050  # CodeMap
+      - UInt32: 3       # CodeMapCount
+      - UInt32: 0       # CodeRangesToEntryPoints
+      - UInt32: 0       # RedirectionMetadata
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0       # CodeRangesToEntryPointsCount
+      - UInt32: 0       # RedirectionMetadataCount
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0
+      - UInt32: 0x1001  # CodeMap[0]
+      - UInt32: 0x8
+      - UInt32: 0x1022  # CodeMap[1]
+      - UInt32: 0x0fe7
+      - UInt32: 0x2021  # CodeMap[2]
+      - UInt32: 0x8
+symbols:         []
+...