peter dalgaard
2020-Jun-07 15:28 UTC
[Rd] use of the tcltk package crashes R 4.0.1 for Windows
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).
-pd
> On 7 Jun 2020, at 16:00 , Jeroen Ooms <jeroenooms at gmail.com>
wrote:
>
> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox at 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 at 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 at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
--
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 at cbs.dk Priv: PDalgd at gmail.com
iuke-tier@ey m@iii@g oii uiow@@edu
2020-Jun-07 15:53 UTC
[Rd] [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
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. Best, luke> > -pd > > >> On 7 Jun 2020, at 16:00 , Jeroen Ooms <jeroenooms at gmail.com> wrote: >> >> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox at 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 at 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 at 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 at uiowa.edu Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu
Fox, John
2020-Jun-07 16:08 UTC
[Rd] [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
Hi, Does it make sense to withdraw the Windows R 4.0.1 binary until the issue is resolved? Best, John> -----Original Message----- > From: luke-tierney at uiowa.edu <luke-tierney at uiowa.edu> > Sent: Sunday, June 7, 2020 11:54 AM > To: peter dalgaard <pdalgd at gmail.com> > Cc: Jeroen Ooms <jeroenooms at gmail.com>; Fox, John <jfox at mcmaster.ca>; r- > devel at r-project.org > Subject: Re: [External] Re: [Rd] use of the tcltk package crashes R 4.0.1 > for Windows > > 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. > > Best, > > luke > > > > > -pd > > > > > >> On 7 Jun 2020, at 16:00 , Jeroen Ooms <jeroenooms at gmail.com> wrote: > >> > >> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox at 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 at 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 at 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 at uiowa.edu > Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu
iuke-tier@ey m@iii@g oii uiow@@edu
2020-Jun-07 16:23 UTC
[Rd] [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
There is one other possibility: It may be that the calloc/free pair picked up by the tcltk package DLL is different from the pair picked up when building base R. (We provide our own malloc framework, but if the macros aren't quite right it may be that the system malloc is picked up in some cases). In that case using Calloc and free would be mismatching the malloc systems and probably segfault. If that is indeed happening we should fix it, but using Free with Calloc should cure the immediate symptom. Best, luke On Sun, 7 Jun 2020, luke-tierney at 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. > > Best, > > luke > >> >> -pd >> >> >>> On 7 Jun 2020, at 16:00 , Jeroen Ooms <jeroenooms at gmail.com> wrote: >>> >>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox at 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 at 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 at 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 at uiowa.edu Iowa City, IA 52242 WWW: http://www.stat.uiowa.edu
Jeroen Ooms
2020-Jun-07 16:59 UTC
[Rd] [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
On Sun, Jun 7, 2020 at 5:53 PM <luke-tierney at 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
peter dalgaard
2020-Jun-07 17:07 UTC
[Rd] [External] use of the tcltk package crashes R 4.0.1 for Windows
> On 7 Jun 2020, at 17:53 , luke-tierney at 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 at gmail.com> wrote: >>> >>> On Sun, Jun 7, 2020 at 3:13 AM Fox, John <jfox at 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 at 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 at 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 at 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 at cbs.dk Priv: PDalgd at gmail.com
Reasonably Related Threads
- [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
- use of the tcltk package crashes R 4.0.1 for Windows
- [External] Re: use of the tcltk package crashes R 4.0.1 for Windows
- [External] use of the tcltk package crashes R 4.0.1 for Windows
- [External] Re: use of the tcltk package crashes R 4.0.1 for Windows