custom allocators, Valgrind and uninitialized memory

Hi,

In my package bettermc, I use a custom allocator, which hands out already defined/initialized memory (mmap of a POSIX shared memory object).

If my code is run in R which was configured --with-valgrind-instrumentation > 0, Valgrind will (correctly) complain about uninitialized memory being used, e.g.

==813836== Conditional jump or move depends on uninitialised value(s)
==813836==    at 0x4F0A9D: getvar (svn/R-devel/src/main/eval.c:5171)
==813836==    by 0x4D9B38: bcEval (svn/R-devel/src/main/eval.c:6867)
==813836==    by 0x4F0077: Rf_eval (svn/R-devel/src/main/eval.c:727)
==813836==    by 0x4F09AF: forcePromise (svn/R-devel/src/main/eval.c:555)
==813836==    by 0x4F0C57: FORCE_PROMISE (svn/R-devel/src/main/eval.c:5136)
==813836==    by 0x4F0C57: getvar (svn/R-devel/src/main/eval.c:5177)
==813836==    by 0x4D9B38: bcEval (svn/R-devel/src/main/eval.c:6867)
==813836==    by 0x4F0077: Rf_eval (svn/R-devel/src/main/eval.c:727)
==813836==    by 0x4F1A8D: R_execClosure (svn/R-devel/src/main/eval.c:1897)
==813836==    by 0x4F2783: Rf_applyClosure (svn/R-devel/src/main/eval.c:1823)
==813836==    by 0x4DF61D: bcEval (svn/R-devel/src/main/eval.c:7083)
==813836==    by 0x4F0077: Rf_eval (svn/R-devel/src/main/eval.c:727)
==813836==    by 0x4F1A8D: R_execClosure (svn/R-devel/src/main/eval.c:1897)
==813836==  Uninitialised value was created by a client request
==813836==    at 0x52D5CF: Rf_allocVector3 (svn/R-devel/src/main/memory.c:2892)
==813836==    by 0x16B415EA: allocate_from_shm (packages/tests-vg/bettermc/src/copy2shm.c:289)
==813836==    by 0x49D123: R_doDotCall (svn/R-devel/src/main/dotcode.c:614)
==813836==    by 0x4DA36D: bcEval (svn/R-devel/src/main/eval.c:7671)
==813836==    by 0x4F0077: Rf_eval (svn/R-devel/src/main/eval.c:727)
==813836==    by 0x4F1A8D: R_execClosure (svn/R-devel/src/main/eval.c:1897)
==813836==    by 0x4F2783: Rf_applyClosure (svn/R-devel/src/main/eval.c:1823)
==813836==    by 0x4F0243: Rf_eval (svn/R-devel/src/main/eval.c:850)
==813836==    by 0x49B68F: do_External (svn/R-devel/src/main/dotcode.c:573)
==813836==    by 0x4D3566: bcEval (svn/R-devel/src/main/eval.c:7115)
==813836==    by 0x4F0077: Rf_eval (svn/R-devel/src/main/eval.c:727)
==813836==    by 0x4F1A8D: R_execClosure (svn/R-devel/src/main/eval.c:1897)

(allocate_from_shm() is my function calling allocVector3() with a custom allocator.) Valgrind is correct, because allocVector3() explicitly calls VALGRIND_MAKE_MEM_UNDEFINED() on the memory my custom allocator returns.

- Sould allocVector3() call VALGRIND_MAKE_MEM_UNDEFINED() also if a custom allocator is used? For some custom allocators this is correct, for others not.

- Or should the code using a custom allocator call VALGRIND_MAKE_MEM_DEFINED() on the DATAPTR() returned by allocVector3()? E.g.

...
ret = PROTECT(allocVector3(asInteger(type), asReal(length), &allocator));
VALGRIND_MAKE_MEM_DEFINED(DATAPTR(ret), size);
...

For the latter to work also on systems without Valgrind installed, I need to include both valgrind.h and memcheck.h in the src of my package and include these (rather than the system headers), correct? Should I best take these headers directly from R (src/include/vg)?

Thanks! Regards,
Andreas

Andreas,

Can you briefly describe what it is you are trying to do?

In general, no R package would use valgrind directly; it is an optional
debugger. Also note _Writing R Extensions_ has a few things to say about how
memory destined for R object can and cannot be allocated -- I presume your
custom allocator is only for objects you managed and do not return to R?

Best, Dirk

-- 
https://dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org

Hi Dirk,  > > Sure, let me try to explain: > > CRAN ran the tests of my package 

Hi Andreas,
On 3/26/21 8:48 PM, Andreas Kersting wrote:

Hi Dirk,  > > Sure, let me try to explain: > > CRAN ran the tests of my package using R which was configured > --with-valgrind-instrumentation > 0. Valgrind reported many errors > related to the use of supposedly uninitialized memory and the CRAN > team asked me to tackle these. > > These errors are false positives, because I pass a custom allocator > to allocVector3() which hands out memory which is already > initialized. However, this memory is explicitly marked for Valgrind > as uninitialized by allocVector3(), and I do not initialize it > subsequently, so Valgrind complains. > > Now I am asking if it is correct that allocVector3() marks memory as > uninitialized/undefined, even if it comes from a custom allocator. > This is because allocVector3() cannot know if the memory might > already by initialized.

I think the semantics of allocVector/allocVector3 should be the same regardless of whether custom allocators are used. The semantics of allocVector is to provide uninitialized memory (non-pointer types, Writing R Extensions 5.9.2). Therefore, it is the caller who needs to take care of initialization. This is also the semantics of "malloc" and Rallocators.h says "custom_alloc_t mem_alloc; /* malloc equivalent */".

So I think that your code using your custom allocator needs to initialize allocated memory to be correct. If your allocator initializes the memory, that is fine, but unnecessary.

So technically speaking, the valgrind reports are not false alarms. I think your call sites should initialize.

Best
Tomas

Hi Tomas,

Thanks for sharing your view on this! I understand your point, but still I think that the current situation is somewhat unfortunate:

I would argue that mmap() is a natural candidate to be used together with allocVector3(); it is even mentioned explicitly here: https://github.com/wch/r-source/blob/trunk/src/main/memory.c#L2575-L2576

However, when using a non-anonymous mapping, i.e. we want mmap() to initialize the memory e.g. from a file or a POSIX shared memory object, this means that we need to use MAP_FIXED in case we are obliged to initialize the memory AFTER allocVector3() returned it; at least I cannot think of a different way to achieve this.

The use of MAP_FIXED
- is discouraged (e.g. https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/mmap.2.html)
- requires two calls to mmap(): (1) to obtain the (anonymous) memory to be handed out by the custom allocater and (2) to actually map the file "over" the just allocated vector (using MAP_FIXED), which will overwrite the vector header; hence, we need to first back it up to later restore it

I have implemented my function using MAP_FIXED here: https://github.com/gfkse/bettermc/commit/f34c4f4c45c9ab11abe9b9e9b8b48064f128d731#diff-7098a5dde34efab163bbef27fe32f95c29e76236649479985d09c70100e4c737R278-R323

This solution, to me, is much more complicated and hacky than my previous one, which assumed it is OK to hand out already initialized memory directly from allocVector3().

Regards,
Andreas


2021-03-29 10:41 GMT+02:00 "Tomas Kalibera" <tomas.kalibera at gmail.com>:

Hi Andreas,
On 3/26/21 8:48 PM, Andreas Kersting wrote:

Hi Dirk,  > > Sure, let me try to explain: > > CRAN ran the tests of my package using R which was configured > --with-valgrind-instrumentation > 0. Valgrind reported many errors > related to the use of supposedly uninitialized memory and the CRAN > team asked me to tackle these. > > These errors are false positives, because I pass a custom allocator > to allocVector3() which hands out memory which is already > initialized. However, this memory is explicitly marked for Valgrind > as uninitialized by allocVector3(), and I do not initialize it > subsequently, so Valgrind complains. > > Now I am asking if it is correct that allocVector3() marks memory as > uninitialized/undefined, even if it comes from a custom allocator. > This is because allocVector3() cannot know if the memory might > already by initialized.

I think the semantics of allocVector/allocVector3 should be the same regardless of whether custom allocators are used. The semantics of allocVector is to provide uninitialized memory (non-pointer types, Writing R Extensions 5.9.2). Therefore, it is the caller who needs to take care of initialization. This is also the semantics of "malloc" and Rallocators.h says "custom_alloc_t mem_alloc; /* malloc equivalent */".

So I think that your code using your custom allocator needs to initialize allocated memory to be correct. If your allocator initializes the memory, that is fine, but unnecessary.

So technically speaking, the valgrind reports are not false alarms. I think your call sites should initialize.

Best
Tomas

appropriate to tell valgrind about it. As this is becoming rather too
technical and specific to the internals, feel free to take this offline
with Simon and me.