aboutsummaryrefslogtreecommitdiff
path: root/buffer_view_unittest.cc
diff options
context:
space:
mode:
authorSamuel Huang <huangs@chromium.org>2018-07-19 14:32:28 +0000
committerCopybara-Service <copybara-worker@google.com>2021-07-25 20:31:52 -0700
commitd758cb7bfc3f9394f8e96a8e14d01af7b0c388b4 (patch)
tree4ec9b0ff5476eef415550672ddd80675d03a9d4f /buffer_view_unittest.cc
parent06a9be1c70878cf651b70192275187bf9a8662a9 (diff)
downloadzucchini-d758cb7bfc3f9394f8e96a8e14d01af7b0c388b4.tar.gz
[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/1142517 Reviewed-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} NOKEYCHECK=True GitOrigin-RevId: 2b31de169e783260c9e2fbaea295b39ae808fbf9
Diffstat (limited to 'buffer_view_unittest.cc')
-rw-r--r--buffer_view_unittest.cc6
1 files changed, 4 insertions, 2 deletions
diff --git a/buffer_view_unittest.cc b/buffer_view_unittest.cc
index b048747..7df34b2 100644
--- a/buffer_view_unittest.cc
+++ b/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}));