`fmap` trying to use uninitialized variables, causing undefined-behavior

[dipterix]

Reported by clang-UBSAN from CRAN

> ### Name: fmap
> ### Title: Map multiple file arrays and save results
> ### Aliases: fmap fmap2 fmap_element_wise
> 
> ### ** Examples
> 
> 
> 
> set.seed(1)
> x1 <- filearray_create(tempfile(), dimension = c(100,20,3))
> x1[] <- rnorm(6000)
> x2 <- filearray_create(tempfile(), dimension = c(100,20,3))
> x2[] <- rnorm(6000)
> 
> # Add two arrays
> output <- filearray_create(tempfile(), dimension = c(100,20,3))
> fmap(list(x1, x2), function(input){
+     input[[1]] + input[[2]]
+ }, output)
save.cpp:50:28: runtime error: signed integer overflow: 4616189618054758400 * 2000 cannot be represented in type 'long'
    #0 0x7f17d676584c in FARR_subset_assign_sequential_bare(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, long const&, SEXPREC*, unsigned int, SEXPREC*, long) /data/gannet/ripley/R/packages/tests-clang-SAN/filearray/src/save.cpp:50:28
    #1 0x7f17d67532ac in FARR_buffer_map(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Rcpp::Function_Impl<Rcpp::PreserveStorage> const&, int const&, int) /data/gannet/ripley/R/packages/tests-clang-SAN/filearray/src/map.cpp:143:13
    #2 0x7f17d66d65b9 in _filearray_FARR_buffer_map /data/gannet/ripley/R/packages/tests-clang-SAN/filearray/src/RcppExports.cpp:273:34
    #3 0x557c1628e7f0 in R_doDotCall /data/gannet/ripley/R/svn/R-devel/src/main/dotcode.c:880:17
    #4 0x557c163f137f in bcEval /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:7682:21
    #5 0x557c163d2855 in Rf_eval /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:748:8
    #6 0x557c16436ad6 in R_execClosure /data/gannet/ripley/R/svn/R-devel/src/main/eval.c
    #7 0x557c164324d3 in Rf_applyClosure /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:1844:16
    #8 0x557c163f71f0 in bcEval /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:7094:12
    #9 0x557c163d2855 in Rf_eval /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:748:8
    #10 0x557c16436ad6 in R_execClosure /data/gannet/ripley/R/svn/R-devel/src/main/eval.c
    #11 0x557c164324d3 in Rf_applyClosure /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:1844:16
    #12 0x557c163d31b8 in Rf_eval /data/gannet/ripley/R/svn/R-devel/src/main/eval.c:871:12
    #13 0x557c164ffab6 in Rf_ReplIteration /data/gannet/ripley/R/svn/R-devel/src/main/main.c:262:2
    #14 0x557c16503140 in R_ReplConsole /data/gannet/ripley/R/svn/R-devel/src/main/main.c:314:11
    #15 0x557c16502f36 in run_Rmainloop /data/gannet/ripley/R/svn/R-devel/src/main/main.c:1192:5
    #16 0x557c16503282 in Rf_mainloop /data/gannet/ripley/R/svn/R-devel/src/main/main.c:1199:5
    #17 0x557c16093d8c in main /data/gannet/ripley/R/svn/R-devel/src/main/Rmain.c:29:5
    #18 0x7f17e6c2954f in __libc_start_call_main (/lib64/libc.so.6+0x2954f) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe)
    #19 0x7f17e6c29608 in __libc_start_main@GLIBC_2.2.5 (/lib64/libc.so.6+0x29608) (BuildId: 9c5863396a11aab52ae8918ae01a362cefa855fe)
    #20 0x557c15fd1244 in _start (/data/gannet/ripley/R/R-clang-SAN/bin/exec/R+0x31b244)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior save.cpp:50:28 in 
Reference class object of class "FileArray"
Mode: readwrite 
Dimension: 100x20x3 
# of partitions: 3 
Partition size: 1 
Storage type: double (internal size: 8)
Location: /tmp/RtmpXmNcyt/file36aaae68a9e2a2 

The bug is caused by the following code:

https://github.com/dipterix/filearray/blob/c99c1229f557c0ca9493082ad40775087fe970ad/src/save.cpp#L49-L50

When the buffer size exceed array lengths, slice_idx2 may exceed nparts (number of partitions), and pointer cum_part will go beyond the end of the vector (that's why *cum_part becomes 4616189618054758400)

The solution could be instead of calculating skip_end, simply replace the following code

https://github.com/dipterix/filearray/blob/c99c1229f557c0ca9493082ad40775087fe970ad/src/save.cpp#L78-L81

to:

write_len = part_nelem - read_start;

if(nwrite + write_len > len) {
    write_len = len - nwrite;
}
if(write_len <= 0) {
    break;
}

[dipterix]

Also the following statement is wrong,

https://github.com/dipterix/filearray/blob/c99c1229f557c0ca9493082ad40775087fe970ad/src/save.cpp#L68-L69

Changing to

if(part == 0) {
    part_nelem = (*cum_part) * unit_partlen;
} else {
    part_nelem = (*cum_part - *(cum_part - 1)) * unit_partlen;
}
if( part == part_start ) {
    read_start = skip_start;
} else {
    read_start = 0;
}

The code exists in both load.cpp and save.cpp... Needs to update both.

[dipterix]

The new commit https://github.com/dipterix/filearray/commit/21bc2081a58dd3431ea87c3d02e526a2ee253393 fixes the issues (filearray 0.1.4.9000)

Testing script:

library(filearray)
filearray_threads(1)

testthat::test_dir('tests')
testthat::test_examples('.')
# loadNamespace("bit64")

set.seed(1)
x1 <- filearray_create(tempfile(), dimension = c(100,20,3))
x1[] <- rnorm(6000)
x2 <- filearray_create(tempfile(), dimension = c(100,20,3))
x2[] <- rnorm(6000)

# Add two arrays
output <- filearray_create(tempfile(), dimension = c(100,20,3))
fmap(list(x1, x2), function(input){
    print(length(input[[1]]))
    input[[1]] + input[[2]]
}, output, .input_size=8100)
print(range(x1[] + x2[] - output[]))
# R -d "valgrind --tool=memcheck --leak-check=full" -f tmp.R | grep ==

Memory check result:

> # R -d "valgrind --tool=memcheck --leak-check=full" -f tmp.R | grep ==
==40499== 
==40499== HEAP SUMMARY:
==40499==     in use at exit: 178,803,840 bytes in 20,419 blocks
==40499==   total heap usage: 1,294,546 allocs, 1,274,127 frees, 1,198,582,624 bytes allocated
==40499== 
==40499== LEAK SUMMARY:
==40499==    definitely lost: 0 bytes in 0 blocks
==40499==    indirectly lost: 0 bytes in 0 blocks
==40499==      possibly lost: 0 bytes in 0 blocks
==40499==    still reachable: 178,803,840 bytes in 20,419 blocks
==40499==                       of which reachable via heuristic:
==40499==                         newarray           : 4,264 bytes in 1 blocks
==40499==         suppressed: 0 bytes in 0 blocks
==40499== Reachable blocks (those to which a pointer was found) are not shown.
==40499== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==40499== 
==40499== For lists of detected and suppressed errors, rerun with: -s
==40499== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[dipterix]

Closing this issue as the fix has been deployed (>= 0.1.5). The version has been submitted to CRAN.