[WebAssembly] Test that invalid symbol/relocation types generate errors

[sbc100]

Summary

See https://bugs.llvm.org/show_bug.cgi?id=48827

[dschuff]

IIUC this doesn't prevent the crash in lld, it just improves the error message when reading the object file?

[sbc100]

Yes, it turns out that the crash in LLD was a special case of llvm being aware of a new symbol type but lld not being testing with it.. in practice that should be rare. It was bad luck that it bit us I think,

[MaskRay]

In ELF, we usually use yaml2obj to generate invalid object files, instead of checking in precanned binaries. You can find lots of grimar's changes migrating away from binaries.

[sbc100]

In D95163#2514514, @MaskRay wrote:

In ELF, we usually use yaml2obj to generate invalid object files, instead of checking in precanned binaries. You can find lots of grimar's changes migrating away from binaries.

I generally prefer that method too, however I didn't think it was possible for this kind of case. How would I generate an invalid relocation type? Currently relocations are specified with a string:

For example here is the yaml I used as a starting point for this invalid binary:

--- !WASM
FileHeader:
  Version:         0x1
Sections:
  - Type:            TYPE
    Signatures:
      - Index:           0
        ParamTypes:      []
        ReturnTypes:
          - I32
  - Type:            FUNCTION
    FunctionTypes:   [ 0 ]
  - Type:            CODE
    Relocations:
      - Type:            R_WASM_TABLE_INDEX_SLEB
        Index:           0
        Offset:          0x4
    Functions:
      - Index:           0
        Locals:          []
        Body:            4181808080000B
  - Type:            CUSTOM
    Name:            linking
    Version:         2
    SymbolTable:
      - Index:           0
        Kind:            FUNCTION
        Name:            foo
        Flags:           [ VISIBILITY_HIDDEN ]
        Function:        0

In this case I modified the binary to change the R_WASM_TABLE_INDEX_SLEB value to 63 (an invalid value). If I try to do that in the yaml I get:

yaml2obj out.yaml > obj.o
YAML:15:26: error: unknown enumerated scalar
      - Type:            63
                         ^~
yaml2obj: error: failed to parse YAML input: Invalid argument

Also, I see a lot of ELF files (and very few yaml files) checked in directly to llvm/test/Object/Inputs/. Is that just because there is ongoing work to covert them?

[MaskRay]

In D95163#2515437, @sbc100 wrote:

In D95163#2514514, @MaskRay wrote:

In ELF, we usually use yaml2obj to generate invalid object files, instead of checking in precanned binaries. You can find lots of grimar's changes migrating away from binaries.

I generally prefer that method too, however I didn't think it was possible for this kind of case. How would I generate an invalid relocation type? Currently relocations are specified with a string:

For example here is the yaml I used as a starting point for this invalid binary:

--- !WASM
FileHeader:
  Version:         0x1
Sections:
  - Type:            TYPE
    Signatures:
      - Index:           0
        ParamTypes:      []
        ReturnTypes:
          - I32
  - Type:            FUNCTION
    FunctionTypes:   [ 0 ]
  - Type:            CODE
    Relocations:
      - Type:            R_WASM_TABLE_INDEX_SLEB
        Index:           0
        Offset:          0x4
    Functions:
      - Index:           0
        Locals:          []
        Body:            4181808080000B
  - Type:            CUSTOM
    Name:            linking
    Version:         2
    SymbolTable:
      - Index:           0
        Kind:            FUNCTION
        Name:            foo
        Flags:           [ VISIBILITY_HIDDEN ]
        Function:        0

In this case I modified the binary to change the R_WASM_TABLE_INDEX_SLEB value to 63 (an invalid value). If I try to do that in the yaml I get:

yaml2obj out.yaml > obj.o
YAML:15:26: error: unknown enumerated scalar
      - Type:            63
                         ^~
yaml2obj: error: failed to parse YAML input: Invalid argument

Also, I see a lot of ELF files (and very few yaml files) checked in directly to llvm/test/Object/Inputs/. Is that just because there is ongoing work to covert them?

Maybe D95661:)

llvm/lib/Object/WasmObjectFile.cpp

case wasm::WASM_SYMBOL_TYPE_EVENT: {
         if (!Import.Module.empty()) {
           Info.ImportModule = Import.Module;
         }
       }
       break;
     }
 
     default:
-      return make_error<GenericBinaryError>("Invalid symbol type",
+      return make_error<GenericBinaryError>("Invalid symbol type: " +
+                                                Twine(unsigned(Info.Kind)),
                                             object_error::parse_failed);
     }
 
     if ((Info.Flags & wasm::WASM_SYMBOL_BINDING_MASK) !=
             wasm::WASM_SYMBOL_BINDING_LOCAL &&
         !SymbolNames.insert(Info.Name).second)
       return make_error<GenericBinaryError>("Duplicate symbol name " +
                                                 Twine(Info.Name),
if (SectionIndex >= Sections.size())
     return make_error<GenericBinaryError>("Invalid section index",
                                           object_error::parse_failed);
   WasmSection &Section = Sections[SectionIndex];
   uint32_t RelocCount = readVaruint32(Ctx);
   uint32_t EndOffset = Section.Content.size();
   uint32_t PreviousOffset = 0;
   while (RelocCount--) {
     wasm::WasmRelocation Reloc = {};
-    Reloc.Type = readVaruint32(Ctx);
+    uint32_t type = readVaruint32(Ctx);
+    Reloc.Type = type;
     Reloc.Offset = readVaruint32(Ctx);
     if (Reloc.Offset < PreviousOffset)
       return make_error<GenericBinaryError>("Relocations not in offset order",
                                             object_error::parse_failed);
     PreviousOffset = Reloc.Offset;
     Reloc.Index = readVaruint32(Ctx);
-    switch (Reloc.Type) {
+    switch (type) {
     case wasm::R_WASM_FUNCTION_INDEX_LEB:
     case wasm::R_WASM_TABLE_INDEX_SLEB:
     case wasm::R_WASM_TABLE_INDEX_SLEB64:
     case wasm::R_WASM_TABLE_INDEX_I32:
     case wasm::R_WASM_TABLE_INDEX_I64:
     case wasm::R_WASM_TABLE_INDEX_REL_SLEB:
       if (!isValidFunctionSymbol(Reloc.Index))
         return make_error<GenericBinaryError>("Bad relocation function index",
case wasm::R_WASM_FUNCTION_OFFSET_I64:
       break;
     case wasm::R_WASM_SECTION_OFFSET_I32:
       if (!isValidSectionSymbol(Reloc.Index))
         return make_error<GenericBinaryError>("Bad relocation section index",
                                               object_error::parse_failed);
       Reloc.Addend = readVarint32(Ctx);
       break;
     default:
-      return make_error<GenericBinaryError>("Bad relocation type: " +
-                                                Twine(Reloc.Type),
-                                            object_error::parse_failed);
+      return make_error<GenericBinaryError>(
+          "Bad relocation type: " + Twine(type), object_error::parse_failed);
     }
 
     // Relocations must fit inside the section, and must appear in order.  They
     // also shouldn't overlap a function/element boundary, but we don't bother
     // to check that.
     uint64_t Size = 5;
     if (Reloc.Type == wasm::R_WASM_MEMORY_ADDR_LEB64 ||
         Reloc.Type == wasm::R_WASM_MEMORY_ADDR_SLEB64 ||

llvm/test/Object/wasm-bad-reloc-type.test

+RUN: not llvm-objdump -s %p/Inputs/WASM/bad-reloc-type.wasm 2>&1 | FileCheck %s
+
+CHECK: Bad relocation type: 63

llvm/test/Object/wasm-bad-symbol-type.test

+RUN: not llvm-objdump -s %p/Inputs/WASM/bad-symbol-type.wasm 2>&1 | FileCheck %s
+
+CHECK: Invalid symbol type: 63