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

Fix ASAN issues with std::function usage #4673

Merged
merged 2 commits into from
Oct 20, 2021
Merged

Conversation

david-cortes
Copy link
Contributor

The C API uses a C++ std::function which gets compared against a nullptr to check if it assigned, which is not correct. This PR makes small changes to make the usage more idiomatic C++.

I think this should fix the (or one of the) issue(s) highlighted by CRAN: #4633

But I'm not sure if this is all that was wrong or if there are more instances of usages like these, or if this will affect anything else other than the dataset construction in the C API.

@jameslamb
Copy link
Collaborator

Thank you so so so much for the quick response helping us with these issues from CRAN!!!

I've created #4674 to track the discussion around this issue, and so we can reference it in the future. I think these changes look good, but I'd like to try to see if I can replicate the errors CRAN is reporting by following https://www.stats.ox.ac.uk/pub/bdr/memtests/README.txt, then test this PR with that setup. I'm going to attempt that tonight or tomorrow.

@jameslamb
Copy link
Collaborator

I was able to reproduce the ASAN errors CRAN reported tonight.

I tried those same steps on this branch to see if the changes here resolve the issue, and it seems that they don't.

code I ran to test this branch with ASAN
git clone \
    --recursive \
    --branch fptr \
    git@github.com:david-cortes/LightGBM.git \
    david-cortes-lgb

cd david-cortes-lgb

docker run \
  --rm \
  -v $(pwd):/usr/LightGBM \
  --workdir /usr/LightGBM \
  -it \
  wch1/r-debug:latest \
  /bin/bash

export ASAN_OPTIONS="detect_leaks=0:detect_odr_violation=0"
export UBSAN_OPTIONS="print_stacktrace=1"
export RJAVA_JVM_STACK_WORKAROUND=0
export RGL_USE_NULL=true
export R_DONT_USE_TK=true

RDscriptsan -e "install.packages(c('R6', 'data.table', 'jsonlite', 'Matrix', 'testthat'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())"

sh build-cran-package.sh

RDsan CMD INSTALL lightgbm_*.tar.gz
cd R-package/tests
RDscriptsan testthat.R >> tests.log 2>&1

# check that tests passed
echo "test exit code: $?"
tail -300 ./tests.log

I see the same issue in logs as CRAN noted:

=================================================================
==2414==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffc64e94c30 at pc 0x7f1637e5cfe1 bp 0x7ffc64e94350 sp 0x7ffc64e94340

full logs
=================================================================
==2414==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffc64e94c30 at pc 0x7f1637e5cfe1 bp 0x7ffc64e94350 sp 0x7ffc64e94340
READ of size 8 at 0x7ffc64e94c30 thread T0
    #0 0x7f1637e5cfe0 in std::_Function_base::_M_empty() const /usr/include/c++/10/bits/std_function.h:248
    #1 0x7f163838e897 in std::function<void (std::vector<std::pair<int, double>, std::allocator<std::pair<int, double> > > const&, double*)>::operator bool() const /usr/include/c++/10/bits/std_function.h:499
    #2 0x7f163836dfd3 in LightGBM::DatasetLoader::ExtractFeaturesFromMemory(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >*, LightGBM::Parser const*, LightGBM::Dataset*) io/dataset_loader.cpp:1146
    #3 0x7f163834f6d5 in LightGBM::DatasetLoader::LoadFromFile(char const*, int, int) io/dataset_loader.cpp:223
    #4 0x7f1638ddf470 in LightGBM::DatasetLoader::LoadFromFile(char const*) include/LightGBM/dataset_loader.h:25
    #5 0x7f1638db40db in LGBM_DatasetCreateFromFile /tmp/RtmpPISuUd/R.INSTALL73c72bc0edc/lightgbm/src/c_api.cpp:960
    #6 0x7f1638e6b4a9 in LGBM_DatasetCreateFromFile_R /tmp/RtmpPISuUd/R.INSTALL73c72bc0edc/lightgbm/src/lightgbm_R.cpp:123
    #7 0x7f16468f78a9 in R_doDotCall /tmp/r-source/src/main/dotcode.c:607
    #8 0x7f1646913c85 in do_dotcall /tmp/r-source/src/main/dotcode.c:1284
    #9 0x7f1646998441 in Rf_eval /tmp/r-source/src/main/eval.c:843
    #10 0x7f16469a5ce0 in do_set /tmp/r-source/src/main/eval.c:2982
    #11 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #12 0x7f16469a3267 in do_begin /tmp/r-source/src/main/eval.c:2530
    #13 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #14 0x7f16469a05d8 in do_if /tmp/r-source/src/main/eval.c:2257
    #15 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #16 0x7f16469a3267 in do_begin /tmp/r-source/src/main/eval.c:2530
    #17 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #18 0x7f16469a05d8 in do_if /tmp/r-source/src/main/eval.c:2257
    #19 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #20 0x7f16469a3267 in do_begin /tmp/r-source/src/main/eval.c:2530
    #21 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #22 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #23 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #24 0x7f16469986f5 in Rf_eval /tmp/r-source/src/main/eval.c:863
    #25 0x7f16469a3267 in do_begin /tmp/r-source/src/main/eval.c:2530
    #26 0x7f1646997ea7 in Rf_eval /tmp/r-source/src/main/eval.c:815
    #27 0x7f16469a843d in do_eval /tmp/r-source/src/main/eval.c:3357
    #28 0x7f16469c5547 in bcEval /tmp/r-source/src/main/eval.c:7128
    #29 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #30 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #31 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #32 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #33 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #34 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #35 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #36 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #37 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #38 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #39 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #40 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #41 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #42 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #43 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #44 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #45 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #46 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #47 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #48 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #49 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #50 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #51 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #52 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #53 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #54 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #55 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #56 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #57 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #58 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #59 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #60 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #61 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #62 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #63 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #64 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #65 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #66 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #67 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #68 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #69 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #70 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #71 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #72 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #73 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #74 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #75 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #76 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #77 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #78 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #79 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #80 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #81 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #82 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #83 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #84 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #85 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #86 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #87 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #88 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #89 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #90 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #91 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #92 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #93 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #94 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #95 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #96 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #97 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #98 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #99 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #100 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #101 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #102 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #103 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #104 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #105 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #106 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #107 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #108 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #109 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #110 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #111 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #112 0x7f16469986f5 in Rf_eval /tmp/r-source/src/main/eval.c:863
    #113 0x7f16469a87c0 in do_eval /tmp/r-source/src/main/eval.c:3380
    #114 0x7f16469c5547 in bcEval /tmp/r-source/src/main/eval.c:7128
    #115 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #116 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #117 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #118 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #119 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #120 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #121 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #122 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #123 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #124 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #125 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #126 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #127 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #128 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #129 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #130 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #131 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #132 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #133 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #134 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #135 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #136 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #137 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #138 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #139 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #140 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #141 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #142 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #143 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #144 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #145 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #146 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #147 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #148 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #149 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #150 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #151 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #152 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #153 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #154 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #155 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #156 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #157 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #158 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #159 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #160 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #161 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #162 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #163 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #164 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #165 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #166 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #167 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #168 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #169 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #170 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #171 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #172 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #173 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #174 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #175 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #176 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #177 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #178 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #179 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #180 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #181 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #182 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #183 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #184 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #185 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #186 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #187 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #188 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #189 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #190 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #191 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #192 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #193 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #194 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #195 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #196 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #197 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #198 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #199 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #200 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #201 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #202 0x7f164699eb7c in R_forceAndCall /tmp/r-source/src/main/eval.c:1977
    #203 0x7f164680f982 in do_lapply /tmp/r-source/src/main/apply.c:70
    #204 0x7f1646a85aa0 in do_internal /tmp/r-source/src/main/names.c:1396
    #205 0x7f16469c5a8e in bcEval /tmp/r-source/src/main/eval.c:7148
    #206 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #207 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #208 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #209 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #210 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #211 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #212 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #213 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #214 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #215 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #216 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #217 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #218 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #219 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #220 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #221 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #222 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #223 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #224 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #225 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #226 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #227 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #228 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #229 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #230 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #231 0x7f1646996977 in forcePromise /tmp/r-source/src/main/eval.c:568
    #232 0x7f16469ae842 in FORCE_PROMISE /tmp/r-source/src/main/eval.c:5149
    #233 0x7f16469aebf2 in getvar /tmp/r-source/src/main/eval.c:5190
    #234 0x7f16469be302 in bcEval /tmp/r-source/src/main/eval.c:6880
    #235 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #236 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #237 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #238 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #239 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #240 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #241 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #242 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #243 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #244 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #245 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836
    #246 0x7f16469c4c71 in bcEval /tmp/r-source/src/main/eval.c:7096
    #247 0x7f164699751c in Rf_eval /tmp/r-source/src/main/eval.c:740
    #248 0x7f164699dbdb in R_execClosure /tmp/r-source/src/main/eval.c:1910
    #249 0x7f164699d2cc in Rf_applyClosure /tmp/r-source/src/main/eval.c:1836

Address 0x7ffc64e94c30 is located in stack of thread T0 at offset 416 in frame
    #0 0x7f1638db3de1 in LGBM_DatasetCreateFromFile /tmp/RtmpPISuUd/R.INSTALL73c72bc0edc/lightgbm/src/c_api.cpp:949

  This frame has 6 object(s):
    [32, 33) '<unknown>'
    [48, 104) 'param' (line 951)
    [144, 328) 'loader' (line 957)
    [400, 432) '<unknown>' <== Memory access at offset 416 is inside this variable
    [464, 496) '<unknown>'
    [528, 2048) 'config' (line 952)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-scope /usr/include/c++/10/bits/std_function.h:248 in std::_Function_base::_M_empty() const
Shadow bytes around the buggy address:
  0x10000c9ca930: f2 f2 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00
  0x10000c9ca940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca950: 00 00 f1 f1 f1 f1 01 f2 00 00 00 00 00 00 00 f2
  0x10000c9ca960: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca970: 00 00 00 00 00 00 00 00 00 00 00 f2 f2 f2 f2 f2
=>0x10000c9ca980: f2 f2 f2 f2 f8 f8[f8]f8 f2 f2 f2 f2 00 00 00 00
  0x10000c9ca990: f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca9a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca9b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca9c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10000c9ca9d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==2414==ABORTING

Could you take a look at the code I used above and see if you see any mistakes? If not, then this change probably does not fix the issue CRAN has noted.

@david-cortes
Copy link
Contributor Author

That's interesting - just noticed that CRAN shows two different errors, and the one I was thinking about was this:
https://www.stats.ox.ac.uk/pub/bdr/memtests/clang-ASAN/lightgbm/tests/testthat.Rout.fail

Perhaps there's another problem when it is not assigned a null pointer, but from a look at the trace, I'd venture to guess it could be a bug in the compiler header. Will investigate later.

@david-cortes
Copy link
Contributor Author

Did a bit more investigation:

  • Just noticed that the variable that's used-after-scope is a const reference. Changing it to a const non-reference solves the issue, at least as tested with RDsan. I don't actually see any other problematic usage and I guess the issue might either be in the compiler headers or be something about incorrect header declarations within lightgbm (not sure where).
  • The problematic variable is actually never used in the R interface. All the instantiations of DatasetLoader initialize it to a nullptr, and since it is const, it cannot be changed after-the-fact.
  • The problematic variable is used in the CLI interface with a non-nullptr initialization. Not sure if anything would break on it with this change.

@david-cortes david-cortes changed the title Don't compare std::function to nullptr Fix ASAN issues with std::function usage Oct 14, 2021
@jameslamb
Copy link
Collaborator

Thank you so much!

Let's see what the CI jobs say about whether this change would break other things. I can also test with the gcc-ASAN and clang-ASAN tests proposed in #4678.

@jameslamb
Copy link
Collaborator

closing and re-opening to retrigger CI, since we've recently fixed some configuration issues with Appveyor and Azure Pipelines jobs (#4675 (comment)).

@jameslamb jameslamb closed this Oct 15, 2021
@jameslamb jameslamb reopened this Oct 15, 2021
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I just tested this locally with the following code that seems to successfully reproduce CRAN's gcc-ASAN and clang-ASAN tests (as described in #4674 (comment) and in progress at #4678).

testing code (click me)
git clone \
    --recursive \
    --branch fptr \
    git@github.com:david-cortes/LightGBM.git \
    david-cortes-lgb

cd david-cortes-lgb

#
# options for CUSTOMIZATION:
#
#   - "san" = gcc-ASAN
#   - "csan" = clang-ASAN
#
docker run \
  --rm \
  -v $(pwd):/usr/LightGBM \
  --workdir /usr/LightGBM \
  --env CUSTOMIZATION=csan \
  -it \
  wch1/r-debug:latest \
  /bin/bash


RDscript${CUSTOMIZATION} \
  -e "install.packages(c('R6', 'data.table', 'jsonlite', 'Matrix', 'testthat'), repos = 'https://cran.r-project.org', Ncpus = parallel::detectCores())"

sh build-cran-package.sh

RD${CUSTOMIZATION} \
  CMD INSTALL lightgbm_*.tar.gz

cd R-package/tests
rm -f ./tests.log
RDscript${CUSTOMIZATION} testthat.R >> tests.log 2>&1

# check that tests passed
echo "test exit code: $?"
tail -300 ./tests.log

No issues were raised by those tests as of the current state of this branch 🎉

So I'm confident that when {lightgbm} 3.3.1 is submitted, this change will resolve the errors CRAN has noted in clang-ASAN and gcc-ASAN checks.

Thanks so much for the help, @david-cortes !

@StrikerRUS
Copy link
Collaborator

StrikerRUS commented Oct 16, 2021

The problematic variable is used in the CLI interface with a non-nullptr initialization. Not sure if anything would break on it with this change.

Let's see what the CI jobs say about whether this change would break other things.

Unfortunately, we are not testing CLI version at CI.

@shiyu1994 shiyu1994 merged commit 13ed38c into microsoft:master Oct 20, 2021
jameslamb added a commit that referenced this pull request Oct 22, 2021
jameslamb added a commit that referenced this pull request Oct 24, 2021
StrikerRUS added a commit that referenced this pull request Oct 26, 2021
…ixes #4674) (#4678)

* add jobs mimicking CRAN gcc-ASAN and clang-ASAN

* comment out CI

* fix redirection

* remove unnecessary echo

* Revert "comment out CI"

This reverts commit 899fbb4.

* remove redundant env variables and update README

* remove inaccurate comment

* change test title

* Revert "Fix ASAN issues with `std::function` usage (#4673)"

This reverts commit 13ed38c.

* Revert "Revert "Fix ASAN issues with `std::function` usage (#4673)""

This reverts commit 24c275b.

* revert unnecessary change in config order

* Apply suggestions from code review

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
@github-actions
Copy link

This pull request 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants