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.

Sign up for GitHub

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

Jump to bottom

Test ASAN SSO string copies; improve basic_string::_Construct preconditions #3039

Merged
merged 2 commits into from
Aug 22, 2022
Merged

Test ASAN SSO string copies; improve basic_string::_Construct preconditions #3039

merged 2 commits into from
Aug 22, 2022

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Aug 18, 2022 •
edited
Loading

To avoid regressing DevCom-10116361 / VSO-1590908

Reviewer note: I did temporarily enable ASAN annotation and verify that all tests (including the new regression test) pass.

@CaseyCarter CaseyCarter added the enhancement Something can be improved label Aug 18, 2022
@CaseyCarter
Test ASAN SSO string copies
7bef7d3 
To avoid regressing DevCom-10116361 / VSO-1590908

Also: Document `_Construct` precondition that elements must be value-initialized, use in all branches.
@CaseyCarter CaseyCarter force-pushed the ASAN-SSO-fix branch 2 times, most recently from 6c731bc to 7bef7d3 Compare August 18, 2022 20:27
@CaseyCarter CaseyCarter marked this pull request as ready for review August 18, 2022 20:27
@CaseyCarter CaseyCarter requested a review from a team as a code owner August 18, 2022 20:27
@StephanTLavavej
Copy link
Member

Looks good, I pushed #include <new>. (I'm going to be so sad when import std; makes this game unplayable 😹)

@CaseyCarter
Copy link
Member Author

Looks good, I pushed #include <new>. (I'm going to be so sad when import std; makes this game unplayable 😹)

GRAWR. I can't believe I double-checked <cstring> and missed <new>. I suppose I deserve this for copying the code but not the includes from my separate repro.

@StephanTLavavej StephanTLavavej self-assigned this Aug 20, 2022
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit ad18417 into microsoft:main Aug 22, 2022
@StephanTLavavej
Copy link
Member

Thanks for enhancing this test coverage and improving bug resistance here! ✅ 🐞 😻

@CaseyCarter CaseyCarter deleted the ASAN-SSO-fix branch August 22, 2022 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
enhancement Something can be improved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CaseyCarter @StephanTLavavej