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

Add ASAN CircleCI Run #7027

Closed
wants to merge 3 commits into from
Closed

Add ASAN CircleCI Run #7027

wants to merge 3 commits into from

Conversation

siying
Copy link
Contributor

@siying siying commented Jun 24, 2020

ASAN run is powerful in finding memory leak bugs. Running it as a part of the pre-merge CI can help contributors avoid to merge some code with bugs.

Test Plan: Watch the test result.

Copy link
Contributor

@mrambacher mrambacher left a comment

Choose a reason for hiding this comment

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

Does this change supercede #6659?

@siying
Copy link
Contributor Author

siying commented Jun 24, 2020

@mrambacher hmm... maybe. How should we proceed? Wait for #6659, or make #6659 rebase later?

@mrambacher
Copy link
Contributor

@siying If this one is ready to go, why don't you merge it and I will update whatever is left for #6659 after this one is committed.

@siying
Copy link
Contributor Author

siying commented Jun 24, 2020

@siying If this one is ready to go, why don't you merge it and I will update whatever is left for #6659 after this one is committed.

I just merged those code changes in #7025

@siying siying changed the title Cci asan Add ASAN CircleCI Run Jun 24, 2020
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

@siying has updated the pull request. Re-import the pull request

@siying siying requested a review from pdillinger June 25, 2020 01:05
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@pdillinger pdillinger left a comment

Choose a reason for hiding this comment

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

Optional clean-up

steps:
- checkout # check out the code in the project directory
- run: pyenv global 3.5.2
- run: sudo sh -c 'echo "deb http://apt.llvm.org/xenial/ llvm-toolchain-xenial-10 main" >> /etc/apt/sources.list'
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional: you could probably simplify to echo "stuff" | sudo tee -a /etc/apt/sources.list

- run: sudo apt-get update -y
- run: sudo apt-get install -y clang-10
- run: sudo apt-get install -y libgflags-dev
- run: SKIP_FORMAT_BUCK_CHECKS=1 COMPILE_WITH_ASAN=1=1 CC=clang-10 CXX=clang++-10 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 PRINT_PARALLEL_OUTPUTS=1 make asan_check -j32 # aligned new doesn't work for reason we haven't figured out
Copy link
Contributor

Choose a reason for hiding this comment

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

Unintentional =1=1 but I think still works

@facebook-github-bot
Copy link
Contributor

@siying has updated the pull request. Re-import the pull request

@facebook-github-bot
Copy link
Contributor

@siying has updated the pull request. Re-import the pull request

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@siying has updated the pull request. Re-import the pull request

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@siying has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7006997.

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.

4 participants