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

ci: enable UBSAN check in ci #4734

Closed
wants to merge 4 commits into from
Closed

Conversation

boquan-fang
Copy link
Contributor

@boquan-fang boquan-fang commented Aug 26, 2024

Resolved issues:

Enable UBSAN check for build batch CI. Resolve requirement 2 of #4684.

Description of changes:

  • create buildspec_address_sanitizer.yml to create build batch for both UBSAN and ASAN check.
  • create buildspec_ubsan.yml to specify build check process.

Call-outs:

Testing:

  • Run manual override test on AWS console. Every builds are successfully passed.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* create buildspec_address_sanitizer.yml to create build batch for both
  UBSAN and ASAN check.
* create buildspec_ubsan.yml to specify build check process.
@github-actions github-actions bot added the s2n-core team label Aug 26, 2024
@boquan-fang boquan-fang requested a review from goatgoose August 26, 2024 23:37
@boquan-fang boquan-fang marked this pull request as ready for review August 26, 2024 23:47
@boquan-fang boquan-fang requested a review from dougch as a code owner August 26, 2024 23:47
codebuild/spec/buildspec_address_sanitizer.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_address_sanitizer.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_address_sanitizer.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_new_asan.yml Outdated Show resolved Hide resolved
* clarify comments to make them previous and to locate them in
  appropriate locations.
* change file name for address_sanitizer to sanitizer, so that it
  reflects the purpose of this file.
@boquan-fang boquan-fang requested a review from goatgoose August 27, 2024 16:55
codebuild/spec/buildspec_ubsan.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_ubsan.yml Outdated Show resolved Hide resolved
@lrstewart lrstewart self-requested a review August 27, 2024 18:14
* Move comments for complier to buildspec_sanitizer.yml.
* Fix a typo in UBSAN file.
Copy link
Contributor

Choose a reason for hiding this comment

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

From my understanding, this is the real replacement for buildspec_asan.yml, while buildspec_new_asan.yml is basically just something new that happens to have the same name. How can you prove that you're covering all the same cases that buildspec_asan.yml currently covers, and that no test cases will be lost in the transition? This PR doesn't really diff anything with our current state, which makes it very difficult to reliably review.

I'm also not convinced that you can't just delete buildspec_asan.yml immediately. What additional testing do you plan to do before deleting it?

@boquan-fang
Copy link
Contributor Author

Replace this PR with PR#4740.

@boquan-fang boquan-fang deleted the add-ubsan-ci branch December 18, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants