More potential rchk issues

[MichaelChirico]

          I just ran `rchk` locally and get

Function gmean
  [UP] allocating function gather may destroy its unprotected argument (x <arg 1>), which is later used. /rchk/packages/build/OFS5pfVE/data.table/src/gsumm.c:597
  [UP] calling allocating function gather with a fresh pointer (x <arg 1>) /rchk/packages/build/OFS5pfVE/data.table/src/gsumm.c:597
  [UP] unprotected variable x while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/gsumm.c:598

Function rbindlist
  [UP] unprotected variable ans while calling allocating function Rf_getAttrib(?,S:names) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:288
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_getAttrib(?,S:names) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:288
  [UP] unprotected variable ans while calling allocating function R_compute_identical /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:312
  [UP] unprotected variable coercedForFactor while calling allocating function R_compute_identical /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:312
  [UP] unprotected variable ans while calling allocating function Rf_mkChar /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:322
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_mkChar /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:322
  [UP] unprotected variable ans while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:327
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:327
  [UP] unprotected variable ans while calling allocating function Rf_copyMostAttrib /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:328
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_copyMostAttrib /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:328
  [UP] unprotected variable ans while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:334
  [UP] unprotected variable ans while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:341
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:341
  [UP] unprotected variable target while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:341
  [UP] unprotected variable ans while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:495
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:495
  [UP] unprotected variable target while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:495
  [UP] unprotected variable ans while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:497
  [UP] unprotected variable ans while calling allocating function Rf_setAttrib(?,S:levels,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:497
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:497
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_setAttrib(?,S:levels,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:497
  [UP] unprotected variable target while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:497
  [UP] unprotected variable ans while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:502
  [UP] unprotected variable ans while calling allocating function Rf_setAttrib(?,S:class,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:502
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:502
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_setAttrib(?,S:class,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:502
  [UP] unprotected variable ans while calling allocating function Rf_ScalarString /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:506
  [UP] unprotected variable ans while calling allocating function Rf_setAttrib(?,S:class,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:506
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_ScalarString /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:506
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_setAttrib(?,S:class,V) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:506
  [UP] unprotected variable ans while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:521
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:521
  [UP] unprotected variable target while calling allocating function Rf_coerceVector /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:521
  [UP] allocating function memrecycle may destroy its unprotected argument (target <arg 1>), which is later used. /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:524
  [UP] calling allocating function memrecycle with a fresh pointer (target <arg 1>) /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:524
  [UP] unprotected variable ans while calling allocating function memrecycle /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:524
  [UP] unprotected variable coercedForFactor while calling allocating function memrecycle /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:524
  [PB] has negative depth /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:525
  [UP] attempt to unprotect more items (1) than protected (0), results will be incomplete /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:525
  [UP] unprotected variable ans while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:526
  [UP] unprotected variable coercedForFactor while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:526
  [UP] unprotected variable target while calling allocating function Rf_warning /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:526
  [PB] has negative depth /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:534
  [UP] attempt to unprotect more items (1) than protected (0), results will be incomplete /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:534
  [UP] attempt to unprotect more items (2) than protected (0), results will be incomplete /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:534
  [UP] attempt to unprotect more items (2) than protected (1), results will be incomplete /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:534
  [PB] has possible protection stack imbalance /rchk/packages/build/OFS5pfVE/data.table/src/rbindlist.c:536

Function shallow
  [UP] unprotected variable newdt while calling allocating function Rf_shallow_duplicate /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:168
  [UP] unprotected variable newdt while calling allocating function Rf_duplicate /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:171
  [UP] unprotected variable newdt while calling allocating function Rf_getAttrib(?,S:names) /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:173
  [UP] unprotected variable newdt while calling allocating function Rf_allocVector /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:174
  [UP] allocating function setselfref may destroy its unprotected argument (newdt <arg 1>), which is later used. /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:198
  [UP] calling allocating function setselfref with a fresh pointer (newdt <arg 1>) /rchk/packages/build/OFS5pfVE/data.table/src/assign.c:198

Function shift
  [UP] protect stack is too deep, unprotecting all variables, results will be incomplete
  [UP] protect stack is too deep, unprotecting all variables, results will be incomplete
  [UP] unsupported form of unprotect, unprotecting all variables, results will be incomplete /rchk/packages/build/OFS5pfVE/data.table/src/shift.c:174
Analyzed 302 functions, traversed 4262978 states.
Library name (usually package name): data_table
Initialization function: R_init_data_table
Functions: 98
Functions: 1
Checked call to R_registerRoutines: 1

Rchk version: 1cae90e208e97a5c41f1c3e128d99b197478443e
R version: 84255/R Under development (unstable) (2023-04-13 r84255)
LLVM version: 14.0.0

Originally posted by @ben-schwen in #6249 (comment)

[MichaelChirico]

I'm not sure if these are new in dev, or due to a new version of rchk, or just that rchk only reports certain issues on CRAN. Anyway, we should at least investigate these before release.

[MichaelChirico]

@tdhock raises a good point:

should we be running rchk on CI?

I do see {arrow} doing so:

https://github.com/apache/arrow/blob/03726178494c8978bf48b9bab15ed9676e7c9196/dev/tasks/r/github.linux.rchk.yml#L48

They are erroring on 3 different types of rchk outputs ("Suspicious call"/[UP]/[PB])

@jonkeane (original author of that workflow) how has your experience been thus far? Is it reliable & fast (e.g. <10min) enough? Are there false positives?

[jonkeane]

@jonkeane (original author of that workflow) how has your experience been thus far?

We added this when we were given a 2 week warning shortly after it was introduced. It helped immensely confirming that we passed this check. I don't think we've ever run into a problem with it again (in other words, we haven't introduced anything that caused it to have a true positive since we added it + fixed the issues that were there)

Is it reliable & fast (e.g. <10min) enough?

We run it in our extended checks (called crossbow) which run once a day on main (that is, they aren't part of checks on every commit). Here's an example run, the vast majority of time there is spent installing dependencies (35 minutes of 38 minutes total).

I don't think this job has ever failed outside of something catastrophic like arrow failing to build so it can't even be checked.

Are there false positives?

I've never seen a false positive, and checking for the last 120 days, it has succeeded each day. If you care to, you can confirm this using the crossbow report and searching for rchk in either the build status or logs section to see old runs.

They are erroring on 3 different types of rchk outputs ("Suspicious call"/[UP]/[PB])

We have not gone back to confirm that these are still the only ones that CRAN is checking (or if they've updated the running scripts to allow for a non-clean exit on error. But since we haven't had any issues either with rchks not catching or it erroring otherwise, not invested time to make sure that's the totality of what CRAN cares about for these.

Overall, it's been very lightweight for us and haven't had any false positive or other issues running it.

[MichaelChirico]

Thanks so much @jonkeane! Our build step is pretty fast so this whole GHA only takes ~3 minutes, we'll run it on every push.

[MichaelChirico]

Closing, let's open new issues to improve on memory management further.