Add sk_careful_memcpy to catch undefined behavior in memcpy.

It's undefined behavior to pass null as src or dst to memcpy, even if len is 0.
This currently triggers -fsanitize=attribute-nonnull warnings, but also can
lead to very unexpected code generation with GCC.

sk_careful_memcpy() checks len first before calling memcpy(),
which prevents that weird undefined situation.

This allows me to mark all sanitizers as no-recover, i.e. make-the-bots-red fatal.

CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-ASAN-Trybot

BUG=skia:4641
NOTREECHECKS=true

Review URL: https://codereview.chromium.org/1510683002

1 files changed, 22 insertions, 0 deletions

diff --git a/include/core/SkTypes.h b/include/core/SkTypes.h
index 5720c30970..6c2e636153 100644
--- a/include/core/SkTypes.h
+++ b/include/core/SkTypes.h
@@ -24,6 +24,28 @@
 
 #include <string.h>
 
+/**
+ *  sk_careful_memcpy() is just like memcpy(), but guards against undefined behavior.
+ *
+ * It is undefined behavior to call memcpy() with null dst or src, even if len is 0.
+ * If an optimizer is "smart" enough, it can exploit this to do unexpected things.
+ *     memcpy(dst, src, 0);
+ *     if (src) {
+ *         printf("%x\n", *src);
+ *     }
+ * In this code the compiler can assume src is not null and omit the if (src) {...} check,
+ * unconditionally running the printf, crashing the program if src really is null.
+ * Of the compilers we pay attention to only GCC performs this optimization in practice.
+ */
+static inline void* sk_careful_memcpy(void* dst, const void* src, size_t len) {
+    // When we pass >0 len we had better already be passing valid pointers.
+    // So we just need to skip calling memcpy when len == 0.
+    if (len) {
+        memcpy(dst,src,len);
+    }
+    return dst;
+}
+
 /** \file SkTypes.h
 */