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

update disable ubsan macro #1992

Conversation

urvishdesai
Copy link
Contributor

no_sanitize attribute now supports undefined-behavior sanitizers including no_sanitize("unsigned-integer-overflow"), no_sanitize("undefined") etc.

In the current code, running __attribute__((no_sanitize("unsigned-integer-overflow"))) for ASan, MSan, or TSan is enabled leads to the warnings for attribute ignored. The logic here is hence incorrect. This kind of an attribute should be used only when UBSan is enabled.

@facebook-github-bot
Copy link
Contributor

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

@urvishdesai
Copy link
Contributor Author

Hi @Orvid, could you merge this into the main branch if everything looks good? Thanks!

@Orvid
Copy link
Contributor

Orvid commented Jun 30, 2023

Yep, working on it. This PR got lost in my pile of things to do. It should hopefully be merged either later today or on monday.

@facebook-github-bot
Copy link
Contributor

@Orvid merged this pull request in 2bb1fe8.

facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Jul 28, 2023
Summary:
no_sanitize attribute now supports undefined-behavior sanitizers including `no_sanitize("unsigned-integer-overflow")`, `no_sanitize("undefined")` etc.

In the current code, running `__attribute__((no_sanitize("unsigned-integer-overflow")))` for ASan, MSan, or TSan is enabled leads to the warnings for attribute ignored. The logic here is hence incorrect. This kind of an attribute should be used only when UBSan is enabled.

X-link: facebook/folly#1992

Reviewed By: yfeldblum, Gownta

Differential Revision: D45674502

fbshipit-source-id: 97be9e9de82a546e553ed8866fb106e2f096709d
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.

None yet

3 participants