[WebAssembly] Generalize section ordering constraints

Summary:
Changes from using a total ordering of known sections to using a
dependency graph approach. This allows our tools to accept and process
binaries that are compliant with the spec and tool conventions that
would have been previously rejected. It also means our own tools can
do less work to enforce an artificially imposed ordering. Using a
general mechanism means fewer special cases and exceptions in the
ordering logic.

Reviewers: aheejin, dschuff

Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, jdoerfert, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D58312

llvm-svn: 354426

diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h
index a0a2c4b..e3d1e24 100644 (file)
--- a/llvm/include/llvm/Object/Wasm.h
+++ b/llvm/include/llvm/Object/Wasm.h
@@ -288,40 +288,49 @@ class WasmSectionOrderChecker {
 public:
   // We define orders for all core wasm sections and known custom sections.
   enum : int {
+    // Sentinel, must be zero
+    WASM_SEC_ORDER_NONE = 0,
+
     // Core sections
-    // The order of standard sections is precisely given by the spec.
-    WASM_SEC_ORDER_TYPE = 1,
-    WASM_SEC_ORDER_IMPORT = 2,
-    WASM_SEC_ORDER_FUNCTION = 3,
-    WASM_SEC_ORDER_TABLE = 4,
-    WASM_SEC_ORDER_MEMORY = 5,
-    WASM_SEC_ORDER_GLOBAL = 6,
-    WASM_SEC_ORDER_EVENT = 7,
-    WASM_SEC_ORDER_EXPORT = 8,
-    WASM_SEC_ORDER_START = 9,
-    WASM_SEC_ORDER_ELEM = 10,
-    WASM_SEC_ORDER_DATACOUNT = 11,
-    WASM_SEC_ORDER_CODE = 12,
-    WASM_SEC_ORDER_DATA = 13,
+    WASM_SEC_ORDER_TYPE,
+    WASM_SEC_ORDER_IMPORT,
+    WASM_SEC_ORDER_FUNCTION,
+    WASM_SEC_ORDER_TABLE,
+    WASM_SEC_ORDER_MEMORY,
+    WASM_SEC_ORDER_GLOBAL,
+    WASM_SEC_ORDER_EVENT,
+    WASM_SEC_ORDER_EXPORT,
+    WASM_SEC_ORDER_START,
+    WASM_SEC_ORDER_ELEM,
+    WASM_SEC_ORDER_DATACOUNT,
+    WASM_SEC_ORDER_CODE,
+    WASM_SEC_ORDER_DATA,
 
     // Custom sections
     // "dylink" should be the very first section in the module
-    WASM_SEC_ORDER_DYLINK = 0,
+    WASM_SEC_ORDER_DYLINK,
     // "linking" section requires DATA section in order to validate data symbols
-    WASM_SEC_ORDER_LINKING = 100,
+    WASM_SEC_ORDER_LINKING,
     // Must come after "linking" section in order to validate reloc indexes.
-    WASM_SEC_ORDER_RELOC = 101,
+    WASM_SEC_ORDER_RELOC,
     // "name" section must appear after DATA. Comes after "linking" to allow
     // symbol table to set default function name.
-    WASM_SEC_ORDER_NAME = 102,
+    WASM_SEC_ORDER_NAME,
     // "producers" section must appear after "name" section.
-    WASM_SEC_ORDER_PRODUCERS = 103
+    WASM_SEC_ORDER_PRODUCERS,
+
+    // Must be last
+    WASM_NUM_SEC_ORDERS
+
   };
 
+  // Sections that may or may not be present, but cannot be predecessors
+  static int DisallowedPredecessors[WASM_NUM_SEC_ORDERS][WASM_NUM_SEC_ORDERS];
+
   bool isValidSectionOrder(unsigned ID, StringRef CustomSectionName = "");
 
 private:
-  int LastOrder = -1; // Lastly seen known section's order
+  bool Seen[WASM_NUM_SEC_ORDERS] = {}; // Sections that have been seen already
 
   // Returns -1 for unknown sections.
   int getSectionOrder(unsigned ID, StringRef CustomSectionName = "");

diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 5a41ca3..9d084bf 100644 (file)
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -1527,7 +1527,7 @@ int WasmSectionOrderChecker::getSectionOrder(unsigned ID,
         .StartsWith("reloc.", WASM_SEC_ORDER_RELOC)
         .Case("name", WASM_SEC_ORDER_NAME)
         .Case("producers", WASM_SEC_ORDER_PRODUCERS)
-        .Default(-1);
+        .Default(WASM_SEC_ORDER_NONE);
   case wasm::WASM_SEC_TYPE:
     return WASM_SEC_ORDER_TYPE;
   case wasm::WASM_SEC_IMPORT:
@@ -1559,15 +1559,68 @@ int WasmSectionOrderChecker::getSectionOrder(unsigned ID,
   }
 }
 
+// Represents the edges in a directed graph where any node B reachable from node
+// A is not allowed to appear before A in the section ordering, but may appear
+// afterward.
+int WasmSectionOrderChecker::DisallowedPredecessors[WASM_NUM_SEC_ORDERS][WASM_NUM_SEC_ORDERS] = {
+  {}, // WASM_SEC_ORDER_NONE
+  {WASM_SEC_ORDER_TYPE, WASM_SEC_ORDER_IMPORT}, // WASM_SEC_ORDER_TYPE,
+  {WASM_SEC_ORDER_IMPORT, WASM_SEC_ORDER_FUNCTION}, // WASM_SEC_ORDER_IMPORT,
+  {WASM_SEC_ORDER_FUNCTION, WASM_SEC_ORDER_TABLE}, // WASM_SEC_ORDER_FUNCTION,
+  {WASM_SEC_ORDER_TABLE, WASM_SEC_ORDER_MEMORY}, // WASM_SEC_ORDER_TABLE,
+  {WASM_SEC_ORDER_MEMORY, WASM_SEC_ORDER_GLOBAL}, // WASM_SEC_ORDER_MEMORY,
+  {WASM_SEC_ORDER_GLOBAL, WASM_SEC_ORDER_EVENT}, // WASM_SEC_ORDER_GLOBAL,
+  {WASM_SEC_ORDER_EVENT, WASM_SEC_ORDER_EXPORT}, // WASM_SEC_ORDER_EVENT,
+  {WASM_SEC_ORDER_EXPORT, WASM_SEC_ORDER_START}, // WASM_SEC_ORDER_EXPORT,
+  {WASM_SEC_ORDER_START, WASM_SEC_ORDER_ELEM}, // WASM_SEC_ORDER_START,
+  {WASM_SEC_ORDER_ELEM, WASM_SEC_ORDER_DATACOUNT}, // WASM_SEC_ORDER_ELEM,
+  {WASM_SEC_ORDER_DATACOUNT, WASM_SEC_ORDER_CODE}, // WASM_SEC_ORDER_DATACOUNT,
+  {WASM_SEC_ORDER_CODE, WASM_SEC_ORDER_DATA}, // WASM_SEC_ORDER_CODE,
+  {WASM_SEC_ORDER_DATA, WASM_SEC_ORDER_LINKING}, // WASM_SEC_ORDER_DATA,
+
+  // Custom Sections
+  {WASM_SEC_ORDER_DYLINK, WASM_SEC_ORDER_TYPE}, // WASM_SEC_ORDER_DYLINK,
+  {WASM_SEC_ORDER_LINKING, WASM_SEC_ORDER_RELOC, WASM_SEC_ORDER_NAME}, // WASM_SEC_ORDER_LINKING,
+  {}, // WASM_SEC_ORDER_RELOC (can be repeated),
+  {WASM_SEC_ORDER_NAME, WASM_SEC_ORDER_PRODUCERS}, // WASM_SEC_ORDER_NAME,
+  {WASM_SEC_ORDER_PRODUCERS}, // WASM_SEC_ORDER_PRODUCERS,
+};
+
 bool WasmSectionOrderChecker::isValidSectionOrder(unsigned ID,
                                                   StringRef CustomSectionName) {
   int Order = getSectionOrder(ID, CustomSectionName);
-  if (Order == -1) // Skip unknown sections
+  if (Order == WASM_SEC_ORDER_NONE)
     return true;
-  // There can be multiple "reloc." sections. Otherwise there shouldn't be any
-  // duplicate section orders.
-  bool IsValid = (LastOrder == Order && Order == WASM_SEC_ORDER_RELOC) ||
-                 LastOrder < Order;
-  LastOrder = Order;
-  return IsValid;
+
+  // Disallowed predecessors we need to check for
+  SmallVector<int, WASM_NUM_SEC_ORDERS> WorkList;
+
+  // Keep track of completed checks to avoid repeating work
+  bool Checked[WASM_NUM_SEC_ORDERS] = {};
+
+  int Curr = Order;
+  while (true) {
+    // Add new disallowed predecessors to work list
+    for (size_t I = 0;; ++I) {
+      int Next = DisallowedPredecessors[Curr][I];
+      if (Next == WASM_SEC_ORDER_NONE)
+        break;
+      if (Checked[Next])
+        continue;
+      WorkList.push_back(Next);
+      Checked[Next] = true;
+    }
+
+    if (WorkList.empty())
+      break;
+
+    // Consider next disallowed predecessor
+    Curr = WorkList.pop_back_val();
+    if (Seen[Curr])
+      return false;
+  }
+
+  // Have not seen any disallowed predecessors
+  Seen[Order] = true;
+  return true;
 }

diff --git a/llvm/test/Object/wasm-relocs-and-producers.yaml b/llvm/test/Object/wasm-relocs-and-producers.yaml
new file mode 100644 (file)
index 0000000..01ad2bb
--- /dev/null
+++ b/llvm/test/Object/wasm-relocs-and-producers.yaml
@@ -0,0 +1,60 @@
+# RUN: yaml2obj %s | llvm-objdump -s - | FileCheck %s
+
+# This is a regression test for an issue with the section order
+# checker being overly strict. yaml2obj places the relocations last,
+# but the section order checker previously checked that relocations
+# came before the producers section, which would cause this test to
+# fail.
+
+# CHECK: Contents of section producers:
+# CHECK: Contents of section reloc.CODE:
+
+--- !WASM
+FileHeader:
+  Version:         0x00000001
+Sections:
+  - Type:            TYPE
+    Signatures:
+      - Index:           0
+        ReturnType:      NORESULT
+        ParamTypes:      []
+  - Type:            IMPORT
+    Imports:
+      - Module:          env
+        Field:           __linear_memory
+        Kind:            MEMORY
+        Memory:
+          Initial:         0x00000000
+      - Module:          env
+        Field:           __indirect_function_table
+        Kind:            TABLE
+        Table:
+          ElemType:        FUNCREF
+          Limits:
+            Initial:         0x00000000
+  - Type:            FUNCTION
+    FunctionTypes:   [ 0 ]
+  - Type:            CODE
+    Relocations:
+      - Type:            R_WASM_FUNCTION_INDEX_LEB
+        Index:           0
+        Offset:          0x00000004
+    Functions:
+      - Index:           0
+        Locals:          []
+        Body:            1080808080000B
+  - Type:            CUSTOM
+    Name:            linking
+    Version:         2
+    SymbolTable:
+      - Index:           0
+        Kind:            FUNCTION
+        Name:            foo
+        Flags:           [ VISIBILITY_HIDDEN ]
+        Function:        0
+  - Type:            CUSTOM
+    Name:            producers
+    Tools:
+      - Name:            clang
+        Version:         9.0.0
+...