must check for zero-length in reader32::read() before calling memcpy

memcpy's behavior is undefined if the dst-ptr is null, but reader32 supports
null as long as the size is 0, so it needs to check explicitly before
calling memcpy. This is implemented (now) by calling sk_careful_memcpy.

BUG=skia:

Change-Id: I7033cc5e6d724f50f0aafd9808e297b953848aa7
Reviewed-on: https://skia-review.googlesource.com/9729
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Reed <reed@google.com>

diff --git a/src/core/SkReader32.h b/src/core/SkReader32.h
index 7e31fb9..1f027f7 100644 (file)
--- a/src/core/SkReader32.h
+++ b/src/core/SkReader32.h
@@ -96,7 +96,7 @@ public:
     void read(void* dst, size_t size) {
         SkASSERT(0 == size || dst != nullptr);
         SkASSERT(ptr_align_4(fCurr));
-        memcpy(dst, fCurr, size);
+        sk_careful_memcpy(dst, fCurr, size);
         fCurr += SkAlign4(size);
         SkASSERT(fCurr <= fStop);
     }

diff --git a/tests/Reader32Test.cpp b/tests/Reader32Test.cpp
index c49e57c..301f67c 100644 (file)
--- a/tests/Reader32Test.cpp
+++ b/tests/Reader32Test.cpp
@@ -78,4 +78,13 @@ DEF_TEST(Reader32, reporter) {
     assert_empty(reporter, reader);
     REPORTER_ASSERT(reporter, nullptr == reader.base());
     REPORTER_ASSERT(reporter, nullptr == reader.peek());
+
+    // need to handle read(null, 0) and not get undefined behavior from memcpy
+    {
+        char storage[100];
+        reader.setMemory(storage, sizeof(storage));
+        char buffer[10];
+        reader.read(buffer, 0);     // easy case, since we pass a ptr
+        reader.read(nullptr, 0);    // undef case, read() can't blindly call memcpy
+    }
 }