[Rd] custom allocators, Valgrind and uninitialized memory

[Andreas Kersting]

Hi Simon,

Yes, if this was acceptable on CRAN, I would agree that calling VALGRIND_MAKE_MEM_DEFINED() in my code would be sufficient. 

But since Tomas said, "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.", I am not sure if it is acceptable.

Regards,
Andreas

2021-03-30 00:39 GMT+02:00 "Simon Urbanek" <simon.urbanek using R-project.org>:
> Andres,
> 
> correct me if I'm wrong, but the issue here is not initialisation but rather valgrind flagging. You simply have to call VALGRIND_MAKE_MEM_DEFINED() in your code after allocVector3() to declare that you have initialised the memory - or am I missing something?
> 
> Cheers,
> Simon
> 
> 
> 
>> On 30/03/2021, at 9:18 AM, Andreas Kersting <r-devel using akersting.de> wrote:
>> 
>> 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 using 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
>>> 
>>> 
>>> 
>> ______________________________________________
>> R-devel using r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> 
> 
>