[R-pkg-devel] Valgrind warning: invalid size of 4, is it due to a simple overrun?

[Tomas Kalibera]

On 6/2/21 3:01 AM, Hugh Parsonage wrote:
> Hello,
>
> I received a valgrind warning about my package, hutilscpp, from CRAN
> (thank you).  I believe I've tracked down and fixed the problem, but I
> wasn't able to reproduce the error on rhub (including with
> --leak-check=full etc) so I'd like to know if I'm missing something.
>
> The valgrind warning is "Invalid write of size 4" and refers to this
> line https://github.com/HughParsonage/hutilscpp/blob/508f134b3f388653985eca372ed5f4f8b8eb3471/src/Cwhich_even.c#L43
>
> the context for this line is:
>
> const double * xp = REAL(xx);
> for (R_xlen_t i = 0, j = 0; i < N; ++i) {
>    int is_even = R_finite(xp[i]) && (fmod(xp[i], 2) == 0);
>    ansp[j] = (int)(i + 1);        // # <-------- line 43
>    j += is_even;
> }
I would simply use an explicit branch, store the index into ansp only 
when the element is even, and increment an index for ansp in that case. 
That would make the code clearer and easy to see potential errors in it.
> where ansp is a pointer to an integer vector whose length is the
> number of "even" doubles in xx.  I can see that the problem arises
> when the last even number occurs before the end of the vector xx, at
> which point j == length(ansp), yet the loop continues.  I've amended
> the i < N in the for loop to (i < N && j < n_even) which I believe is
> sufficient, but obviously I thought it was ok before.
>
> I'd rather not resubmit only to discover that I've overlooked something else.
>
> As a supplementary question, does the valgrind check depend on the
> runtime values in the test suite or does it perform fuzz testing?  In
> other words, do I need to add tests to reproduce?

With valgrind, the code is ran deterministically.

I would not worry about trying to reproduce if the code is rewritten in 
the way suggested, because that's easy to validate visually.

In principle, if you wanted still to reproduce, you could add an 
explicit runtime check that the identified write is in bounds, run the 
original code, hopefully see it fail. And then do the same with the 
fixed code and see it pass. And then remove the runtime check and submit.

Best
Tomas

>
> Best,
>
>
> Hugh.
>
> ______________________________________________
> R-package-devel using r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-package-devel