[COFF] Error on unexpected .pdata size

[hans]

Summary

Previously, lld would crash if the .pdata size is not an even multiple of the expected .pdata entry size. This makes it error instead.

(We hit this in Chromium due to an assembler problem: https://crbug.com/1101577)

[thakis]

Nice!

[MaskRay]

lld/COFF/Writer.cpp

1868

Nit: Twine(end-begin)

... to avoid a temporary std::string.

lld/test/COFF/pdata-arm64-bad.yaml

1

yaml2obj %s -o %t.obj

Drop the # on the empty line.

[MaskRay]

lld/test/COFF/pdata-arm64-bad.yaml

5

The yaml can be further simplified.

[hans]

lld/COFF/Writer.cpp

1868

Thanks! That's nicer.

lld/test/COFF/pdata-arm64-bad.yaml

1

Actually, I'll drop the empty line :)

5

It could be simplified a lot and still hit the error case here, but to ensure that the rest of the file makes sense, I'd prefer to keep it as it is: just a small modification to the known good pdata-arm64.yaml file.

lld/COFF/Writer.cpp

auto bufAddr = [&](Chunk *c) {
     OutputSection *os = c->getOutputSection();
     return buffer->getBufferStart() + os->getFileOff() + c->getRVA() -
            os->getRVA();
   };
   uint8_t *begin = bufAddr(firstPdata);
   uint8_t *end = bufAddr(lastPdata) + lastPdata->getSize();
   if (config->machine == AMD64) {
     struct Entry { ulittle32_t begin, end, unwind; };
+    if ((end - begin) % sizeof(Entry) != 0) {
+      fatal("unexpected .pdata size: " + std::to_string(end - begin) +
+            " is not a multiple of " + std::to_string(sizeof(Entry)));
+    }
     parallelSort(
         MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
         [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
     return;
   }
   if (config->machine == ARMNT || config->machine == ARM64) {
     struct Entry { ulittle32_t begin, unwind; };
+    if ((end - begin) % sizeof(Entry) != 0) {
+      fatal("unexpected .pdata size: " + std::to_string(end - begin) +
+            " is not a multiple of " + std::to_string(sizeof(Entry)));
+    }
     parallelSort(
         MutableArrayRef<Entry>((Entry *)begin, (Entry *)end),
         [](const Entry &a, const Entry &b) { return a.begin < b.begin; });
     return;
   }
   lld::errs() << "warning: don't know how to handle .pdata.\n";
 }
 

lld/test/COFF/pdata-arm64-bad.yaml

+# RUN: yaml2obj < %s > %t.obj
+#
+# RUN: not lld-link /out:%t.exe /entry:func1 /subsystem:console %t.obj 2>&1 | FileCheck %s
+
+# This file is like pdata-arm64.yaml, except that .pdata has been extended with
+# 4 bytes. This can happen due to for example bad assembler input. Check that
+# lld errors gracefully instead of crashing.
+
+# CHECK: unexpected .pdata size: 20 is not a multiple of 8
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_ARM64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     ff4300d1f37b00a9f303012a00000094e003132a00000094f37b40a9ff430091c0035fd6f353bea9fe0b00f9f303012af403022a00000094e003132a00000094e003142a00000094fe0b40f9f353c2a8c0035fd6c0035fd6
+    Relocations:
+      - VirtualAddress:  12
+        SymbolName:      func3
+        Type:            IMAGE_REL_ARM64_BRANCH26
+      - VirtualAddress:  20
+        SymbolName:      func3
+        Type:            IMAGE_REL_ARM64_BRANCH26
+      - VirtualAddress:  52
+        SymbolName:      func3
+        Type:            IMAGE_REL_ARM64_BRANCH26
+      - VirtualAddress:  60
+        SymbolName:      func3
+        Type:            IMAGE_REL_ARM64_BRANCH26
+      - VirtualAddress:  68
+        SymbolName:      func3
+        Type:            IMAGE_REL_ARM64_BRANCH26
+  - Name:            .pdata
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    SectionData:     0000000031002201000000002500a10000000000
+    Relocations:
+      - VirtualAddress:  0
+        SymbolName:      func2
+        Type:            IMAGE_REL_ARM64_ADDR32NB
+      - VirtualAddress:  8
+        SymbolName:      func1
+        Type:            IMAGE_REL_ARM64_ADDR32NB
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          57
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          1
+  - Name:            .pdata
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          20
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          2
+  - Name:            func1
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func2
+    Value:           36
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func3
+    Value:           84
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...