Skip to content

Add valid arguments to -sanitize-coverage: pc-table and inline-8bit-counter. #73587

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

Merged

Conversation

thetruestblue
Copy link
Contributor

As is, you cannot set all default fuzzer options directly using -sanitize-coverage. Because of this you can use default fuzzer sanitize-coverage args, or a limited number coverage options.

This PR adds pc-table and inline-8bit-counter as valid args to be parsed for sanitize-coverage flag. These are default fuzzer options -- and will allow customizing sanitizer-coverage in variations of fuzzer defaults. (i.e. w/o pc-table enabled).

In upstream clang the option -fno-sanitize-coverage exists to disable various coverage options. Swift currently does not have a convention of using excluding args for sanitizer flags. This aims to limit the need for a new flag by inclusively setting desired coverage up to current fuzzer defaults.

Corresponding swift-driver change: swiftlang/swift-driver#1604

rdar://127881891

@thetruestblue
Copy link
Contributor Author

@swift-ci test

@thetruestblue thetruestblue marked this pull request as draft May 13, 2024 14:33
…counter.

As is, you cannot set all default fuzzer options directly using -sanitize-coverage. Because of this you can use default fuzzer sanitize-coverage args, or a limited number coverage options.

This PR adds pc-table and inline-8bit-counter as valid args to be parsed for sanitize-coverage flag. These are default fuzzer options -- and will allow customizing sanitizer-coverage in variations of fuzzer defaults. (i.e. w/o pc-table enabled).

In upstream clang the option -fno-sanitize-coverage exists to disable various coverage options. Swift currently does not have a convention of using excluding args for sanitizer flags. This aims to limit the need for a new flag by inclusively setting desired coverage up to current fuzzer defaults.

rdar://127881891
@thetruestblue thetruestblue force-pushed the add-sanitizer-coverage-options branch from 81fa2bd to 68923d4 Compare May 13, 2024 14:51
@thetruestblue
Copy link
Contributor Author

Using:
swiftlang/swift-driver#1604
@swift-ci smoke test

@thetruestblue thetruestblue marked this pull request as ready for review May 13, 2024 15:07
@thetruestblue thetruestblue requested a review from artemcm as a code owner May 13, 2024 15:07
@thetruestblue thetruestblue changed the title Adds valid arguments to -sanitize-coverage: pc-table and inline-8bit-counter. Add valid arguments to -sanitize-coverage: pc-table and inline-8bit-counter. May 13, 2024
@thetruestblue
Copy link
Contributor Author

Using:
swiftlang/swift-driver#1604
@swift-ci smoke test

@@ -5,7 +5,7 @@
// REQUIRES: asan_runtime

// Try some options
// RUN: %swiftc_driver -driver-print-jobs -sanitize-coverage=edge,indirect-calls,trace-bb,trace-cmp,8bit-counters -sanitize=address %s | %FileCheck -check-prefix=SANCOV_EDGE_WITH_OPTIONS %s
// RUN: %swiftc_driver -driver-print-jobs -sanitize-coverage=edge,indirect-calls,trace-bb,trace-cmp,8bit-counters,inlnie-8bit-counters,pc-table -sanitize=address %s | %FileCheck -check-prefix=SANCOV_EDGE_WITH_OPTIONS %s
Copy link
Contributor

Choose a reason for hiding this comment

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

inline*

Copy link
Contributor Author

@thetruestblue thetruestblue May 13, 2024

Choose a reason for hiding this comment

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

ty!
Already fixed and re-running test

@usama54321
Copy link
Contributor

Reading from the docs, it seems like pc-table requires one other flag too. If clang throws an error/shows a warning if one of the other flags is missing, I would recommend replicating that here as well. Otherwise, LGTM
https://clang.llvm.org/docs/SanitizerCoverage.html

@thetruestblue
Copy link
Contributor Author

I think you're referring to : -fsanitize-coverage=inline-8bit-counters or no? which I also added in this PR

@thetruestblue thetruestblue requested a review from yln May 13, 2024 20:10
Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@thetruestblue thetruestblue merged commit 35cda47 into swiftlang:main May 14, 2024
3 checks passed
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