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.

Sign up for GitHub

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

Jump to bottom

CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite #41530

Merged
merged 12 commits into from
Jul 13, 2021
Merged

CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite #41530

merged 12 commits into from
Jul 13, 2021

Conversation

tkf
Copy link
Member

@tkf tkf commented Jul 9, 2021

This is a continuation of #41522 and simply "codifies" what is already documented.

@staticfloat @DilumAluthge I mostly just copied llvmpasses. Let me know if I need to fix something.

@tkf tkf requested a review from a team as a code owner July 9, 2021 23:48
@DilumAluthge DilumAluthge requested review from DilumAluthge and staticfloat and removed request for a team July 10, 2021 01:15
@DilumAluthge DilumAluthge added the ci Continuous integration label Jul 10, 2021
@DilumAluthge DilumAluthge changed the title Setup CI for ASAN CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite Jul 10, 2021
@DilumAluthge
Copy link
Member

Instead of trying to write stuff into /tmp, can you just do make in-place, like we do in the llvm passes?

@tkf
Copy link
Member Author

tkf commented Jul 10, 2021

Hmm... I kinda want to keep the out-of-tree -based script so that it is easy to try it out. It's also nice to have a separated toolchain directory so that you don't need to re-install the toolchain (e.g., we can have contrib/tsan.sh that shares the toolchain with contrib/asan.sh).

Maybe we can use ./tmp (which is already listed in .gitignore)?

@DilumAluthge
Copy link
Member

Maybe we can use ./tmp (which is already listed in .gitignore)?

Sure, let's give that a try.

@tkf
Copy link
Member Author

tkf commented Jul 10, 2021

image

Too long? (Interesting that it's just two seconds less than the time limit)

tkf
tkf commented Jul 10, 2021
View reviewed changes
.buildkite/sanitizers.yml Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Member

image

Too long? (Interesting that it's just two seconds less than the time limit)

We should probably increase the timeout for that specific pipeline to be 120 minutes.

@DilumAluthge
Copy link
Member

It would be good to edit those echo commands to be a little more descriptive. For example, what does contrib/asan.sh do, and what does contrib/check-asan.jl do?

@DilumAluthge DilumAluthge mentioned this pull request Jul 10, 2021
Closed
34 tasks
@DilumAluthge DilumAluthge added the building Build system, or building Julia or its dependencies label Jul 10, 2021
@DilumAluthge
Copy link
Member

DilumAluthge commented Jul 10, 2021 •
edited
Loading

LGTM.

@vchuravy
Copy link
Member

One issue here is that the LLVM used is not built with ASAN itself, normally we recommend USE_BINARYBUILDER_LLVM=0 for the second stage to build LLVM itself with ASAN, but that might be to expensive to do on CI and we would need to maintain a binary cache (like for the assert build)

@vchuravy vchuravy requested a review from maleadt July 10, 2021 16:28
@tkf
Copy link
Member Author

tkf commented Jul 10, 2021

I think it's still nice to make sure our codebase itself is ASAN compatible; i.e., the code path with JL_ASAN_ENABLED is compilable and there is no apparent problem in the run-time that can be detected with ASAN.

staticfloat
staticfloat approved these changes Jul 13, 2021
View reviewed changes
contrib/asan/Make.user.asan Outdated Show resolved Hide resolved
@DilumAluthge DilumAluthge added merge me PR is reviewed. Merge when all tests are passing backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Jul 13, 2021
@staticfloat staticfloat merged commit 84934e6 into JuliaLang:master Jul 13, 2021
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jul 13, 2021
@tkf tkf deleted the ci-asan branch July 13, 2021 23:54
KristofferC pushed a commit that referenced this pull request Jul 19, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
e133d5c 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
KristofferC pushed a commit that referenced this pull request Jul 19, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
531d4d9 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
@KristofferC KristofferC mentioned this pull request Jul 19, 2021
Merged
75 tasks
KristofferC pushed a commit that referenced this pull request Jul 19, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
1aecc99 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
@KristofferC KristofferC mentioned this pull request Jul 19, 2021
Merged
82 tasks
@tkf tkf removed backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Jul 19, 2021
@tkf
Copy link
Member Author

tkf commented Jul 19, 2021

Maybe better not backporting this?
#41554 (comment)
#41499 (comment)

@DilumAluthge DilumAluthge added backport 1.6 Change should be backported to release-1.6 backport 1.7 labels Jul 20, 2021
@DilumAluthge
Copy link
Member

This is my thinking: the CI PRs that we've been making tend to build on one another. So it will probably be cleanest just to backport all CI PRs.

Once I merge #41606, it can be backported. One of the things that #41606 does is that it only runs asan on master. So if we backport #41606 to 1.6, then asan won't run on the release-1.6 branch, which I think is the correct behavior.

@tkf
Copy link
Member Author

tkf commented Jul 20, 2021

Thanks for the explanation. I saw your comments in #41554 (comment) too and I see that it makes sense either way.

KristofferC pushed a commit that referenced this pull request Jul 26, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
903ae5a 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
KristofferC pushed a commit that referenced this pull request Jul 26, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
a0fd992 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
@KristofferC KristofferC mentioned this pull request Aug 25, 2021
Merged
63 tasks
KristofferC pushed a commit that referenced this pull request Aug 31, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
3a8173a 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
KristofferC pushed a commit that referenced this pull request Sep 3, 2021
@tkf @KristofferC
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
7e61365 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Sep 7, 2021
@tkf tkf mentioned this pull request Oct 1, 2021
Merged
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
@tkf @staticfloat
CI: add the sanitizers pipelines (e.g. ASAN) to Buildkite (#41530)
ede817c 
* Setup CI for ASAN

* Launch the `sanitizers.yml` unsigned pipeline

* Use a workspace directory in ./tmp

* Add some log group headers to make the logs easier to navigate

* Install `julia` binary inside sandbox

* Double timeout

* More descriptive message from sanitizer CI

* Fix the path to the binary

* Use addenv

* Apply suggestions from code review

Co-authored-by: Elliot Saba <[email protected]>

* Group ASAN related files under contrib/asan/

* Remove redundant JULIA_PRECOMPILE=1

Co-authored-by: Dilum Aluthge <[email protected]>
Co-authored-by: Elliot Saba <[email protected]>
(cherry picked from commit 84934e6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DilumAluthge DilumAluthge DilumAluthge approved these changes

@staticfloat staticfloat staticfloat approved these changes

@maleadt maleadt Awaiting requested review from maleadt

Assignees
No one assigned
Labels
building Build system, or building Julia or its dependencies ci Continuous integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tkf @DilumAluthge @vchuravy @staticfloat @KristofferC