[WebAssembly] Fix R_WEBASSEMBLY_FUNCTION_OFFSET_I32 relocation entries

[yurydelendik]

Summary

Fixes: lld: warning: unexpected existing value for R_WEBASSEMBLY_FUNCTION_OFFSET_I32: existing=839 expected=838

The existing solution is trying to erroneously recover correct offset of the
function code from the body (which is not a function segment that includes its
size, locals, and code).

The D46763 is trying to maintain the offset of the function code allowing
properly calculate the new relocation entry.

(Test case is possible, but will require 128+ bytes function code segments)

[sbc100]

wasm/InputFiles.cpp

48 ↗

(On Diff #146371)

Where is the actual bug? This code looks like its doing the right thing, no?

[yurydelendik]

wasm/InputFiles.cpp

48 ↗

(On Diff #146371)

Actual bug is: Function->getFunctionBody() returns only operators data (without function size and params/locals), but I assumed it will be an entire function data (inculding function size).

The actual intent was to know function code offset (+params/locals) to properly perform calcNewValue and calcExpectedValue

[sbc100]

I think you can remove getFunctionBody()?

[yurydelendik]

Remove getFunctionBody()

lld/trunk/wasm/InputChunks.h

public:
   static bool classof(const InputChunk *C) {
     return C->kind() == InputChunk::Function ||
            C->kind() == InputChunk::SyntheticFunction;
   }
 
   StringRef getName() const override { return Function->SymbolName; }
   StringRef getDebugName() const override { return Function->DebugName; }
   uint32_t getComdat() const override { return Function->Comdat; }
-  const ArrayRef<uint8_t> getFunctionBody() const { return Function->Body; }
   uint32_t getFunctionInputOffset() const { return getInputSectionOffset(); }
+  uint32_t getFunctionCodeOffset() const { return Function->CodeOffset; }
   uint32_t getFunctionIndex() const { return FunctionIndex.getValue(); }
   bool hasFunctionIndex() const { return FunctionIndex.hasValue(); }
   void setFunctionIndex(uint32_t Index);
   uint32_t getTableIndex() const { return TableIndex.getValue(); }
   bool hasTableIndex() const { return TableIndex.hasValue(); }
   void setTableIndex(uint32_t Index);
 
   const WasmSignature &Signature;

lld/trunk/wasm/InputFiles.cpp

 #include "Config.h"
 #include "InputChunks.h"
 #include "InputGlobal.h"
 #include "SymbolTable.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Memory.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/Wasm.h"
-#include "llvm/Support/LEB128.h"
 #include "llvm/Support/raw_ostream.h"
 
 #define DEBUG_TYPE "lld"
 
 using namespace lld;
 using namespace lld::wasm;
 
 using namespace llvm;
Optional<MemoryBufferRef> lld::wasm::readFile(StringRef Path) {
   }
   std::unique_ptr<MemoryBuffer> &MB = *MBOrErr;
   MemoryBufferRef MBRef = MB->getMemBufferRef();
   make<std::unique_ptr<MemoryBuffer>>(std::move(MB)); // take MB ownership
 
   return MBRef;
 }
 
-static size_t getFunctionCodeOffset(ArrayRef<uint8_t> FunctionBody) {
-  unsigned Count;
-  llvm::decodeULEB128(FunctionBody.data(), &Count);
-  return Count;
-}
-
 void ObjFile::dumpInfo() const {
   log("info for: " + getName() +
       "\n              Symbols : " + Twine(Symbols.size()) +
       "\n     Function Imports : " + Twine(WasmObj->getNumImportedFunctions()) +
       "\n       Global Imports : " + Twine(WasmObj->getNumImportedGlobals()));
 }
 
 // Relocations contain either symbol or type indices.  This function takes a
case R_WEBASSEMBLY_MEMORY_ADDR_LEB: {
     if (Sym.isUndefined())
       return 0;
     const WasmSegment& Segment = WasmObj->dataSegments()[Sym.Info.DataRef.Segment];
     return Segment.Data.Offset.Value.Int32 + Sym.Info.DataRef.Offset +
            Reloc.Addend;
   }
   case R_WEBASSEMBLY_FUNCTION_OFFSET_I32:
     if (auto *Sym = dyn_cast<DefinedFunction>(getFunctionSymbol(Reloc.Index))) {
-      size_t FunctionCodeOffset =
-          getFunctionCodeOffset(Sym->Function->getFunctionBody());
-      return Sym->Function->getFunctionInputOffset() + FunctionCodeOffset +
-             Reloc.Addend;
+      return Sym->Function->getFunctionInputOffset() +
+             Sym->Function->getFunctionCodeOffset() + Reloc.Addend;
     }
     return 0;
   case R_WEBASSEMBLY_SECTION_OFFSET_I32:
     return Reloc.Addend;
   case R_WEBASSEMBLY_TYPE_INDEX_LEB:
     return Reloc.Index;
   case R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
   case R_WEBASSEMBLY_GLOBAL_INDEX_LEB: {
uint32_t ObjFile::calcNewValue(const WasmRelocation &Reloc) const {
   case R_WEBASSEMBLY_TYPE_INDEX_LEB:
     return TypeMap[Reloc.Index];
   case R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
     return getFunctionSymbol(Reloc.Index)->getFunctionIndex();
   case R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
     return getGlobalSymbol(Reloc.Index)->getGlobalIndex();
   case R_WEBASSEMBLY_FUNCTION_OFFSET_I32:
     if (auto *Sym = dyn_cast<DefinedFunction>(getFunctionSymbol(Reloc.Index))) {
-      size_t FunctionCodeOffset =
-          getFunctionCodeOffset(Sym->Function->getFunctionBody());
-      return Sym->Function->OutputOffset + FunctionCodeOffset + Reloc.Addend;
+      return Sym->Function->OutputOffset +
+             Sym->Function->getFunctionCodeOffset() + Reloc.Addend;
     }
     return 0;
   case R_WEBASSEMBLY_SECTION_OFFSET_I32:
     return getSectionSymbol(Reloc.Index)->Section->OutputOffset + Reloc.Addend;
   default:
     llvm_unreachable("unknown relocation type");
   }
 }