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 <jeroeno...@gmail.com> wrote:

On Sun, Jun 7, 2020 at 5:53 PM <luke-tier...@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-tier...@uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

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

Reply via email to