[Zucchini] Fix BufferRegion::FitsIn() so empty region fits at end of buffer.

This CL is similar to:
https://chromium-review.googlesource.com/1133688

BufferRegion::FitsIn() (and BufferViewBase::covers()) decides whether
a BufferRegion fits inside a buffer. A special case is whether an empty
region fits at the end of a buffer?

Previously this was considered to be a pathological case, so the result
is "false". However, this led to a DCHECK failure found by the DEX
fuzzer: a CodeItem with insns_size = 0 is checked against an empty
buffer.

It may seem straightforward to change the DCHECK to a handled failure.
However, the failing code (in CodeItemParser::GetCodeItemInsns())
occurs after CodeItem have been supposedly validated, so the DCHECK
is correctly placed! Two causes are:
(1) Technically insns_size should be > 0, as dictated by constraint A1
    ("The insns array mus tnot be empty") in Dalvik spec.
(2) The FitsIn() check is too stringent.

This CL focuses on relaxing (2). This makes checking slightly more
permissive elsewhere in code (patch_reader.cc and Win32 disassembler),
but this looks like the right thing to do.

As for (1), we plan to visit
https://source.android.com/devices/tech/dalvik/constraints
and implement more rigorous checks. So we simply add a TODO for now.

Bug: 863478
Change-Id: Iacbb2bb9bf26701db960192c7b727351ea5afdec
Reviewed-on: https://chromium-review.googlesource.com/1142517Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576482}

components/zucchini/buffer_view.h

@@ -24,9 +24,9 @@ struct BufferRegion {
   size_t hi() const { return offset + size; }
 
   // Returns whether the Region fits in |[0, container_size)|. Special case:
-  // a size-0 region starting at |container_size| does not fit.
+  // a size-0 region starting at |container_size| fits.
   bool FitsIn(size_t container_size) const {
-    return offset < container_size && container_size - offset >= size;
+    return offset <= container_size && container_size - offset >= size;
   }
 
   // Returns |v| clipped to the inclusive range |[lo(), hi()]|.

components/zucchini/buffer_view_unittest.cc

@@ -154,7 +154,7 @@ TEST_F(BufferViewTest, LocalRegion) {
 }
 
 TEST_F(BufferViewTest, Covers) {
-  EXPECT_FALSE(ConstBufferView().covers({0, 0}));
+  EXPECT_TRUE(ConstBufferView().covers({0, 0}));
   EXPECT_FALSE(ConstBufferView().covers({0, 1}));
 
   ConstBufferView view(bytes_.data(), bytes_.size());
@@ -168,8 +168,10 @@ TEST_F(BufferViewTest, Covers) {
   EXPECT_TRUE(view.covers({bytes_.size() - 1, 0}));
   EXPECT_TRUE(view.covers({bytes_.size() - 1, 1}));
   EXPECT_FALSE(view.covers({bytes_.size() - 1, 2}));
-  EXPECT_FALSE(view.covers({bytes_.size(), 0}));
+  EXPECT_TRUE(view.covers({bytes_.size(), 0}));
   EXPECT_FALSE(view.covers({bytes_.size(), 1}));
+  EXPECT_FALSE(view.covers({bytes_.size() + 1, 0}));
+  EXPECT_FALSE(view.covers({bytes_.size() + 1, 1}));
 
   EXPECT_FALSE(view.covers({1, size_t(-1)}));
   EXPECT_FALSE(view.covers({size_t(-1), 1}));

components/zucchini/disassembler_dex.cc

@@ -162,6 +162,7 @@ class CodeItemParser {
       return kInvalidOffset;
     DCHECK(Is32BitAligned(code_item_offset));
 
+    // TODO(huangs): Fail if |code_item->insns_size == 0| (Constraint A1).
     // Skip instruction bytes.
     if (!source_.GetArray<uint16_t>(code_item->insns_size))
       return kInvalidOffset;