[R-package] Protection stack imbalance and unprotected objects

[fabsig]

The R-package has two protection issues. First, there is a protection stack imbalance when functions are exited by returns from the catch statements in the R_API_END macro. This issue is found when running rchk; see the output below. Second, several objects (results of Rf_asChar(.), e.g. here) which should be protected are currently not protected.

Disclaimer: the main credit for finding the causes of these issues goes to @kalibera.

I will make a pull request / proposal with a suggestion for a solution for this.

Below is output when running rchk on the LightGBM CRAN R-package after this commit
42ddff1. Note that the missing protections for Rf_asChar(.) are not found by rchk.

Function LGBM_DatasetCreateFromMat_R
  [PB] has possible protection stack imbalance /rchk/packages/build/JczWIO8n/lightgbm/src/lightgbm_R.cpp:126

Function LGBM_DatasetGetFeatureNames_R
  [PB] has possible protection stack imbalance /rchk/packages/build/JczWIO8n/lightgbm/src/lightgbm_R.cpp:210

Function LGBM_DatasetGetSubset_R
  [PB] has possible protection stack imbalance /rchk/packages/build/JczWIO8n/lightgbm/src/lightgbm_R.cpp:150
Analyzed 39640 functions, traversed 108278 states.
Library name (usually package name): lightgbm
Initialization function: R_init_lightgbm
Functions: 41
Checked call to R_registerRoutines: 1

Rchk version: 3d653b7c8f92dac912532856b55f44d2986c6553
R version: 79889/R Under development (unstable) (2021-01-27 r79889)
LLVM version: 10.0.0

[jameslamb]

Thanks very much for your investigation and write-up! Linking #4282 as there is some overlap between this issue and that one, but this issue adds a lot of good details not contained there.

Could you describe how you ran rchk on {lightgbm}? Ideally a docker run command or something else that would be easy for me to reproduce. I have tried before by following the examples at https://github.com/kalibera/rchk and was not successful (possibly because of some non-standard things we did in {lightgbm}.

Separate from #4391, I'd like to add a continuous integration job that runs rchk so we can detect and prevent such issues in the future.

[fabsig]

Thanks a lot for your feedback.

Could you describe how you ran rchk on {lightgbm}? Ideally a docker run command or something else that would be easy for me to reproduce. I have tried before by following the examples at https://github.com/kalibera/rchk and was not successfully (possibly because of some non-standard things we did in {lightgbm}.

I used docker and followed the steps described here. After creating the CRAN lightgbm_*.tar.gz file with your shell script, I ran the following docker command docker run -v %cd%/packages:/rchk/packages kalibera/rchk:latest /rchk/packages/lightgbm_3.2.1.99.tar.gz

[jameslamb]

Excellent, thanks very much! For the sake of others looking to this issue interested in reproducing these problems, this set of commands worked for me.

sh build-cran-package.sh
mkdir packages
cp lightgbm_3.2.1.99.tar.gz packages
docker run -v `pwd`/packages:/rchk/packages kalibera/rchk:latest /rchk/packages/lightgbm_3.2.1.99.tar.gz

And that resulted in the following logs

Library name (usually package name): lightgbm
Initialization function: R_init_lightgbm
Functions: 41
Checked call to R_registerRoutines: 1
ERROR: too many states (abstraction error?) in function strptime_internal

Function LGBM_BoosterCreateFromModelfile_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:368

Function LGBM_BoosterCreate_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:355

Function LGBM_BoosterDumpModel_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:718

Function LGBM_BoosterGetEvalNames_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:521

Function LGBM_BoosterLoadModelFromString_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:381

Function LGBM_BoosterSaveModelToString_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:695

Function LGBM_DatasetCreateFromCSC_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:102

Function LGBM_DatasetCreateFromFile_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:70

Function LGBM_DatasetCreateFromMat_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:126

Function LGBM_DatasetGetFeatureNames_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:210

Function LGBM_DatasetGetSubset_R
  [PB] has possible protection stack imbalance /rchk/packages/build/ziRQDfzj/lightgbm/src/lightgbm_R.cpp:150
Analyzed 39640 functions, traversed 108278 states.

Rchk version: 3d653b7c8f92dac912532856b55f44d2986c6553
R version: 79889/R Under development (unstable) (2021-01-27 r79889)
LLVM version: 10.0.0

I've also opened #4400 to track the work for possibly adding a CI job.

[github-actions]

This issue has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.