fix off-by-one bugs that could cause canonicalization to change the value of a message

c++/src/capnp/canonicalize-test.c++

@@ -37,8 +37,14 @@ KJ_TEST("canonicalize yields canonical message") {
 
   initTestMessage(root);
 
-  canonicalize(root.asReader());
-  //Will assert if canonicalize failed to do so
+  auto canonicalWords = canonicalize(root.asReader());
+  // Throws an exception on canonicalization failure.
+
+  kj::ArrayPtr<const capnp::word> canonicalSegments[1] = {canonicalWords.asPtr()};
+  capnp::SegmentArrayMessageReader canonicalReader(kj::arrayPtr(canonicalSegments, 1));
+
+  KJ_ASSERT(AnyStruct::Reader(root.asReader()) ==
+            AnyStruct::Reader(canonicalReader.getRoot<TestAllTypes>()));
 }
 
 KJ_TEST("canonicalize succeeds on empty struct") {
@@ -48,6 +54,56 @@ KJ_TEST("canonicalize succeeds on empty struct") {
   canonicalize(root.asReader());  // Throws an exception on canoncalization failure.
 }
 
+KJ_TEST("data word with only its most significant bit set does not get truncated") {
+  AlignedData<3> segment = {{
+    // Struct pointer, body immediately follows, two data words
+    0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+
+    // First data word
+    0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11,
+
+    // Second data word, all zero except most significant bit
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80,
+  }};
+  kj::ArrayPtr<const word> segments[1] = {kj::arrayPtr(segment.words, 3)};
+  SegmentArrayMessageReader messageReader(kj::arrayPtr(segments, 1));
+
+  KJ_ASSERT(messageReader.isCanonical());
+  auto canonicalWords = canonicalize(messageReader.getRoot<TestAllTypes>());
+
+  // At one point this failed because an off-by-one bug in canonicalization
+  // caused the second word of the data section to be truncated.
+  ASSERT_EQ(canonicalWords.asBytes(), kj::arrayPtr(segment.bytes, 3 * 8));
+}
+
+KJ_TEST("INLINE_COMPOSITE data word with only its most significant bit set does not get truncated") {
+  AlignedData<5> segment = {{
+    // Struct pointer, body immediately follows, one pointer
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00,
+
+    // List pointer, no offset, inline composite, two words long
+    0x01, 0x00, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00,
+
+    // Tag word, list has one element with two data words and no pointers
+    0x04, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00,
+
+    // First data word
+    0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22, 0x22,
+
+    // Second data word, all zero except most significant bit
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80,
+  }};
+  kj::ArrayPtr<const word> segments[1] = {kj::arrayPtr(segment.words, 5)};
+  SegmentArrayMessageReader messageReader(kj::arrayPtr(segments, 1));
+
+  KJ_ASSERT(messageReader.isCanonical());
+  auto canonicalWords = canonicalize(messageReader.getRoot<TestLists>());
+
+  // At one point this failed because an off-by-one bug in canonicalization
+  // caused the second word of the data section to be truncated.
+  ASSERT_EQ(canonicalWords.asBytes(), kj::arrayPtr(segment.bytes, 5 * 8));
+}
+
 KJ_TEST("canonical non-null empty struct field") {
   AlignedData<4> nonNullEmptyStruct = {{
     // Struct pointer, body immediately follows, two pointer fields, no data.

c++/src/capnp/layout.c++

@@ -1571,12 +1571,12 @@ struct WireHelpers {
       } else {
         // Truncate the data section
         while (dataSize != 0 * BYTES) {
-          size_t end = (dataSize - 1 * BYTES) / BYTES;
+          size_t end = dataSize / BYTES;
           ByteCount window = dataSize % BYTES_PER_WORD;
           if (window == 0 * BYTES) {
             window = BYTES_PER_WORD * WORDS;
           }
-          size_t start = end + 1 - window / BYTES;
+          size_t start = end - window / BYTES;
           kj::ArrayPtr<const byte> lastWord = value.getDataSectionAsBlob().slice(start, end);
           bool lastWordZero = true;
           //TODO(MRM) once this is known to work, replace with fast memcmp
@@ -1683,9 +1683,9 @@ struct WireHelpers {
           auto se = value.getStructElement(ec);
           WordCount localDataSize = declDataSize;
           while (localDataSize != 0 * WORDS) {
-            size_t end = (localDataSize * BYTES_PER_WORD - 1 * BYTES) / BYTES;
+            size_t end = (localDataSize * BYTES_PER_WORD * BYTES) / BYTES;
             ByteCount window = BYTES_PER_WORD * WORDS;
-            size_t start = end + 1 - window / BYTES;
+            size_t start = end - window / BYTES;
             kj::ArrayPtr<const byte> lastWord = se.getDataSectionAsBlob().slice(start, end);
             bool lastWordZero = true;
             //TODO(MRM) once this is known to work, replace with fast memcmp