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

[Gábor Csárdi]

FWIW, you can "test" this by using the daily R builds. They are here:
https://ci.appveyor.com/project/jeroen/base/history

This is the build before the change, it does not crash:
https://ci.appveyor.com/project/jeroen/base/builds/32781690

This is the build right after the change, it does crash:
https://ci.appveyor.com/project/jeroen/base/builds/32808414

(To get the installers, click on "Artifacts" in the top right corner,
download the zip file, it has an exe installer.)

There are a handful of commits between the two installers, but only
this change seems to be related to the crash:
https://github.com/wch/r-source/commit/ba76508c8041335c1896df491ac227fdde0cfb0d
https://github.com/wch/r-source/commit/2c047b94bfe0996419f8871ed6b62b1e7d5ec7bd
https://github.com/wch/r-source/commit/59840c37eb20e05241fb9e85228331fa31db9a2b
https://github.com/wch/r-source/commit/161fc3f703e46201299e87bf7717a93d13c23970
https://github.com/wch/r-source/commit/4c267df39d776fa10c4b2d6b3872dbb85b073681
https://github.com/wch/r-source/commit/f3de035e12a8c8920772297405ed25ee6b3ec4a6

Gabor

On Sun, Jun 7, 2020 at 6:40 PM peter dalgaard <pdalgd using gmail.com> 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
>
> --
> Peter Dalgaard, Professor,
> Center for Statistics, Copenhagen Business School
> Solbjerg Plads 3, 2000 Frederiksberg, Denmark
> Phone: (+45)38153501
> Office: A 4.23
> Email: pd.mes using cbs.dk  Priv: PDalgd using gmail.com
>
> ______________________________________________
> R-devel using r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel