[WebAssembly][Object] Remove requirement that objects must have code sections

[dschuff]

Summary

When parsing name and linking sections, we currently require that the object
must have a code section (it seems that this was intended to verify section
ordering). However it can be useful for binaries to have their code sections
stripped out (e.g. if we just want the debug info). In that case we need
the rest of the known sections (so e.g. we know how many functions there
are, to verify the name section) but not the actual code.

I've removed the restriction completely. I think this is OK because the
section-parsing code already checks function and global indices in many
places for validity and will return appropriate errors if the relevant sections
are missing. Also we can't just replace the requirement of seeing a code section
with a requirement that we see a function or global section, because a binary
may just not have any functions or globals.
But there's only an problem if the name or linking section tries to name a
nonexistent function.

Part of a fix for https://github.com/emscripten-core/emscripten/issues/13084

[sbc100]

llvm/lib/Object/WasmObjectFile.cpp

526

What happens if the linking metdata comes before the code section now? Is that OK?

llvm/test/tools/llvm-objdump/wasm/no-codesec.test

27

This is no longer a valid wasm file though right? Since it refers to a function that doesn't have a body. I guess that is maybe ok as long as we produce valid wasm files by default?

[dschuff]

llvm/lib/Object/WasmObjectFile.cpp

526

The code section is not actually needed to check the linking metadata; only the Function, Global, and Import sections IIUC. So yes it's OK.

llvm/test/tools/llvm-objdump/wasm/no-codesec.test

27

Correct; I think a function without a body would not validate. The use case for this is to have a wasm file with just debug info in it (e.g. to take a debug-info-containing wasm file and ship a stripped version, and archive the debug info separately).

27

and I think that's ok for low-level tools like objcopy and objdump.

[aheejin]

(Also we can't just always require the presence of e.g. the
function or global sections, because a binary may just have any functions.
There's only an problem if the name or linking section tries to name a
nonexistent function).

Code LGTM, but not sure what this sentence means.. Can you provide a little more context?

[dschuff]

In D128094#3593734, @aheejin wrote:

(Also we can't just always require the presence of e.g. the
function or global sections, because a binary may just have any functions.
There's only an problem if the name or linking section tries to name a
nonexistent function).

Code LGTM, but not sure what this sentence means.. Can you provide a little more context?

Oops, that should say "a binary may just *not* have any functions." (or globals). In other words, we could try replacing the requirement that we've seen the code section with a requirement that we've seen the Function section; but there still could be perfectly valid wasm binaries with no function section (or maybe we want to be able to parse binaries that only have custom sections and no functions, and we don't care about validation).

[dschuff]

rebase

llvm/include/llvm/Object/Wasm.h

private:
   std::vector<WasmSegment> DataSegments;
   llvm::Optional<size_t> DataCount;
   std::vector<wasm::WasmFunction> Functions;
   std::vector<WasmSymbol> Symbols;
   std::vector<wasm::WasmDebugName> DebugNames;
   uint32_t StartFunction = -1;
   bool HasLinkingSection = false;
   bool HasDylinkSection = false;
-  bool SeenCodeSection = false;
   bool HasMemory64 = false;
   wasm::WasmLinkingData LinkingData;
   uint32_t NumImportedGlobals = 0;
   uint32_t NumImportedTables = 0;
   uint32_t NumImportedFunctions = 0;
   uint32_t NumImportedTags = 0;
   uint32_t CodeSection = 0;
   uint32_t DataSection = 0;

llvm/lib/Object/WasmObjectFile.cpp

return make_error<GenericBinaryError>("dylink.0 section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseNameSection(ReadContext &Ctx) {
   llvm::DenseSet<uint64_t> SeenFunctions;
   llvm::DenseSet<uint64_t> SeenGlobals;
   llvm::DenseSet<uint64_t> SeenSegments;
-  if (Functions.size() && !SeenCodeSection) {
-    return make_error<GenericBinaryError>("names must come after code section",
-                                          object_error::parse_failed);
-  }
 
   while (Ctx.Ptr < Ctx.End) {
     uint8_t Type = readUint8(Ctx);
     uint32_t Size = readVaruint32(Ctx);
     const uint8_t *SubSectionEnd = Ctx.Ptr + Size;
     switch (Type) {
     case wasm::WASM_NAMES_FUNCTION:
     case wasm::WASM_NAMES_GLOBAL:
     case wasm::WASM_NAMES_DATA_SEGMENT: {
       uint32_t Count = readVaruint32(Ctx);
       while (Count--) {
         uint32_t Index = readVaruint32(Ctx);
         StringRef Name = readString(Ctx);
         wasm::NameType nameType = wasm::NameType::FUNCTION;
         if (Type == wasm::WASM_NAMES_FUNCTION) {
           if (!SeenFunctions.insert(Index).second)
             return make_error<GenericBinaryError>(
                 "function named more than once", object_error::parse_failed);
           if (!isValidFunctionIndex(Index) || Name.empty())
-            return make_error<GenericBinaryError>("invalid name entry",
+            return make_error<GenericBinaryError>("invalid function name entry",
                                                   object_error::parse_failed);
 
           if (isDefinedFunctionIndex(Index))
             getDefinedFunction(Index).DebugName = Name;
         } else if (Type == wasm::WASM_NAMES_GLOBAL) {
           nameType = wasm::NameType::GLOBAL;
           if (!SeenGlobals.insert(Index).second)
             return make_error<GenericBinaryError>("global named more than once",
                                                   object_error::parse_failed);
           if (!isValidGlobalIndex(Index) || Name.empty())
-            return make_error<GenericBinaryError>("invalid name entry",
+            return make_error<GenericBinaryError>("invalid global name entry",
                                                   object_error::parse_failed);
         } else {
           nameType = wasm::NameType::DATA_SEGMENT;
           if (!SeenSegments.insert(Index).second)
             return make_error<GenericBinaryError>(
                 "segment named more than once", object_error::parse_failed);
           if (Index > DataSegments.size())
-            return make_error<GenericBinaryError>("invalid named data segment",
+            return make_error<GenericBinaryError>("invalid data segment name entry",
                                                   object_error::parse_failed);
         }
         DebugNames.push_back(wasm::WasmDebugName{nameType, Index, Name});
       }
       break;
     }
     // Ignore local names for now
     case wasm::WASM_NAMES_LOCAL:
Error WasmObjectFile::parseNameSection(ReadContext &Ctx) {
   if (Ctx.Ptr != Ctx.End)
     return make_error<GenericBinaryError>("name section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseLinkingSection(ReadContext &Ctx) {
   HasLinkingSection = true;
-  if (Functions.size() && !SeenCodeSection) {
-    return make_error<GenericBinaryError>(
-        "linking data must come after code section",
-        object_error::parse_failed);
-  }
 
   LinkingData.Version = readVaruint32(Ctx);
   if (LinkingData.Version != wasm::WasmMetadataVersion) {
     return make_error<GenericBinaryError>(
         "unexpected metadata version: " + Twine(LinkingData.Version) +
             " (Expected: " + Twine(wasm::WasmMetadataVersion) + ")",
         object_error::parse_failed);
   }
Error WasmObjectFile::parseStartSection(ReadContext &Ctx) {
   StartFunction = readVaruint32(Ctx);
   if (!isValidFunctionIndex(StartFunction))
     return make_error<GenericBinaryError>("invalid start function",
                                           object_error::parse_failed);
   return Error::success();
 }
 
 Error WasmObjectFile::parseCodeSection(ReadContext &Ctx) {
-  SeenCodeSection = true;
   CodeSection = Sections.size();
   uint32_t FunctionCount = readVaruint32(Ctx);
   if (FunctionCount != Functions.size()) {
     return make_error<GenericBinaryError>("invalid function count",
                                           object_error::parse_failed);
   }
 
   for (uint32_t i = 0; i < FunctionCount; i++) {

llvm/test/tools/llvm-objdump/wasm/no-codesec.test

+## Test that sections that use function indices (e.g. name) work without the code section
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump -h %t | FileCheck %s
+
+# CHECK: Sections:
+# CHECK-NEXT: Idx Name          Size     VMA      Type
+# CHECK-NEXT:   0 TYPE          00000004 00000000 
+# CHECK-NEXT:   1 FUNCTION      00000002 00000000 
+# CHECK-NEXT:   2 name          00000008 00000000
+  
+--- !WASM
+FileHeader:
+  Version: 0x00000001
+Sections:
+  - Type: TYPE
+    Signatures:
+      - Index: 0
+        ParamTypes: []
+        ReturnTypes: []
+  - Type: FUNCTION
+    FunctionTypes: [ 0 ]
+  - Type: CUSTOM
+    Name: name
+    FunctionNames:
+     - Index:          0
+       Name:           foo