Skip to content

Commit

Permalink
Fix GCC sanitiser issue in df_list()
Browse files Browse the repository at this point in the history
==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
  • Loading branch information
lionel- committed Aug 28, 2020
1 parent a672f8c commit ee5e377
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 16 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 2 additions & 0 deletions cran-comments.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@

Attempt at fixing the gcc-asan error.

## Test environments

* local macOS: release
Expand Down
30 changes: 16 additions & 14 deletions src/type-data-frame.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
Expand All @@ -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;
}

Expand All @@ -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) {
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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]);
Expand Down
2 changes: 1 addition & 1 deletion src/version.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#define R_NO_REMAP
#include <Rinternals.h>

const char* vctrs_version = "0.3.3";
const char* vctrs_version = "0.3.4";

/**
* This file records the expected package version in the shared
Expand Down

0 comments on commit ee5e377

Please sign in to comment.