[WebAssembly] Improved LLD error messages in case of mixed wasm32/wasm64 object files

[sbc100]

lld/wasm/InputChunks.cpp

63

Maybe just initialize to 5 and override for the 64-bit cases.

Also maybe a better name, since this is not that amount of padded by the overall width. Howabout maxLEBWidth? Or paddedLEBWidth?

lld/wasm/InputFiles.cpp

51

No need to escape single quote is there?

Can we get tests for these new error cases?

600

Passing this at each callside makes me think this should be a method, no?

llvm/lib/MC/WasmObjectWriter.cpp

502

Revert this ?

[aardappel]

lld/wasm/InputFiles.cpp

51

You want a test that builds two different .o files and tries to link them? Doesn't that sound a bit overkill for this? Do all LLD errors have tests? Certainly the ones we already had that I am replacing didn't.

[sbc100]

lld/wasm/InputFiles.cpp

51

You thats exactly what I mean. If the existing error message didn't have tests they should have done.

It should be trivial to just use and empty .s file as the test, since all you need to do is compile it two ways and try to link it two ways to get each error.

Or you can use a .test that references and existing .s such as lld/test/wasm/Inputs/start.s

[aardappel]

lld/wasm/InputFiles.cpp

51

well, it can't be empty since it

600

I considered that, but then I made it similar to the already existing toString, as an implementation detail local to this file seemed cleaner to me. Also pulls a new include into the header. But as you wish.

[aardappel]

Review fixes and tests.

[sbc100]

Nice!

lld/wasm/InputChunks.cpp

if (index == UINT32_MAX)
     return StringRef();
   return file->getWasmObj()->linkingData().Comdats[index];
 }
 
 void InputChunk::verifyRelocTargets() const {
   for (const WasmRelocation &rel : relocations) {
     uint64_t existingValue;
     unsigned bytesRead = 0;
+    unsigned padding = 0;
     auto offset = rel.Offset - getInputSectionOffset();
     const uint8_t *loc = data().data() + offset;
     switch (rel.Type) {
     case R_WASM_TYPE_INDEX_LEB:
     case R_WASM_FUNCTION_INDEX_LEB:
     case R_WASM_GLOBAL_INDEX_LEB:
     case R_WASM_EVENT_INDEX_LEB:
     case R_WASM_MEMORY_ADDR_LEB:
+      existingValue = decodeULEB128(loc, &bytesRead);
+      padding = 5;
+      break;
     case R_WASM_MEMORY_ADDR_LEB64:
       existingValue = decodeULEB128(loc, &bytesRead);
+      padding = 10;
       break;
     case R_WASM_TABLE_INDEX_SLEB:
-    case R_WASM_TABLE_INDEX_SLEB64:
     case R_WASM_TABLE_INDEX_REL_SLEB:
     case R_WASM_MEMORY_ADDR_SLEB:
-    case R_WASM_MEMORY_ADDR_SLEB64:
     case R_WASM_MEMORY_ADDR_REL_SLEB:
+      existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
+      padding = 5;
+      break;
+    case R_WASM_TABLE_INDEX_SLEB64:
+    case R_WASM_MEMORY_ADDR_SLEB64:
     case R_WASM_MEMORY_ADDR_REL_SLEB64:
       existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
+      padding = 10;
       break;
     case R_WASM_TABLE_INDEX_I32:
     case R_WASM_MEMORY_ADDR_I32:
     case R_WASM_FUNCTION_OFFSET_I32:
     case R_WASM_SECTION_OFFSET_I32:
     case R_WASM_GLOBAL_INDEX_I32:
       existingValue = read32le(loc);
       break;
     case R_WASM_TABLE_INDEX_I64:
     case R_WASM_MEMORY_ADDR_I64:
       existingValue = read64le(loc);
       break;
     default:
       llvm_unreachable("unknown relocation type");
     }
 
-    if (bytesRead && bytesRead != 5)
-      warn("expected LEB at relocation site be 5-byte padded");
+    if (bytesRead && bytesRead != padding)
+      warn("expected LEB at relocation site be 5/10-byte padded");
 
     if (rel.Type != R_WASM_GLOBAL_INDEX_LEB &&
         rel.Type != R_WASM_GLOBAL_INDEX_I32) {
       auto expectedValue = file->calcExpectedValue(rel);
       if (expectedValue != existingValue)
         warn("unexpected existing value for " + relocTypeToString(rel.Type) +
              ": existing=" + Twine(existingValue) +
              " expected=" + Twine(expectedValue));

lld/wasm/InputFiles.cpp

std::string toString(const wasm::InputFile *file) {
 
   if (file->archiveName.empty())
     return std::string(file->getName());
 
   return (file->archiveName + "(" + file->getName() + ")").str();
 }
 
 namespace wasm {
+
+void checkArch(Triple::ArchType arch, const wasm::InputFile *file) {
+  bool is64 = arch == Triple::wasm64;
+  if (is64 && !config->is64.hasValue()) {
+    fatal(toString(file) +
+          ": must specify -mwasm64 to process wasm64 object files");
+  } else if (config->is64.getValueOr(false) != is64) {
+    fatal(toString(file) +
+          ": wasm32 object file can\'t be linked in wasm64 mode");
+  }
+}
+
 std::unique_ptr<llvm::TarWriter> tar;
 
 Optional<MemoryBufferRef> readFile(StringRef path) {
   log("Loading: " + path);
 
   auto mbOrErr = MemoryBuffer::getFile(path);
   if (auto ec = mbOrErr.getError()) {
     error("cannot open " + path + ": " + ec.message());
void ObjFile::parse(bool ignoreComdats) {
   if (!obj)
     fatal(toString(this) + ": not a wasm file");
   if (!obj->isRelocatableObject())
     fatal(toString(this) + ": not a relocatable wasm file");
 
   bin.release();
   wasmObj.reset(obj);
 
+  checkArch(obj->getArch(), this);
+
   // Build up a map of function indices to table indices for use when
   // verifying the existing table index relocations
   uint32_t totalFunctions =
       wasmObj->getNumImportedFunctions() + wasmObj->functions().size();
   tableEntriesRel.resize(totalFunctions);
   tableEntries.resize(totalFunctions);
   for (const WasmElemSegment &seg : wasmObj->elements()) {
     int64_t offset;
void BitcodeFile::parse() {
 
   obj = check(lto::InputFile::create(MemoryBufferRef(
       mb.getBuffer(), saver.save(archiveName + mb.getBufferIdentifier()))));
   Triple t(obj->getTargetTriple());
   if (!t.isWasm()) {
     error(toString(this) + ": machine type must be wasm32 or wasm64");
     return;
   }
-  bool is64 = t.getArch() == Triple::wasm64;
-  if (config->is64.hasValue() && *config->is64 != is64) {
-    error(toString(this) + ": machine type for all bitcode files must match");
-    return;
-  }
-  config->is64 = is64;
+  checkArch(t.getArch(), this);
   std::vector<bool> keptComdats;
   for (StringRef s : obj->getComdatTable())
     keptComdats.push_back(symtab->addComdat(s));
 
   for (const lto::InputFile::Symbol &objSym : obj->symbols())
     symbols.push_back(createBitcodeSymbol(keptComdats, objSym, *this));
 }
 
 } // namespace wasm
 } // namespace lld

llvm/lib/MC/WasmObjectWriter.cpp

if (!SectionSymbol)
       report_fatal_error("section symbol is required for relocation");
 
     C += Layout.getSymbolOffset(*SymA);
     SymA = cast<MCSymbolWasm>(SectionSymbol);
   }
 
   // Relocation other than R_WASM_TYPE_INDEX_LEB are required to be
   // against a named symbol.
-  if (Type != wasm::R_WASM_TYPE_INDEX_LEB) {
+  if (Type != wasm::R_WASM_TYPE_INDEX_LEB /*&&
+      Type != wasm::R_WASM_MEMORY_ADDR_I64*/) {
     if (SymA->getName().empty())
       report_fatal_error("relocations against un-named temporaries are not yet "
                          "supported by wasm");
 
     SymA->setUsedInReloc();
   }
 
   if (RefA->getKind() == MCSymbolRefExpr::VK_GOT)