[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 ↗

(On Diff #301749)

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/test/wasm/check-arch-32-in-64.test

+RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/start.s -o %t.o
+RUN: not wasm-ld -mwasm64 %t.o -o %t.wasm 2>&1 | FileCheck %s
+
+CHECK: wasm32 object file can't be linked in wasm64 mode

lld/test/wasm/check-arch-64-in-32.test

+RUN: llvm-mc -filetype=obj -triple=wasm64-unknown-unknown %p/Inputs/start.s -o %t.o
+RUN: not wasm-ld %t.o -o %t.wasm 2>&1 | FileCheck %s
+
+CHECK: must specify -mwasm64 to process wasm64 object files

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 paddedLEBWidth = 5;
     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);
+      break;
     case R_WASM_MEMORY_ADDR_LEB64:
       existingValue = decodeULEB128(loc, &bytesRead);
+      paddedLEBWidth = 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));
+      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));
+      paddedLEBWidth = 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 != paddedLEBWidth)
+      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.h

 //===- InputFiles.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_WASM_INPUT_FILES_H
 #define LLD_WASM_INPUT_FILES_H
 
 #include "Symbols.h"
 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Triple.h"
 #include "llvm/LTO/LTO.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <vector>
 
 namespace llvm {
 class TarWriter;
public:
   // An InputFile is considered live if any of the symbols defined by it
   // are live.
   void markLive() { live = true; }
   bool isLive() const { return live; }
 
 protected:
   InputFile(Kind k, MemoryBufferRef m)
       : mb(m), fileKind(k), live(!config->gcSections) {}
+
+  void checkArch(llvm::Triple::ArchType arch) const;
+
   MemoryBufferRef mb;
 
   // List of all symbols referenced or defined by this file.
   std::vector<Symbol *> symbols;
 
 private:
   const Kind fileKind;
   bool live;

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 InputFile::checkArch(Triple::ArchType arch) const {
+  bool is64 = arch == Triple::wasm64;
+  if (is64 && !config->is64.hasValue()) {
+    fatal(toString(this) +
+          ": must specify -mwasm64 to process wasm64 object files");
+  } else if (config->is64.getValueOr(false) != is64) {
+    fatal(toString(this) +
+          ": 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());
+
   // 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());
   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