SymbolixAU / colourvalues

R library for assigning colours to values
https://symbolixau.github.io/colourvalues/
46 stars 6 forks source link

ASAN memory leak #72

Closed dcooley closed 3 years ago

dcooley commented 3 years ago

Reported from cran

  > test_check("colourvalues")
   =================================================================
   ==2935734==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x625001521818 at pc 0x7f0dcefc803f bp 0x7fff4a4253e0 sp 0x7fff4a4253d0
   READ of size 8 at 0x625001521818 thread T0
       #0 0x7f0dcefc803e in Rcpp::sugar::Times_Vector_Primitive<14,
true, Rcpp::Vector<14, Rcpp::PreserveStorage> >::operator[](long) const
/data/gannet/ripley/R/test-4.1/Rcpp/include/Rcpp/sugar/operators/times.h:273

Reproducible using wch/r-devel docker image, found in amongst these tests

context("palettes")

test_that("palettes are returned", {

  expect_error(colour_values(1:5, palette = "mypalette"), "unknown palette")

  all_palettes <- color_palettes()
  expect_true( length(all_palettes) == 52 )

  for( palette in all_palettes ) {
    expect_silent( colour_values(1, palette = palette ))
    res <- colourvalues::get_palette( palette )
    expect_true( ncol( res ) == 3 & nrow( res ) == 256 )

    ## And call teh function directly
    res <- eval(parse(text = paste0("colourvalues:::", palette, "()")))
    expect_true( ncol( res ) == 3 )
  }

})
dcooley commented 3 years ago

running all_palettes[1:7] breaks on 7th item running all_palettes[2:7] is fine running all_palettes[2:8] breaks on 8th item (7th really, starting from 2)

putting an rm(res) doesn't help.

dcooley commented 3 years ago

ok, the 256-length vectors were initialised, but not always filled, because some palettes are only 9 rows.

Fixed by not initialising with a length, and only initialising the output matrix after the RGB vectors have been allocated.