Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[R-package] [ci] Add a CI job testing the R package with rchk #4400

Closed
jameslamb opened this issue Jun 23, 2021 · 2 comments · Fixed by #4449
Closed

[R-package] [ci] Add a CI job testing the R package with rchk #4400

jameslamb opened this issue Jun 23, 2021 · 2 comments · Fixed by #4449

Comments

@jameslamb
Copy link
Collaborator

jameslamb commented Jun 23, 2021

Summary

A continuous integration (CI) job should be added which tests the R package with rchk.

From that project's description,

This project consists of several bug-finding tools that look for memory protection errors in C source code using R API, that is in the source code of R itself and packages. The tools perform whole-program static analysis on LLVM bitcode and run on Linux.

Motivation

rchk can automatically find issues in C/C++ code used in an R package. Adding a CI job in this repository which runs rchk should provide higher confidence in releases and aid maintainers in catching user-facing issues before they make it into releases.

For example, I found that rchk run on the most recent CRAN release (3.2.1) would have caught the bug that led to #4045 (fixed in #4155).

ERROR: initialization function R_init_lightgbm(_DllInfo*) in C++ will not be used by R

Following the instructions at https://github.com/kalibera/rchk/blob/master/doc/DOCKER.md#checking-a-package-from-a-tarball to check the latest version of {lightgbm} on CRAN.

wget https://cran.r-project.org/src/contrib/lightgbm_3.2.1.tar.gz
mkdir packages
cp lightgbm_3.2.1.tar.gz packages
docker run -v `pwd`/packages:/rchk/packages kalibera/rchk:latest /rchk/packages/lightgbm_3.2.1.tar.gz

Results in logs that end as follows.

Installed libraries of package  lightgbm 
[1] "/rchk/packages/libsonly/lightgbm/libs/lightgbm.so"

Library name (usually package name): lightgbm
ERROR: initialization function R_init_lightgbm(_DllInfo*) in C++ will not be used by R
ERROR: did not find initialization function R_init_lightgbm
ERROR: too many states (abstraction error?) in function strptime_internal
Analyzed 39643 functions, traversed 106572 states.

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

Description

If this job can be run on CI in less than 20 minutes, I think it should be included in the standard R checks run on every commit (https://github.com/microsoft/LightGBM/blob/47208894d819f1b8cef193e71f78e9a8c2f38462/.github/workflows/r_package.yml).

If not, it should be added as a standalone job that can be triggered by a comment like /gha run r-rchk. See https://github.com/microsoft/LightGBM/blob/47208894d819f1b8cef193e71f78e9a8c2f38462/.github/workflows/r_valgrind.yml for an example.

NOTE: when implementing this, the error too many states (abstraction error) in strptime_internal can safely be ignored. See kalibera/rchk#22 (comment).

References

Many thanks to @fabsig for bringing this up in #4390.

See #4390 (comment) for some details on the current state of this tool's output against master of LightGBM.

@jameslamb
Copy link
Collaborator Author

Per our policy in this project, I've added this feature request to #2302. Anyone is welcome to contribute this feature! If you'd like to contribute, say so in a comment here and this can be re-opened.

@jameslamb
Copy link
Collaborator Author

re-opening this as I am actively working on it.

@jameslamb jameslamb reopened this Jul 6, 2021
StrikerRUS pushed a commit that referenced this issue Jul 10, 2021
* [ci] add CI job running rchk

* try commenting out more stuff

* ignore R internal error

* pipes

* remove PROTECT()

* try removing testthat

* revert temporary testing changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant