[Rd] [External] Re: use of the tcltk package crashes R 4.0.1 for Windows

[iuke-tier@ey m@iii@g oii uiow@@edu]

I've committed the change to use Free instead of free in tcltk.c and
sys-std.c (r78652 for R-devel, r78653 for R-patched).

We might consider either moving Calloc/Free out of the Windows
remapping or moving the remapping into header files so everything
seeing our header files uses our calloc/free. Either would be less
brittle that the current status.

Best,

luke

On Sun, 7 Jun 2020, peter dalgaard wrote:

>
>
>> On 7 Jun 2020, at 18:59 , Jeroen Ooms <jeroenooms using gmail.com> wrote:
>>
>> On Sun, Jun 7, 2020 at 5:53 PM <luke-tierney using uiowa.edu> wrote:
>>>
>>> On Sun, 7 Jun 2020, peter dalgaard wrote:
>>>
>>>> So this wasn't tested for a month?
>>>>
>>>> Anyways, Free() is just free() with a check that we're not freeing a null pointer, followed by setting the pointer to NULL. At that point of tcltk.c, we have
>>>>
>>>>  for (objc = i = 0; i < length(avec); i++){
>>>>       const char *s;
>>>>       char *tmp;
>>>>       if (!isNull(nm) && strlen(s = translateChar(STRING_ELT(nm, i)))){
>>>>           //  tmp = calloc(strlen(s)+2, sizeof(char));
>>>>           tmp = Calloc(strlen(s)+2, char);
>>>>           *tmp = '-';
>>>>           strcpy(tmp+1, s);
>>>>           objv[objc++] = Tcl_NewStringObj(tmp, -1);
>>>>           free(tmp);
>>>>       }
>>>>       if (!isNull(t = VECTOR_ELT(avec, i)))
>>>>           objv[objc++] = (Tcl_Obj *) R_ExternalPtrAddr(t);
>>>>   }
>>>>
>>>> and I can't see how tmp can be NULL at the free(), nor can I see it mattering if it is not set to NULL (notice that it goes out of scope with the for loop).
>>>
>>> Right. And the calloc->Calloc change doesn't look like an issue either
>>> -- just checking for a NULL.
>>>
>>> If the crash is happening in free() then that most likely means
>>> corrupted malloc data structures. Unfortunately that could be
>>> happening anywhere.
>>
>> Writing R extensions, section 6.1.2 says: "Do not assume that memory
>> allocated by Calloc/Realloc comes from the same pool as used by
>> malloc: in particular do not use free or strdup with it."
>>
>> I think the reason is that R uses dlmalloc for Calloc on Windows:
>> https://github.com/wch/r-source/blob/c634fec5214e73747b44d7c0e6f047fefe44667d/src/main/memory.c#L94-L103
>
> But that section #defines calloc and free to Rm_... counterparts in lockstep? (I assume that is where dlmalloc comes in?)
>
> Anyways, does it actually work to change free() to Free()? If so, then all this post mortem analysis is rather a moot point.
>
> -pd
>
>

-- 
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   luke-tierney using uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu