Ah, I see it now: The remapping of free() to Rm_free() and calloc() to Rm_calloc() happens in memory.c, but not in tcltk.c; the macro Calloc in R_ext/RS.h maps to a call to R_chk_alloc which is defined in memory.h; RS.h is included in tcltk.c, so tcltk.c winds up calling Rm_calloc() via Calloc(), but then the NON-remapped free(), and the walls come tumbling down.
If the "#if defined(Win32)" block had been inside RS.h, the problem wouldn't arise. -pd > On 8 Jun 2020, at 00:03 , luke-tier...@uiowa.edu wrote: > > 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 -- Peter Dalgaard, Professor, Center for Statistics, Copenhagen Business School Solbjerg Plads 3, 2000 Frederiksberg, Denmark Phone: (+45)38153501 Office: A 4.23 Email: pd....@cbs.dk Priv: pda...@gmail.com ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel