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

Remove fast_unwind_on_malloc and malloc_context_size restrictions in ASAN CI #42515

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

tkf
Copy link
Member

@tkf tkf commented Oct 6, 2021

This implements @maleadt's suggestion #42498 (comment)

A quick benchmark (#42498 (comment)) suggests the peak memory consumption is about 5GB. I think the CI machines can handle this.

@tkf tkf added the ci Continuous integration label Oct 6, 2021
@vchuravy
Copy link
Member

vchuravy commented Oct 6, 2021 via email

Copy link
Member

@maleadt maleadt left a comment

Choose a reason for hiding this comment

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

LGTM.

The options were mainly for reducing memory usage when running the tests, which apparently we do not do on CI (understandably so, it takes a really long time to finish the test suite under ASAN, but maybe it should perform some smoke tests at least?).

@tkf
Copy link
Member Author

tkf commented Oct 7, 2021

There was a simple check to see if ASAN can detect a bug but we removed it #42229 since my implementation contrib/asan/check.jl was not reliable. But yes, it'd be nice to run some subset of tests. Maybe the ones that do some unsafe operations.

@DilumAluthge Do you want to wait for merging this until the issue in #41936 is resolved?

@DilumAluthge
Copy link
Member

Do you want to wait for merging this until the issue in #41936 is resolved?

Yeah I think that makes sense.

@maleadt
Copy link
Member

maleadt commented Nov 9, 2021

Rebased.

@vtjnash
Copy link
Member

vtjnash commented Nov 9, 2021

We need to fix the ASAN tests first (#42540)

@maleadt
Copy link
Member

maleadt commented Nov 9, 2021

Oh oops, I thought that had been fixed already. Sorry for the noise then.

@vtjnash vtjnash merged commit d261458 into JuliaLang:master Jan 26, 2022
@tkf tkf deleted the asan-unlimited branch January 26, 2022 22:35
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
…ASAN CI (JuliaLang#42515)

No need to set ASAN_OPTIONS any more either: these options are already
set in `loader_exe.c` with `__asan_default_options`.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
…ASAN CI (JuliaLang#42515)

No need to set ASAN_OPTIONS any more either: these options are already
set in `loader_exe.c` with `__asan_default_options`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants