Use safer r_memcpy() and r_memset() everywhere (#1797)

* Introduce `r_memcpy()` and `r_memset()`

* Use `r_memcpy()` and `r_memset()`

* NEWS bullet

Author: DavisVaughan

NEWS.md

@@ -1,5 +1,9 @@
 # rlang (development version)
 
+* C code no longer calls `memcpy()` and `memset()` on 0-length R object memory
+  (#1797).
+
+
 # rlang 1.1.6
 
 * Fixes for CRAN checks.

src/internal/cnd.c

@@ -41,7 +41,7 @@ const char* rlang_format_error_arg(r_obj* arg) {
 
   // Uses the vmax protection stack.
   char* out = R_alloc(n, sizeof(char));
-  memcpy(out, arg_str, n);
+  r_memcpy(out, arg_str, n);
 
   FREE(1);
   return out;
@@ -216,7 +216,7 @@ const char* rlang_obj_type_friendly_full(r_obj* x, bool value, bool _length) {
   // Uses the vmax protection stack.
   int n = strlen(out_str) + 1;
   char* out = R_alloc(n, sizeof(char));
-  memcpy(out, out_str, n);
+  r_memcpy(out, out_str, n);
 
   FREE(1);
   return out;

src/internal/hash.c

@@ -225,7 +225,7 @@ void hash_skip(struct hash_state_t* p_state, void* p_input, int n) {
   if (p_state->n_skipped == N_BYTES_SERIALIZATION_INFO) {
     // We've skipped all serialization info bytes.
     // Incoming bytes tell the size of the native encoding string.
-    memcpy(&p_state->n_native_enc, p_input, sizeof(int));
+    r_memcpy(&p_state->n_native_enc, p_input, sizeof(int));
     p_state->n_skipped += n;
     return;
   }

src/internal/names.c

@@ -65,7 +65,7 @@ r_obj* names_as_unique(r_obj* names, bool quiet) {
     char buf[buf_size];
     buf[0] = '\0';
 
-    memcpy(buf, name, size);
+    r_memcpy(buf, name, size);
     int remaining = buf_size - size;
 
     int needed = snprintf(buf + size,

src/internal/sym-unescape.c

@@ -86,7 +86,7 @@ r_obj* str_unserialise_unicode(r_obj* r_string) {
     // Need to check first if the string has any UTF-8 escapes.
     int orig_len = strlen(re_enc);
     char tmp[orig_len + 1];
-    memcpy(tmp, re_enc, orig_len + 1);
+    r_memcpy(tmp, re_enc, orig_len + 1);
     return unescape_char_to_sexp(tmp);
   } else {
     // The string has been copied so it's safe to use as buffer

src/internal/vec-raw.c

@@ -33,7 +33,7 @@ r_obj* ffi_raw_deparse_str(r_obj* x, r_obj* prefix, r_obj* suffix) {
   r_obj* buf = KEEP(r_alloc_raw(len));
   char* p_buf = (char*) r_raw_begin(buf);
 
-  memcpy(p_buf, s_prefix, len_prefix);
+  r_memcpy(p_buf, s_prefix, len_prefix);
   p_buf += len_prefix;
 
   const char* lookup = "0123456789abcdef";
@@ -44,7 +44,7 @@ r_obj* ffi_raw_deparse_str(r_obj* x, r_obj* prefix, r_obj* suffix) {
     *p_buf++ = lookup[value % 16];
   }
 
-  memcpy(p_buf, s_suffix, len_suffix);
+  r_memcpy(p_buf, s_suffix, len_suffix);
   p_buf += len_suffix;
 
   // Invariant: p_buf == r_raw_begin(buf) + len

src/rlang/c-utils.h

@@ -8,6 +8,7 @@
 #include <inttypes.h>
 #include <math.h>
 #include <float.h>
+#include <string.h>
 #include "cnd.h"
 
 #define R_ARR_SIZEOF(X) sizeof(X) / sizeof(X[0])
@@ -164,4 +165,32 @@ double r_double_mult(double x, double y) {
   return out;
 }
 
+// Slightly safer version of `memcpy()` for use with R object memory
+//
+// Prefer this over `memcpy()`, especially when providing pointers to R object
+// memory. As of R 4.5.0, `DATAPTR()` and friends return `(void*) 1` on 0-length
+// R objects, so we must be extremely careful to never use dereference those
+// pointers. In particular, it is not safe to call `memcpy(dest, src, 0)` on
+// some machines (likely with sanitizers active) when either `dest` or `src`
+// resolve to `(void*) 1`.
+//
+// https://github.com/r-lib/vctrs/pull/1968
+// https://github.com/r-devel/r-svn/blob/9976c3d7f08c754593d01ba8380afb6be803dde2/src/main/memory.c#L4137-L4150
+static inline
+void r_memcpy(void* dest, const void* src, size_t count) {
+  if (count) {
+    memcpy(dest, src, count);
+  }
+}
+
+// Slightly safer version of `memset()` for use with R object memory
+//
+// See `r_memcpy()` for rationale
+static inline
+void r_memset(void* dest, int value, size_t count) {
+  if (count) {
+    memset(dest, value, count);
+  }
+}
+
 #endif

src/rlang/dict.c

@@ -85,7 +85,7 @@ void r_dict_resize(struct r_dict* p_dict, r_ssize size) {
   r_obj* old_shelter = p_dict->shelter;
   r_list_poke(old_shelter, 1, r_list_get(p_new_dict->shelter, 1));
 
-  memcpy(p_dict, p_new_dict, sizeof(*p_dict));
+  r_memcpy(p_dict, p_new_dict, sizeof(*p_dict));
   p_dict->shelter = old_shelter;
 
   FREE(1);

src/rlang/dyn-array.c

@@ -76,9 +76,9 @@ void r_dyn_push_back(struct r_dyn_array* p_arr,
     r_obj* value = *((r_obj* const *) p_elt);
     p_arr->barrier_set(p_arr->data, loc, value);
   } else if (p_elt) {
-    memcpy(r_dyn_last(p_arr), p_elt, p_arr->elt_byte_size);
+    r_memcpy(r_dyn_last(p_arr), p_elt, p_arr->elt_byte_size);
   } else {
-    memset(r_dyn_last(p_arr), 0, p_arr->elt_byte_size);
+    r_memset(r_dyn_last(p_arr), 0, p_arr->elt_byte_size);
   }
 }
 

src/rlang/dyn-list-of.c

@@ -208,9 +208,9 @@ bool reserve_push_back(struct r_dyn_list_of* p_lof, r_ssize i, void* p_elt) {
   void* p = ((unsigned char*) p_lof->v_reserve) + offset;
 
   if (p_elt) {
-    memcpy(p, p_elt, p_lof->elt_byte_size);
+    r_memcpy(p, p_elt, p_lof->elt_byte_size);
   } else {
-    memset(p, 0, p_lof->elt_byte_size);
+    r_memset(p, 0, p_lof->elt_byte_size);
   }
 
   return true;
@@ -227,7 +227,7 @@ void reserve_move(struct r_dyn_list_of* p_lof, r_ssize i, void* p_elt) {
 
   void* v_new = r_dyn_begin(p_new);
   void* v_old = R_DYN_GET(struct r_pair_ptr_ssize, p_lof->p_arrays, i).ptr;
-  memcpy(v_new, v_old, r_ssize_mult(n, p_lof->elt_byte_size));
+  r_memcpy(v_new, v_old, r_ssize_mult(n, p_lof->elt_byte_size));
 
   p_new->count = n;
 

src/rlang/env-binding.c

@@ -14,7 +14,7 @@ static r_obj* new_binding_types(r_ssize n) {
   r_obj* types = r_alloc_integer(n);
 
   int* types_ptr = r_int_begin(types);
-  memset(types_ptr, 0, n * sizeof *types_ptr);
+  r_memset(types_ptr, 0, n * sizeof *types_ptr);
 
   return types;
 }

src/rlang/vec.c

@@ -38,7 +38,7 @@ r_obj* r_chr_n(const char* const * strings, r_ssize n) {
     C_TYPE* p_out = DEREF(out);                                 \
                                                                 \
     r_ssize cpy_size = (size > x_size) ? x_size : size;         \
-    memcpy(p_out, p_x, cpy_size * sizeof(C_TYPE));              \
+    r_memcpy(p_out, p_x, cpy_size * sizeof(C_TYPE));            \
                                                                 \
     FREE(1);                                                    \
     return out;                                                 \

src/rlang/vec.h

@@ -5,6 +5,7 @@
 
 #include <string.h>
 #include "rlang-types.h"
+#include "c-utils.h"
 #include "cnd.h"
 #include "globals.h"
 #include "obj.h"
@@ -219,7 +220,7 @@ r_obj* r_alloc_raw0(r_ssize n) {
   r_obj* out = r_alloc_raw(n);
 
   unsigned char* p_out = (unsigned char*) r_raw_begin(out);
-  memset(p_out, 0, n);
+  r_memset(p_out, 0, n);
 
   return out;
 }
@@ -438,7 +439,7 @@ r_obj* r_vec_n(enum r_type type, void* v_src, r_ssize n) {
   case R_TYPE_complex:
   case R_TYPE_raw: {
     r_obj* out = r_alloc_vector(type, n);
-    memcpy(r_vec_begin(out), v_src, n * r_vec_elt_sizeof0(type));
+    r_memcpy(r_vec_begin(out), v_src, n * r_vec_elt_sizeof0(type));
     return out;
   }
   case R_TYPE_character:
@@ -474,7 +475,7 @@ r_obj* r_raw_n(int* v_src, r_ssize n) {
 static inline
 r_obj* r_copy_in_raw(const void* src, size_t size) {
   r_obj* out = r_alloc_raw(size);
-  memcpy(r_raw_begin(out), src, size);
+  r_memcpy(r_raw_begin(out), src, size);
   return out;
 }