[R-pkg-devel] Valgrind warning on saveRDS, about object in external pointer

[David Cortes]

Thanks a lot for the answer!

I unfortunately wasn’t able to reproduce it with the same OS + compiler
+ valgrind settings, but from what I see in the message, turns out in
the end it might indeed be due to Cereal copying raw bytes into an R
vector and Valgrind complaining about the copied bytes rather than the
original object.

And it also looks like it could be issue with default initializations.
With this I’ve now learned that ‘Struct() {}’ and ‘Struct()=default’
are not the same thing.

Still, oddly enough that the complaint happens only at ‘saveRDS’ when
tested on the CRAN servers.

Thanks again for your help!

Best regards,
David Cortes

On Wed, 2020-07-15 at 16:51 +0300, Ivan Krylov wrote:
> On Wed, 08 Jul 2020 22:43:13 +0300
> David Cortes <david.cortes.rivera using gmail.com> wrote:
> 
> > About the source code: it actually complains about line
> > fit_model.cpp:751 :
> > hplane_root->reserve(exp_nodes);
> 
> My fault. I was reading the GitHub source code instead of CRAN
> package
> source code.
> 
> > I’m not able to reproduce the warning when trying R CMD check with
> > valgrind on my computer (tried compiling with gcc9 and clang9), nor
> > with the r-debug docker images from github (
> > https://github.com/wch/r-debug).
> 
> My current system is amd64 Debian 9.12 (gcc 6.3.0-18+deb9u1) with R
> 3.6.3-1~stretchcran.0. I have built a package from the current GitHub
> source of the package and ran R -d 'valgrind --track-origins=yes'
> --vanilla < isotree-Ex.R. This resulted in multiple Valgrind warnings
> about accessing uninitialised values created by heap allocations at
> fit_model.cpp:783 and fit_model.cpp:788.
> 
> The first such error can be traced to a pointer to
> ext_model_ptr->hplanes[0][2].remainder:
> 
> (gdb) frame 2
> #2  cereal::BinaryOutputArchive::saveBinary (this=<optimized out>, 
> data=data using entry=0x14a3ad40, size=size using entry=8)
>     at /home/me/R/x86_64-pc-linux-gnu-
> library/3.6/Rcereal/include/cereal/archives/binary.hpp:67
> 67              auto const writtenSize = static_cast<std::size_t>(
> itsStream.rdbuf()->sputn( reinterpret_cast<const char*>( data ), size
> ) );
> (gdb) p data
> $19 = (const void *) 0x14a3ad40
> (gdb) frame 41
> #41 0x0000000019d8ce38 in fit_model (X_num=..., X_cat=..., ncat=...,
> Xc=..., Xc_ind=..., Xc_indptr=..., sample_weights=...,
> col_weights=..., nrows=100,
>     ncols_numeric=2, ncols_categ=0, ndim=2, ntry=3, coef_type=...,
> coef_by_prop=false, with_replacement=false, weight_as_sample=true,
> sample_size=100, ntrees=3,
>     max_depth=7, limit_depth=false, penalize_range=true,
> calc_dist=false, standardize_dist=true, sq_dist=false,
> calc_depth=false, standardize_depth=true,
>     weigh_by_kurt=false, 
> prob_pick_by_gain_avg=prob_pick_by_gain_avg using entry=0, 
> prob_split_by_gain_avg=prob_split_by_gain_avg using entry=0,
>     prob_pick_by_gain_pl=prob_pick_by_gain_pl using entry=0, 
> prob_split_by_gain_pl=prob_split_by_gain_pl using entry=0, 
> min_gain=min_gain using entry=0, cat_split_type=...,
>     new_cat_action=..., missing_action=..., all_perm=false,
> build_imputer=false, output_imputations=false, min_imp_obs=3,
> depth_imp=..., weigh_imp_rows=...,
>     random_seed=1, nthreads=1) at Rwrapper.cpp:320
> 320         serialized_obj  =  serialize_cpp_obj(ext_model_ptr.get())
> ;
> (gdb) p &ext_model_ptr->hplanes[0][2].remainder
> $20 = (double *) 0x14a3ad40
> 
> (In some stack frames gdb helpfully says that the pointer is
> optimised
> out, but in others it can be accessed.)
> 
> I grepped the source code for 'remainder =' and found one assignment
> to
> hplanes.back().remainder in extended.cpp:507. Could
> hplanes.emplace_back() happen without a corresponding assignment to
> hplanes.back().remainder?
>