[wasm] Add support for 64-bit LEB encodings.

src/wasm/decoder.h

Index: src/wasm/decoder.h
diff --git a/src/wasm/decoder.h b/src/wasm/decoder.h
index 2dfdd02aed912387d75b4c31cef0656a26ca5263..590eb2a34c5ee7095e0cb1880186391304a74fc0 100644
--- a/src/wasm/decoder.h
+++ b/src/wasm/decoder.h
@@ -77,44 +77,15 @@ class Decoder {
     return check(base, offset, 8, msg) ? read_u64(base + offset) : 0;
   }
 
+  // Reads a variable-length unsigned integer (little endian).
   uint32_t checked_read_u32v(const byte* base, int offset, int* length,
-                             const char* msg = "expected LEB128") {
-    if (!check(base, offset, 1, msg)) {
-      *length = 0;
-      return 0;
-    }
-
-    const ptrdiff_t kMaxDiff = 5;  // maximum 5 bytes.
-    const byte* ptr = base + offset;
-    const byte* end = ptr + kMaxDiff;
-    if (end > limit_) end = limit_;
-    int shift = 0;
-    byte b = 0;
-    uint32_t result = 0;
-    while (ptr < end) {
-      b = *ptr++;
-      result = result | ((b & 0x7F) << shift);
-      if ((b & 0x80) == 0) break;
-      shift += 7;
-    }
-    DCHECK_LE(ptr - (base + offset), kMaxDiff);
-    *length = static_cast<int>(ptr - (base + offset));
-    if (ptr == end) {
-      if (*length == kMaxDiff && (b & 0xF0) != 0) {
-        error(base, ptr, "extra bits in LEB128");
-        return 0;
-      }
-      if ((b & 0x80) != 0) {
-        error(base, ptr, msg);
-        return 0;
-      }
-    }
-    return result;
+                             const char* msg = "expected LEB32") {
+    return checked_read_leb<uint32_t>(base, offset, length, msg);
   }
 
   // Reads a variable-length signed integer (little endian).
   int32_t checked_read_i32v(const byte* base, int offset, int* length,
-                            const char* msg = "expected SLEB128") {
+                            const char* msg = "expected SLEB32") {
     uint32_t result = checked_read_u32v(base, offset, length, msg);
     if (*length == 5) return bit_cast<int32_t>(result);
     if (*length > 0) {
@@ -124,6 +95,24 @@ class Decoder {
     return 0;
   }
 
+  // Reads a variable-length unsigned integer (little endian).
+  uint64_t checked_read_u64v(const byte* base, int offset, int* length,
+                             const char* msg = "expected LEB64") {
+    return checked_read_leb<uint64_t>(base, offset, length, msg);
+  }
+
+  // Reads a variable-length signed integer (little endian).
+  int64_t checked_read_i64v(const byte* base, int offset, int* length,
+                            const char* msg = "expected SLEB64") {
+    uint64_t result = checked_read_u64v(base, offset, length, msg);
+    if (*length == 10) return bit_cast<int64_t>(result);
+    if (*length > 0) {
+      int shift = 64 - 7 * *length;
+      return bit_cast<int64_t>(result << shift) >> shift;

[ahaas, 2016/03/01 08:22:58]

Could you add a comment to explain this line?
Something like "// Use arithmetic shift to get sign extension."

[titzer, 2016/03/01 18:49:12]

Done.

+    }
+    return 0;
+  }
+
   // Reads a single 16-bit unsigned integer (little endian).
   inline uint16_t read_u16(const byte* ptr) {
     DCHECK(ptr >= start_ && (ptr + 2) <= end_);
@@ -333,6 +322,45 @@ class Decoder {
   const byte* error_pc_;
   const byte* error_pt_;
   base::SmartArrayPointer<char> error_msg_;
+
+ private:
+  template <typename IntType>
+  IntType checked_read_leb(const byte* base, int offset, int* length,
+                           const char* msg) {
+    if (!check(base, offset, 1, msg)) {
+      *length = 0;
+      return 0;
+    }
+
+    const int kMaxDiff = (sizeof(IntType) * 8 + 6) / 7;

[ahaas, 2016/03/01 08:22:58]

Should this be "kMaxLength"? It is the maximum value of length, is it not?

[titzer, 2016/03/01 18:49:12]

Done.

+    const byte* ptr = base + offset;
+    const byte* end = ptr + kMaxDiff;

[binji, 2016/03/01 18:49:40]

I don't think this is right if we allow non-minimal LEB (which was suggested in
the discussion)

[titzer, 2016/03/01 19:09:19]

I don't think we should support arbitrary-length LEB where stuff is expected to
be 32 or 64 bits, so making the cutoff 5 or 10 bytes respectively seems OK.

+    if (end > limit_) end = limit_;
+    int shift = 0;
+    byte b = 0;
+    IntType result = 0;
+    while (ptr < end) {
+      b = *ptr++;
+      result = result | (static_cast<IntType>(b & 0x7F) << shift);
+      if ((b & 0x80) == 0) break;
+      shift += 7;
+    }
+    DCHECK_LE(ptr - (base + offset), kMaxDiff);
+    *length = static_cast<int>(ptr - (base + offset));
+    if (ptr == end) {
+      const int kExtraBits = (1 + kMaxDiff * 7) - (sizeof(IntType) * 8);

[ahaas, 2016/03/01 08:22:58]

Could you add a comment here, to hint what this extra bits stuff is about?

[titzer, 2016/03/01 18:49:12]

Done.

[binji, 2016/03/01 18:49:40]

Yeah, it took me walking through it to understand the purpose of the +1, it'd be
nice to have a comment. :)

+      const byte kExtraBitsMask = static_cast<byte>(0xFF << (8 - kExtraBits));
+      if (*length == kMaxDiff && (b & kExtraBitsMask) != 0) {
+        error(base, ptr, "extra bits in varint");
+        return 0;
+      }
+      if ((b & 0x80) != 0) {
+        error(base, ptr, msg);
+        return 0;
+      }
+    }
+    return result;
+  }
 };
 
 #undef TRACE