From dab146a505e2904b6989492a99084590f61be987 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sat, 10 Jan 2026 16:25:58 -0800 Subject: [PATCH 01/10] attempt PROTECT for new rchk issues --- src/assign.c | 4 ++-- src/mergelist.c | 7 ++++--- src/rbindlist.c | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/assign.c b/src/assign.c index 29f5dff4c8..c55fe79799 100644 --- a/src/assign.c +++ b/src/assign.c @@ -70,11 +70,11 @@ static int _selfrefok(SEXP x, Rboolean checkNames, Rboolean verbose) { if (!isNull(p)) internal_error(__func__, ".internal.selfref ptr is neither NULL nor R_NilValue"); // # nocov tag = R_ExternalPtrTag(v); if (!(isNull(tag) || isString(tag))) internal_error(__func__, ".internal.selfref tag is neither NULL nor a character vector"); // # nocov - names = getAttrib(x, R_NamesSymbol); prot = R_ExternalPtrProtected(v); if (TYPEOF(prot) != EXTPTRSXP) // Very rare. Was error(_(".internal.selfref prot is not itself an extptr")). return 0; // # nocov ; see http://stackoverflow.com/questions/15342227/getting-a-random-internal-selfref-error-in-data-table-for-r - return checkNames ? names==tag : x==R_ExternalPtrAddr(prot); + if (!checkNames) return x == R_ExternalPtrAddr(prot); + return getAttrib(x, R_NamesSymbol) == tag; } static Rboolean selfrefok(SEXP x, Rboolean verbose) { // for readability diff --git a/src/mergelist.c b/src/mergelist.c index 90854ae824..2ed3950455 100644 --- a/src/mergelist.c +++ b/src/mergelist.c @@ -82,9 +82,10 @@ SEXP cbindlist(SEXP x, SEXP copyArg) { SET_VECTOR_ELT(ans, ians, thisxcol); SET_STRING_ELT(names, ians, STRING_ELT(thisnames, j)); } - mergeIndexAttrib(index, getAttrib(thisx, sym_index)); - if (isNull(key)) // first key is retained - key = getAttrib(thisx, sym_sorted); + mergeIndexAttrib(index, PROTECT(getAttrib(thisx, sym_index))); protecti++; + if (isNull(key)) { // first key is retained + key = PROTECT(getAttrib(thisx, sym_sorted)); protecti++; + } UNPROTECT(protecti); // thisnames, thisxcol } if (!ANY_ATTRIB(index)) diff --git a/src/rbindlist.c b/src/rbindlist.c index a752be6d39..0ad2a38e84 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -303,7 +303,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isOrdered(thisCol)) { orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (thisLen>longestLen) { longestLen=thisLen; longestLevels=getAttrib(thisCol, R_LevelsSymbol); /*for warnings later ...*/longestW=w; longestI=i; } + if (thisLen>longestLen) { longestLen=thisLen; longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) { @@ -531,7 +531,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor } else { setAttrib(target, R_ClassSymbol, ScalarString(char_factor)); } - UNPROTECT(1); // marks + UNPROTECT(2); // longestLevels, marks } else { // factor==false for (int i=0; i Date: Sat, 10 Jan 2026 16:54:16 -0800 Subject: [PATCH 02/10] different approach for longestLevels --- src/rbindlist.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index 0ad2a38e84..72e25709f6 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -301,9 +301,10 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isNull(getAttrib(thisCol,R_LevelsSymbol))) error(_("Column %d of item %d has type 'factor' but has no levels; i.e. malformed."), w+1, i+1); factor = true; if (isOrdered(thisCol)) { + if (longestLen > 0) PROTECT(longestLevels); orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (thisLen>longestLen) { longestLen=thisLen; longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } + if (thisLen>longestLen) { longestLen=thisLen; longestLevels=getAttrib(thisCol, R_LevelsSymbol); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) { @@ -424,6 +425,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor } } } + UNPROTECT(1); // longestLevels } for (int i=0; i Date: Sat, 10 Jan 2026 17:04:35 -0800 Subject: [PATCH 03/10] use nprotect? --- src/rbindlist.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index 72e25709f6..c05dc22391 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -301,7 +301,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isNull(getAttrib(thisCol,R_LevelsSymbol))) error(_("Column %d of item %d has type 'factor' but has no levels; i.e. malformed."), w+1, i+1); factor = true; if (isOrdered(thisCol)) { - if (longestLen > 0) PROTECT(longestLevels); + if (longestLen > 0) { PROTECT(longestLevels); nprotect++; } orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); if (thisLen>longestLen) { longestLen=thisLen; longestLevels=getAttrib(thisCol, R_LevelsSymbol); /*for warnings later ...*/longestW=w; longestI=i; } @@ -425,7 +425,6 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor } } } - UNPROTECT(1); // longestLevels } for (int i=0; i Date: Sat, 10 Jan 2026 23:45:32 -0800 Subject: [PATCH 04/10] no longer using names SEXP --- src/assign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/assign.c b/src/assign.c index c55fe79799..5901d5a152 100644 --- a/src/assign.c +++ b/src/assign.c @@ -52,7 +52,7 @@ void setselfref(SEXP x) { */ static int _selfrefok(SEXP x, Rboolean checkNames, Rboolean verbose) { - SEXP v, p, tag, prot, names; + SEXP v, p, tag, prot; v = getAttrib(x, SelfRefSymbol); if (v==R_NilValue || TYPEOF(v)!=EXTPTRSXP) { // .internal.selfref missing is expected and normal for i) a pre v1.7.8 data.table loaded From 05d3815576e2a45ce4d2a9f6aec45801cfb663d5 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 11 Jan 2026 00:18:34 -0800 Subject: [PATCH 05/10] only UNPROTECT near exit --- src/rbindlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index c05dc22391..c35fb472a8 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -532,7 +532,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor } else { setAttrib(target, R_ClassSymbol, ScalarString(char_factor)); } - UNPROTECT(2); // marks + UNPROTECT(1); // marks } else { // factor==false for (int i=0; i Date: Sun, 11 Jan 2026 00:34:24 -0800 Subject: [PATCH 06/10] no, that cant be it... --- src/rbindlist.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index c35fb472a8..6a8f386f75 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -277,7 +277,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor int maxType=LGLSXP; // initialize with LGLSXP for test 2002.3 which has col x NULL in both lists to be filled with NA for #1871 bool factor=false, orderedFactor=false; // ordered factor is class c("ordered","factor"). isFactor() is true when isOrdered() is true. int longestLen=-1, longestW=-1, longestI=-1; // just for ordered factor; longestLen must be initialized as -1 so that rbind zero-length ordered factor could work #4795 - SEXP longestLevels=R_NilValue; // just for ordered factor + SEXP longestLevels=PROTECT(R_NilValue); nprotect++; // just for ordered factor bool int64=false, date=false, posixct=false, itime=false, asis=false; const char *foundName=NULL; bool anyNotStringOrFactor=false; @@ -301,10 +301,9 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isNull(getAttrib(thisCol,R_LevelsSymbol))) error(_("Column %d of item %d has type 'factor' but has no levels; i.e. malformed."), w+1, i+1); factor = true; if (isOrdered(thisCol)) { - if (longestLen > 0) { PROTECT(longestLevels); nprotect++; } orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (thisLen>longestLen) { longestLen=thisLen; longestLevels=getAttrib(thisCol, R_LevelsSymbol); /*for warnings later ...*/longestW=w; longestI=i; } + if (thisLen>longestLen) { longestLen=thisLen; UNPROTECT(1); longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) { From f76a4cea95c90792feab5400c1ecf92ca9d595bd Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 11 Jan 2026 09:03:51 -0800 Subject: [PATCH 07/10] move assignment into loop --- src/rbindlist.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index 6a8f386f75..5154c32fbe 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -277,7 +277,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor int maxType=LGLSXP; // initialize with LGLSXP for test 2002.3 which has col x NULL in both lists to be filled with NA for #1871 bool factor=false, orderedFactor=false; // ordered factor is class c("ordered","factor"). isFactor() is true when isOrdered() is true. int longestLen=-1, longestW=-1, longestI=-1; // just for ordered factor; longestLen must be initialized as -1 so that rbind zero-length ordered factor could work #4795 - SEXP longestLevels=PROTECT(R_NilValue); nprotect++; // just for ordered factor + SEXP longestLevels; // just for ordered factor bool int64=false, date=false, posixct=false, itime=false, asis=false; const char *foundName=NULL; bool anyNotStringOrFactor=false; @@ -303,7 +303,8 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isOrdered(thisCol)) { orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (thisLen>longestLen) { longestLen=thisLen; UNPROTECT(1); longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } + if (longestLen == -1) longestLevels = PROTECT(R_NilValue); + if (thisLen > longestLen) { longestLen=thisLen; UNPROTECT(1); longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) { From 3897ad062bd1f57bce88e4383bd9673280ac848b Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 11 Jan 2026 12:11:01 -0800 Subject: [PATCH 08/10] REPROTECT approach --- src/rbindlist.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index 5154c32fbe..67e85374ed 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -277,7 +277,9 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor int maxType=LGLSXP; // initialize with LGLSXP for test 2002.3 which has col x NULL in both lists to be filled with NA for #1871 bool factor=false, orderedFactor=false; // ordered factor is class c("ordered","factor"). isFactor() is true when isOrdered() is true. int longestLen=-1, longestW=-1, longestI=-1; // just for ordered factor; longestLen must be initialized as -1 so that rbind zero-length ordered factor could work #4795 - SEXP longestLevels; // just for ordered factor + PROTECT_INDEX ILongestLevels; + SEXP longestLevels = R_NilValue; // just for ordered factor + PROTECT_WITH_INDEX(longestLevels, &ILongestLevels); nprotect++; bool int64=false, date=false, posixct=false, itime=false, asis=false; const char *foundName=NULL; bool anyNotStringOrFactor=false; @@ -303,8 +305,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isOrdered(thisCol)) { orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (longestLen == -1) longestLevels = PROTECT(R_NilValue); - if (thisLen > longestLen) { longestLen=thisLen; UNPROTECT(1); longestLevels=PROTECT(getAttrib(thisCol, R_LevelsSymbol)); /*for warnings later ...*/longestW=w; longestI=i; } + if (thisLen > longestLen) { longestLen=thisLen; REPROTECT(longestLevels = getAttrib(thisCol, R_LevelsSymbol), ILongestLevels); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) { From bda946b2db88456727ccc329581e7d59e7265b70 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 11 Jan 2026 12:24:54 -0800 Subject: [PATCH 09/10] reduce diff --- src/rbindlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index 67e85374ed..f0bdf169d2 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -278,7 +278,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor bool factor=false, orderedFactor=false; // ordered factor is class c("ordered","factor"). isFactor() is true when isOrdered() is true. int longestLen=-1, longestW=-1, longestI=-1; // just for ordered factor; longestLen must be initialized as -1 so that rbind zero-length ordered factor could work #4795 PROTECT_INDEX ILongestLevels; - SEXP longestLevels = R_NilValue; // just for ordered factor + SEXP longestLevels=R_NilValue; // just for ordered factor PROTECT_WITH_INDEX(longestLevels, &ILongestLevels); nprotect++; bool int64=false, date=false, posixct=false, itime=false, asis=false; const char *foundName=NULL; From 09d384c5cae772e7bf67d5a1c57ce459465e3bf6 Mon Sep 17 00:00:00 2001 From: Michael Chirico Date: Sun, 11 Jan 2026 12:26:30 -0800 Subject: [PATCH 10/10] reduce diff --- src/rbindlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rbindlist.c b/src/rbindlist.c index f0bdf169d2..a047b1d49e 100644 --- a/src/rbindlist.c +++ b/src/rbindlist.c @@ -305,7 +305,7 @@ SEXP rbindlist(SEXP l, SEXP usenamesArg, SEXP fillArg, SEXP idcolArg, SEXP ignor if (isOrdered(thisCol)) { orderedFactor = true; int thisLen = length(getAttrib(thisCol, R_LevelsSymbol)); - if (thisLen > longestLen) { longestLen=thisLen; REPROTECT(longestLevels = getAttrib(thisCol, R_LevelsSymbol), ILongestLevels); /*for warnings later ...*/longestW=w; longestI=i; } + if (thisLen > longestLen) { longestLen=thisLen; REPROTECT(longestLevels=getAttrib(thisCol, R_LevelsSymbol), ILongestLevels); /*for warnings later ...*/longestW=w; longestI=i; } } } else if (!isString(thisCol)) anyNotStringOrFactor=true; // even for length 0 columns for consistency; test 2113.3 if (INHERITS(thisCol, char_integer64)) {