Subzero. Wasm. Implement sbrk and correctly do bounds checks.

fetch-torture-tests.sh

 #!/bin/bash
 
-BUILDID=5430
+BUILDID=5528
 BUILD_PATH=https://storage.googleapis.com/wasm-llvm/builds/git
 
-wget -O - /wasm-torture-s-$BUILDID.tbz2 \
+wget -O - $BUILD_PATH/wasm-torture-s-$BUILDID.tbz2 \
   | tar xj
 
 wget -O - $BUILD_PATH/wasm-torture-s2wasm-sexpr-wasm-$BUILDID.tbz2 \

pydir/wasm-run-torture-tests.py

@@ -21,25 +21,8 @@ import sys
 import threading
 
 IGNORED_TESTS = set([
-  'loop-2f.c.wasm',       # mmap not in MVP
-  'loop-2g.c.wasm',       # mmap not in MVP
-
-  '960521-1.c.wasm',      # sbrk
-  'ipa-sra-2.c.wasm',     # sbrk
-  'pr41463.c.wasm',       # sbrk
-  '20051113-1.c.wasm',    # sbrk
-  '990628-1.c.wasm',      # sbrk
-  'pr41395-2.c.wasm',     # sbrk
-  'pr42614.c.wasm',       # sbrk
-  'pr41395-1.c.wasm',     # sbrk
-  '920810-1.c.wasm',      # sbrk
-  '20000914-1.c.wasm',    # sbrk
-  'pr15262-1.c.wasm',     # sbrk
-  '941014-2.c.wasm',      # sbrk
-  'va-arg-21.c.wasm',     # sbrk
-  '20020406-1.c.wasm',    # sbrk
-
-  # waterfall known failures
+  # The remaining tests are known waterfall failures
+
   '20010122-1.c.wasm',
   '20031003-1.c.wasm',
   '20071018-1.c.wasm',

runtime/wasm-runtime.cpp

@@ -12,13 +12,27 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <cassert>
 #include <cmath>
+// TODO (eholk): change to cstdint
+#include <stdint.h>
+
+namespace {
+uint32_t HeapBreak;
+
+// TODO (eholk): make all of these constexpr.
+const uint32_t PageSizeLog2 = 16;
+const uint32_t PageSize = 1 << PageSizeLog2; // 64KB
+const uint32_t StackPtrLoc = 1024;           // defined by emscripten
+
+uint32_t pageNum(uint32_t Index) { return Index >> PageSizeLog2; }
+} // end of anonymous namespace
 
 namespace env {
 double floor(double X) { return std::floor(X); }
 
 float floor(float X) { return std::floor(X); }
-}
+} // end of namespace env
 
 // TODO (eholk): move the C parts outside and use C++ name mangling.
 extern "C" {
@@ -33,6 +47,8 @@ extern "C" {
 #include <unistd.h>
 
 extern char WASM_MEMORY[];
+extern uint32_t WASM_DATA_SIZE;
+extern uint32_t WASM_NUM_PAGES;
 
 void env$$abort() {
   fprintf(stderr, "Aborting...\n");
@@ -53,7 +69,11 @@ void env$$_exit(int Status) { env$$exit(Status); }
     abort();                                                                   \
   }
 
-UNIMPLEMENTED(sbrk)
+int32_t env$$sbrk(int32_t Increment) {
+  HeapBreak += Increment;
+  return HeapBreak;
+}
+
 UNIMPLEMENTED(setjmp)
 UNIMPLEMENTED(longjmp)
 UNIMPLEMENTED(__assert_fail)
@@ -83,18 +103,16 @@ UNIMPLEMENTED(__lock)
 UNIMPLEMENTED(__unlock)
 UNIMPLEMENTED(__syscall6)   // sys_close
 UNIMPLEMENTED(__syscall140) // sys_llseek
-UNIMPLEMENTED(__syscall192) // sys_mmap?
 UNIMPLEMENTED(__unordtf2)
 UNIMPLEMENTED(__fixunstfsi)
 UNIMPLEMENTED(__floatunsitf)
 UNIMPLEMENTED(__extenddftf2)
 
-void *wasmPtr(int Index) {
-  // TODO (eholk): get the mask from the WASM file.
-  const int MASK = 0xffffff;
-  Index &= MASK;
-
+void *wasmPtr(uint32_t Index) {
+  if (pageNum(Index) < WASM_NUM_PAGES) {
     return WASM_MEMORY + Index;
+  }
+  abort();
 }
 
 extern int __szwasm_main(int, const char **);
@@ -102,7 +120,15 @@ extern int __szwasm_main(int, const char **);
 #define WASM_REF(Type, Index) ((Type *)wasmPtr(Index))
 #define WASM_DEREF(Type, Index) (*WASM_REF(Type, Index))
 
-int main(int argc, const char **argv) { return __szwasm_main(argc, argv); }
+int main(int argc, const char **argv) {
+  // Initialize the break to the nearest page boundary after the data segment
+  HeapBreak = (WASM_DATA_SIZE + PageSize - 1) & ~(PageSize - 1);
+
+  // Initialize the stack pointer.
+  WASM_DEREF(int32_t, StackPtrLoc) = WASM_NUM_PAGES << PageSizeLog2;
+
+  return __szwasm_main(argc, argv);
+}
 
 int env$$abs(int a) { return abs(a); }
 
@@ -168,4 +194,14 @@ int env$$__syscall146(int Which, int VarArgs) {
   }
   return Count;
 }
+
+/// sys_mmap_pgoff
+int env$$__syscall192(int Which, int VarArgs) {
+  (void)Which;
+  (void)VarArgs;
+
+  // TODO (eholk): figure out how to implement this.
+
+  return -ENOMEM;
+}
 } // end of extern "C"

src/WasmTranslator.cpp

@@ -1065,9 +1065,7 @@ public:
     assert(Module);
     const auto &IndirectTable = Module->function_table;
 
-    // TODO(eholk): This should probably actually call abort instead.
-    auto *Abort = Func->makeNode();
-    Abort->appendInst(InstUnreachable::create(Func));
+    auto *Abort = getAbortTarget();
 
     assert(Args[0].toOperand());
 
@@ -1137,14 +1135,23 @@ public:
 
   Operand *sanitizeAddress(Operand *Base, uint32_t Offset) {
     SizeT MemSize = Module->module->min_mem_pages * WASM_PAGE_SIZE;
-    SizeT MemMask = MemSize - 1;
 
     bool ConstZeroBase = false;
 
     // first, add the index and the offset together.
     if (auto *ConstBase = llvm::dyn_cast<ConstantInteger32>(Base)) {
       uint32_t RealOffset = Offset + ConstBase->getValue();
-      RealOffset &= MemMask;
+      if (RealOffset >= MemSize) {
+        // We've proven this will always be an out of bounds access, so insert
+        // an unconditional trap.
+        Control()->appendInst(InstUnreachable::create(Func));
+        // It doesn't matter what we return here, so return something that will
+        // allow the rest of code generation to happen.
+        //
+        // We might be tempted to just abort translation here, but out of bounds
+        // memory access is a runtime trap, not a compile error.
+        return Ctx->getConstantZero(getPointerType());
+      }
       Base = Ctx->getConstantInt32(RealOffset);
       ConstZeroBase = (0 == RealOffset);
     } else if (0 != Offset) {
@@ -1156,12 +1163,25 @@ public:
       Base = Addr;
     }
 
+    // Do the bounds check.
+    //
+    // TODO (eholk): Add a command line argument to control whether bounds
+    // checks are inserted, and maybe add a way to duplicate bounds checks to
+    // get a better sense of the overhead.
     if (!llvm::dyn_cast<ConstantInteger32>(Base)) {
-      auto *ClampedAddr = makeVariable(Ice::getPointerType());
+      // TODO (eholk): creating a new basic block on every memory access is
+      // terrible (see https://goo.gl/Zj7DTr). Try adding a new instruction that
+      // encapsulates this "abort if false" pattern.
+      auto *CheckPassed = Func->makeNode();
+      auto *CheckFailed = getAbortTarget();
+
+      auto *Check = makeVariable(IceType_i1);
+      Control()->appendInst(InstIcmp::create(Func, InstIcmp::Ult, Check, Base,
+                                             Ctx->getConstantInt32(MemSize)));
       Control()->appendInst(
-          InstArithmetic::create(Func, InstArithmetic::And, ClampedAddr, Base,
-                                 Ctx->getConstantInt32(MemSize - 1)));
-      Base = ClampedAddr;
+          InstBr::create(Func, Check, CheckPassed, CheckFailed));
+
+      *ControlPtr = OperandNode(CheckPassed);
     }
 
     Ice::Operand *RealAddr = nullptr;
@@ -1261,6 +1281,8 @@ private:
   class Cfg *Func;
   GlobalContext *Ctx;
 
+  CfgNode *AbortTarget = nullptr;
+
   SizeT NextArg = 0;
 
   CfgUnorderedMap<Operand *, InstPhi *> PhiMap;
@@ -1297,6 +1319,18 @@ private:
     return Iter->second;
   }
 
+  CfgNode *getAbortTarget() {
+    if (!AbortTarget) {
+      // TODO (eholk): Move this node to the end of the CFG, or even better,
+      // have only one abort block for the whole module.
+      AbortTarget = Func->makeNode();
+      // TODO (eholk): This should probably actually call abort instead.
+      AbortTarget->appendInst(InstUnreachable::create(Func));
+    }
+
+    return AbortTarget;
+  }
+
   template <typename F = std::function<void(Ostream &)>> void log(F Fn) const {
     if (BuildDefs::dump() && (getFlags().getVerbose() & IceV_Wasm)) {
       Fn(Ctx->getStrDump());
@@ -1470,6 +1504,14 @@ void WasmTranslator::translate(
       WritePtr += Seg.source_size;
     }
 
+    // Save the size of the initialized data in a global variable so the runtime
+    // can use it to determine the initial heap break.
+    auto *GlobalDataSize = VariableDeclaration::createExternal(Globals.get());
+    GlobalDataSize->setName(Ctx->getGlobalString("WASM_DATA_SIZE"));
+    GlobalDataSize->addInitializer(VariableDeclaration::DataInitializer::create(
+        Globals.get(), reinterpret_cast<const char *>(&WritePtr),
+        sizeof(WritePtr)));
+
     // Pad the rest with zeros
     SizeT DataSize = Module->min_mem_pages * WASM_PAGE_SIZE;
     if (WritePtr < DataSize) {
@@ -1477,7 +1519,16 @@ void WasmTranslator::translate(
           Globals.get(), DataSize - WritePtr));
     }
 
+    // Save the number of pages for the runtime
+    auto *GlobalNumPages = VariableDeclaration::createExternal(Globals.get());
+    GlobalNumPages->setName(Ctx->getGlobalString("WASM_NUM_PAGES"));
+    GlobalNumPages->addInitializer(VariableDeclaration::DataInitializer::create(
+        Globals.get(), reinterpret_cast<const char *>(&Module->min_mem_pages),
+        sizeof(Module->min_mem_pages)));
+
     Globals->push_back(WasmMemory);
+    Globals->push_back(GlobalDataSize);
+    Globals->push_back(GlobalNumPages);
 
     lowerGlobals(std::move(Globals));
   }