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

[ci] [R-package] re-enable 'rchk' checks #6713

Merged
merged 5 commits into from
Nov 5, 2024
Merged

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Nov 3, 2024

Follow-up to #6545

Proposes re-enabling the rchk checks, now that that tool can successfully check lightgbm's source code: kalibera/rchk#35 (comment)

How I tested this

Intentionally caused a stack imbalance by reducing the count in an Rf_UNPROTECT() call by 1: https://github.com/microsoft/LightGBM/pull/6713/files#diff-effa19f4965843414974b8ad1ddcf65710b25c978e203317ce409749841da338

Saw rchk catch it!

Function make_altrepped_raw_vec(void*)
  [PB] has possible protection stack imbalance /__w/LightGBM/LightGBM/lightgbm/src/lightgbm_R.cpp:71
Analyzed 66521 functions, traversed 184251 states.
------------------------------------------------------
rchk found issues
image

(build link)

Then reverted that, and saw rchk exit normally and not report any issues:

Analyzed 66521 functions, traversed 184251 states.
------------------------------------------------------
rchk did not find any issues

(build link)

@jameslamb jameslamb changed the title WIP: [ci] [R-package] re-enable 'rchk' checks [ci] [R-package] re-enable 'rchk' checks Nov 4, 2024
@jameslamb jameslamb marked this pull request as ready for review November 4, 2024 03:45
Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Thank you so much for going to the end with rchk!

@StrikerRUS StrikerRUS merged commit 5151fe8 into master Nov 5, 2024
49 checks passed
@StrikerRUS StrikerRUS deleted the ci/restore-rchk branch November 5, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants