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@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel

______________________________________________
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel

Reply via email to