[ci] cmake: remove linking to sanitizer library

[cyfdecyf]

Adding compiler flag will automatically link to the required libraries.

Besides, clang static links to sanitizer libraries by default. Thus linking to dynamic library must be removed in order to use clang.

For reference:

• Difference of AddressSanitizer in Clang and GCC
• StackOverflow answer about -lasan option has been discouraged. It directs to mailing list answer by ASan developer
• LeakSanitizer doc mentions that standalone mode can work without the slowdown of ASan. This mode will link to "a runtime library containing just the bare necessities required for LeakSanitizer to work". For GCC,
  • when turning on both ASan and LSan, the executable only links with libasan without liblsan
  • when disabling ASan and enabling LSan, liblsan will be linked

[cyfdecyf]

The "Python-package / mpi source (macOS-latest, Python 3.9)" failure should not be caused by this PR.

[jameslamb]

The "Python-package / mpi source (macOS-latest, Python 3.9)" failure should not be caused by this PR.

hmmm ok yeah I suspect you're right.

When commenting on CI jobs, it's usually desirable to leave a link to the logs so maintainers can go look: https://github.com/microsoft/LightGBM/pull/4176/checks?check_run_id=2342784529.

E   ImportError: 
E   
E   IMPORTANT: PLEASE READ THIS FOR ADVICE ON HOW TO SOLVE THIS ISSUE!
E   
E   Importing the numpy C-extensions failed. This error can happen for
E   many reasons, often due to issues with your setup or how NumPy was
E   installed.
E   
E   We have compiled some common reasons and troubleshooting tips at:
E   
E       https://numpy.org/devdocs/user/troubleshooting-importerror.html
E   
E   Please note and check the following:
E   
E     * The Python version is: Python3.9 from "/Users/runner/miniconda/envs/test-env/bin/python"
E     * The NumPy version is: "1.19.2"
E   
E   and make sure that they are the versions you expect.
E   Please carefully study the documentation linked above for further help.
E   
E   Original error was: dlopen(/Users/runner/miniconda/envs/test-env/lib/python3.9/site-packages/numpy/core/_multiarray_umath.cpython-39-darwin.so, 2): Library not loaded: @rpath/libopenblas.dylib
E     Referenced from: /Users/runner/miniconda/envs/test-env/lib/python3.9/site-packages/numpy/core/_multiarray_umath.cpython-39-darwin.so
E     Reason: image not found

Can you please push an empty commit to this branch to re-trigger CI? So it will rebuild but we won't lose the logs.

[cyfdecyf]

@jameslamb Didn't realise that I can put a link commenting about CI.

Empty commit pushed now.

[jameslamb]

Didn't realise that I can put a link commenting about CI.

No problem! If you ever need to look, you can click on "Details" next to a check to see its logs.

Empty commit pushed now

Thanks, seems like all checks are passing now 🎉

[jameslamb]

I'm removing my request-for-review on this one, I don't think I'm qualified to approve it. Thanks very much for the help, @cyfdecyf !

If you have time, could you please update this to the latest master? Someone from the team will review it soon.

[StrikerRUS]

Hey @hcho3 @trivialfis ! Could you please take a look at #4176 (comment)?

[trivialfis]

I think the changes make sense. In older gcc versions linking those libraries was required. But the requirement was removed in newer versions. In fact, with the latest gcc distribution, those libraries are hidden deeply as internal shared objects so those CMake scripts will stop working.

In XGBoost the find_package is kept but the REQUIRED argument is removed.

[cyfdecyf]

@jameslamb OK. I just did a rebase to latest master commit and force-pushed.

[StrikerRUS]

@trivialfis Thanks a lot for your comment!

@cyfdecyf Could you please share versions of compilers you tested your changes from this PR with? We need to support quite wide range of versions of gcc and Clang. But gcc 4.8.4 is must-have because with this version we create public artifacts.

[cyfdecyf]

@StrikerRUS Versions of compiler tested (all from Ubuntu 18.04):

• Clang 7.0.0
• GCC 7.5.0
• GCC 4.8.5
  • Note: seems only AddressSanitizer is supported for GCC 4.8.5. Must run cmake as cmake -DUSE_SANITIZER=on -DENABLED_SANITIZERS=address. Otherwise cmake will fail because unrecongnized command line option.
  • I guess the public artifacts build options won't enable sanitizers, so this PR shouldn't cause any problems in this regard
  • Besides, as it's better to use newer version of compiler when using sanitizers, I suggest just ignore this issue

[StrikerRUS]

@cyfdecyf Thanks for the clarification!

Note: seems only AddressSanitizer is supported for GCC 4.8.5.

Are you speaking about the current master branch or version in this PR?

I guess the public artifacts build options won't enable sanitizers, so this PR shouldn't cause any problems in this regard

Yes, sure! Public artifacts are built with optimization flags and without any sanitizers.

Besides, as it's better to use newer version of compiler when using sanitizers, I suggest just ignore this issue

Initially I thought about testing with sanitizers the same version of compiler with which public releases are made. To make sure that version of compiler doesn't find any issues. Now I believe your proposal to prefer testing with sanitizers only the latest compiler version makes sense. Let me think about it a little bit more time.

[cyfdecyf]

@cyfdecyf Thanks for the clarification!

Note: seems only AddressSanitizer is supported for GCC 4.8.5.

Are you speaking about the current master branch or version in this PR?

It's the problem of GCC 4.8.5 itself. If we don't limit to only ASan during cmake invocation when using GCC 4.8.5, building will result compiling error because g++ doesn't recognize -fsanitize=leak, -fsanitize=undefined options.

[StrikerRUS]

OK, thanks a lot for your explanations @cyfdecyf ! Now I think running sanitizers only with the latest versions of compilers is a good idea!

[github-actions]

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.