Commit 68dd310e authored by Adrienne Walker's avatar Adrienne Walker Committed by Commit Bot

cc: Make paint_op_buffer_eq_fuzzer not binary compare

This test case has a DrawRecord in the middle of it.  There was some
hope previously that because this fuzzer memset all the memory it
was allocating, then all the unwritten memory could be consistent.
However, because a bunch of other memory gets allocated when serializing
then that memory can have garbage in the padding.  When simple ops
memcpy as a part of serializing or deserializing, they bring along
that garbage, which can cause binary differences.

This was lucky that this has ever worked on Skia serialization, so
remove the binary comparison and just use the logical comparison.

Bug: 870647
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie20097c4df4953bb6503caff81ad811dc69acc63
Reviewed-on: https://chromium-review.googlesource.com/1162746Reviewed-by: default avatarKhushal <khushalsagar@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581265}
parent 3db08a94
...@@ -21,18 +21,19 @@ ...@@ -21,18 +21,19 @@
// //
// serialized1 -> deserialized1 -> serialized2 -> deserialized2 -> serialized3 // serialized1 -> deserialized1 -> serialized2 -> deserialized2 -> serialized3
// //
// It does a binary comparison that serialized2 == serialized3
// Ideally this test would compare serialized1 to serialized2, however: // Ideally this test would compare serialized1 to serialized2, however:
// (1) Deserializing is a destructive process on bad input, e.g. SkMatrix // (1) Deserializing is a destructive process on bad input, e.g. SkMatrix
// that says it is identity will be clobbered to have identity values. // that says it is identity will be clobbered to have identity values.
// (2) Padding for alignment is skipped and so serialized1 may have garbage. // (2) Padding for alignment is skipped and so serialized1 may have garbage.
// serialized2 and serialized3 are cleared to zero first. // serialized2 and serialized3 are cleared to zero first.
// (3) Any internal allocated memory (e.g. DrawRecord ops) also will not
// be initialized to zero, and some ops memcpy when serializing or
// deserializing, and may copy some of this garbage.
// //
// Binary comparing serialized2 to serialized3 is not identical to comparing // It'd be nice to be able to binary compare, but because of all the above
// serialized1 to serialized2, as this could overlook some bugs that clobbered // reasons, this is impossible to do for all ops, so this test only does
// object state to something that serialized cleanly at that point. // a logical comparison of deserialized1 and deserialized2, and verifies
// To mitigate those errors, this test also compares the logical equality // that serialized2 and serialized3 wrote the exact same number of bytes.
// deserialized1 and deserialized2 using PaintOp::operator==.
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
const size_t kMaxSerializedSize = 1000000; const size_t kMaxSerializedSize = 1000000;
...@@ -115,7 +116,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { ...@@ -115,7 +116,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
CHECK(*deserialized_op1 == *deserialized_op2) CHECK(*deserialized_op1 == *deserialized_op2)
<< "\n1: " << cc::PaintOpHelper::ToString(deserialized_op1) << "\n1: " << cc::PaintOpHelper::ToString(deserialized_op1)
<< "\n2: " << cc::PaintOpHelper::ToString(deserialized_op2); << "\n2: " << cc::PaintOpHelper::ToString(deserialized_op2);
CHECK_EQ(0, memcmp(serialized2.get(), serialized3.get(), written_bytes2));
deserialized_op1->DestroyThis(); deserialized_op1->DestroyThis();
deserialized_op2->DestroyThis(); deserialized_op2->DestroyThis();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment