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

[peter dalgaard]

> On 7 Jun 2020, at 17:53 , 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.
> 
> Best bet to narrow this down is for someone with a good Windows setup
> who can reproduce this to bisect the svn commits and see at what
> commit this started happening. Unfortunately my office Windows machine
> isn't responding and it will probably take some time to get that
> fixed.

Also, it is possible that the issue is really a line or two earlier, so it would be good to get in with a debugger and see what is actually in *tmp and objv[objc++] at the point of the crash. 

Also, Tcl_NewStringObj(tmp, -1) obviously must allocate, but it would be rather odd if it didn't use the system allocator (Tcl is designed to be embeddable, the only strange thing R does in that respect is the marriage of the two event loops).

-pd


> 
> Best,
> 
> luke
> 
>> 
>> -pd
>> 
>> 
>>> On 7 Jun 2020, at 16:00 , Jeroen Ooms <jeroenooms using gmail.com> wrote:
>>> 
>>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox using mcmaster.ca> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> The following code, from the examples in ?TkWidgets , immediately crashes R 4.0.1 for Windows:
>>>> 
>>>> --------------------- snip --------------------
>>>> library("tcltk")
>>>> tt <- tktoplevel()
>>>> label.widget <- tklabel(tt, text = "Hello, World!")
>>>> button.widget <- tkbutton(tt, text = "Push",
>>>>        command = function()cat("OW!\n"))
>>>> tkpack(label.widget, button.widget) # geometry manager
>>>> --------------------- snip --------------------
>>> 
>>> 
>>> I can reproduce this. The backtrace shows the crash happens in
>>> dotTclObjv  [/src/library/tcltk/src/tcltk.c using 243 ]. This looks like a
>>> bug that was introduced by commit 78408/78409 about a month ago. I
>>> think the problem is that this commit changes 'calloc' to 'Calloc'
>>> without changing the corresponding 'free' to 'Free'.
>>> 
>>> This has nothing to do with the Windows build or installation. Nothing
>>> has changed in the windows build procedure between 4.0.0 and 4.0.1.
>>> 
>>> ______________________________________________
>>> R-devel using r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> 
>> 
> 
> -- 
> 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

-- 
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