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: Enhance sanitizer's config #603

Merged
merged 2 commits into from
Oct 20, 2020
Merged

ci: Enhance sanitizer's config #603

merged 2 commits into from
Oct 20, 2020

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Oct 14, 2020

No description provided.

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #603 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #603   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files          64       64           
  Lines        9432     9432           
=======================================
  Hits         9268     9268           
  Misses        164      164           

@chfast chfast force-pushed the ci_sanitizers branch 2 times, most recently from e31e0ad to 0c441d7 Compare October 14, 2020 08:39
@chfast chfast changed the title ci: Clean up sanitizer's config ci: Enhance sanitizer's config Oct 14, 2020
@chfast chfast requested review from axic and gumb0 October 14, 2020 11:01
CMakeLists.txt Show resolved Hide resolved
UBSAN_OPTIONS: halt_on_error=1
steps:
- install_testfloat
- checkout
- build:
build_type: RelWithDebInfo
# TODO: pointer-compare produces failure in std::string operator+.
Copy link
Member

Choose a reason for hiding this comment

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

This didn't even use pointer-compare, but I see you removed pointer-subtract -- perhaps that should be removed in the first commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

Choose a reason for hiding this comment

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

It seem it only removed the comment and not the line below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, fixed.

@@ -595,10 +591,10 @@ jobs:
rustup toolchain install nightly-x86_64-unknown-linux-gnu
- run:
name: Build (debug mode)
command: RUSTFLAGS="-Z sanitizer=address" ASAN_OPTIONS=detect_leaks=1 cargo +nightly build --target x86_64-unknown-linux-gnu
Copy link
Member

Choose a reason for hiding this comment

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

Ok since this on by default on Linux (not on MacOS): https://clang.llvm.org/docs/AddressSanitizer.html#memory-leak-detection

@@ -595,10 +591,10 @@ jobs:
rustup toolchain install nightly-x86_64-unknown-linux-gnu
- run:
name: Build (debug mode)
command: RUSTFLAGS="-Z sanitizer=address" ASAN_OPTIONS=detect_leaks=1 cargo +nightly build --target x86_64-unknown-linux-gnu
command: RUSTFLAGS="-Z sanitizer=address" cargo +nightly build --target x86_64-unknown-linux-gnu
Copy link
Member

Choose a reason for hiding this comment

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

Does asan does anything at compile time? If not we can remove this step.

Copy link
Member

Choose a reason for hiding this comment

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

Apparently it does some compile-thing things too: rust-lang/rust#45220

Any we should also pass on sanitize mode to cmake: https://users.rust-lang.org/t/sanitizers-in-mixed-rust-and-c-code/48863/4

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You should be building fizzy with -DSANITIZE=address.

@chfast chfast force-pushed the ci_sanitizers branch 2 times, most recently from d6ede7e to 0e3296a Compare October 15, 2020 18:59
@chfast chfast merged commit 5db71c8 into master Oct 20, 2020
@chfast chfast deleted the ci_sanitizers branch October 20, 2020 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants