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

ASan: Remove noexcept from __sanitizer_annotate_contiguous_container #4150

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

StephanTLavavej
Copy link
Member

#4106 marked __sanitizer_annotate_contiguous_container as noexcept because it's extern "C". However, it has to match ASan's primary declaration, so we can't mark it.

This PR reverts this declaration to its original form, and adds a comment for future maintainers.

@StephanTLavavej StephanTLavavej added bug Something isn't working ASan Address Sanitizer labels Nov 6, 2023
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner November 6, 2023 18:24
@AlexGuteniev
Copy link
Contributor

If it throws, should be noexcept(false) to opt out of implicit noexcept, no?

@CaseyCarter
Copy link
Member

CaseyCarter commented Nov 6, 2023

If it throws, should be noexcept(false) to opt out of implicit noexcept, no?

I don't believe it does throw, I think it's simply unannotated. This is common for shared C/C++ headers: it's easy to stick a conditional extern "C" { ... } around everything to make a C header into a C++ header, but conditionally tacking noexcept onto function declarations takes more effort.

Ideally we would fix this upstream and then bring it back here.

@StephanTLavavej StephanTLavavej self-assigned this Nov 6, 2023
@StephanTLavavej
Copy link
Member Author

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 900eeac into microsoft:main Nov 7, 2023
37 checks passed
@StephanTLavavej StephanTLavavej deleted the except branch November 7, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASan Address Sanitizer bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants