From 1742a31644110f65422a4875e6081d2ce330bb4d Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 28 Aug 2020 14:21:53 +0200 Subject: [PATCH] Fix GCC sanitiser issue in `df_list()` ==2349903==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62500d7ca018 at pc 0x7f2671a13015 bp 0x7ffd56440d30 sp 0x7ffd56440d20 READ of size 8 at 0x62500d7ca018 thread T0 #0 0x7f2671a13014 in df_list_splice /data/gannet/ripley/R/packages/tests-gcc-SAN/vctrs/src/type-data-frame.c:300 #1 0x7f2671a13014 in df_list /data/gannet/ripley/R/packages/tests-gcc-SAN/vctrs/src/type-data-frame.c:246 #2 0x7f2671a1438e in data_frame /data/gannet/ripley/R/packages/tests-gcc-SAN/vctrs/src/type-data-frame.c:201 #3 0x7f2671a144fa in vctrs_data_frame /data/gannet/ripley/R/packages/tests-gcc-SAN/vctrs/src/type-data-frame.c:192 --- DESCRIPTION | 2 +- NEWS.md | 5 +++++ src/type-data-frame.c | 30 ++++++++++++++++-------------- src/version.c | 2 +- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index adac2e8529..f52296d58e 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: vctrs Title: Vector Helpers -Version: 0.3.3 +Version: 0.3.4 Authors@R: c(person(given = "Hadley", family = "Wickham", diff --git a/NEWS.md b/NEWS.md index 0223d3fe40..4f3fa7001a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,9 @@ +# vctrs 0.3.4 + +* Fixed a GCC sanitiser error revealed by CRAN checks. + + # vctrs 0.3.3 * The `table` class is now implemented as a wrapper type that diff --git a/src/type-data-frame.c b/src/type-data-frame.c index 351b401732..0954fb188a 100644 --- a/src/type-data-frame.c +++ b/src/type-data-frame.c @@ -223,8 +223,8 @@ SEXP vctrs_df_list(SEXP x, SEXP size, SEXP name_repair) { return out; } -static SEXP df_list_drop_null(SEXP x, r_ssize n); -static SEXP df_list_splice(SEXP x, r_ssize n); +static SEXP df_list_drop_null(SEXP x); +static SEXP df_list_splice(SEXP x); SEXP df_list(SEXP x, r_ssize size, const struct name_repair_opts* p_name_repair_opts) { if (TYPEOF(x) != VECSXP) { @@ -242,8 +242,8 @@ SEXP df_list(SEXP x, r_ssize size, const struct name_repair_opts* p_name_repair_ UNPROTECT(1); } - x = PROTECT(df_list_drop_null(x, n_cols)); - x = PROTECT(df_list_splice(x, n_cols)); + x = PROTECT(df_list_drop_null(x)); + x = PROTECT(df_list_splice(x)); SEXP names = PROTECT(r_names(x)); names = PROTECT(vec_as_names(names, p_name_repair_opts)); @@ -253,10 +253,11 @@ SEXP df_list(SEXP x, r_ssize size, const struct name_repair_opts* p_name_repair_ return x; } -static SEXP df_list_drop_null(SEXP x, r_ssize n) { +static SEXP df_list_drop_null(SEXP x) { + r_ssize n_cols = r_length(x); r_ssize count = 0; - for (r_ssize i = 0; i < n; ++i) { + for (r_ssize i = 0; i < n_cols; ++i) { count += VECTOR_ELT(x, i) == R_NilValue; } @@ -267,12 +268,12 @@ static SEXP df_list_drop_null(SEXP x, r_ssize n) { SEXP names = PROTECT(r_names(x)); const SEXP* p_names = STRING_PTR_RO(names); - r_ssize n_out = n - count; + r_ssize n_out = n_cols - count; SEXP out = PROTECT(Rf_allocVector(VECSXP, n_out)); SEXP out_names = PROTECT(Rf_allocVector(STRSXP, n_out)); r_ssize out_i = 0; - for (r_ssize i = 0; i < n; ++i) { + for (r_ssize i = 0; i < n_cols; ++i) { SEXP col = VECTOR_ELT(x, i); if (col != R_NilValue) { @@ -288,14 +289,15 @@ static SEXP df_list_drop_null(SEXP x, r_ssize n) { return out; } -static SEXP df_list_splice(SEXP x, r_ssize n) { +static SEXP df_list_splice(SEXP x) { SEXP names = PROTECT(r_names(x)); const SEXP* p_names = STRING_PTR_RO(names); bool any_needs_splice = false; + r_ssize n_cols = r_length(x); r_ssize i = 0; - for (; i < n; ++i) { + for (; i < n_cols; ++i) { // Only splice unnamed data frames if (p_names[i] != strings_empty) { continue; @@ -314,16 +316,16 @@ static SEXP df_list_splice(SEXP x, r_ssize n) { return x; } - SEXP splice = PROTECT(r_new_logical(n)); + SEXP splice = PROTECT(r_new_logical(n_cols)); int* p_splice = LOGICAL(splice); - for (r_ssize j = 0; j < n; ++j) { + for (r_ssize j = 0; j < n_cols; ++j) { p_splice[j] = 0; } r_ssize width = i; - for (; i < n; ++i) { + for (; i < n_cols; ++i) { // Only splice unnamed data frames if (p_names[i] != strings_empty) { ++width; @@ -346,7 +348,7 @@ static SEXP df_list_splice(SEXP x, r_ssize n) { r_ssize loc = 0; // Splice loop - for (r_ssize i = 0; i < n; ++i) { + for (r_ssize i = 0; i < n_cols; ++i) { if (!p_splice[i]) { SET_VECTOR_ELT(out, loc, VECTOR_ELT(x, i)); SET_STRING_ELT(out_names, loc, p_names[i]); diff --git a/src/version.c b/src/version.c index 6269b8233e..a09036616e 100644 --- a/src/version.c +++ b/src/version.c @@ -1,7 +1,7 @@ #define R_NO_REMAP #include -const char* vctrs_version = "0.3.3"; +const char* vctrs_version = "0.3.4"; /** * This file records the expected package version in the shared