Fix heap-use-after-free for _HAS_EXCEPTIONS=0 by wolframw · Pull Request #5406 · microsoft/STL (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@wolframw

@wolframw

@microsoft-github-policy-service agree

@frederick-vs-ja

@StephanTLavavej

We do have some precedent of outright =deleteing Standard APIs when compiler options make them literally unimplementable (e.g. dynamic_pointer_cast is deleted under /GR-).

In this case, because the option is ancient and the affected constructors (e.g. system_error) are old, I think that's a bit risky. Yes, we're dropping the string information on the floor, but we're preserving the type information in the string literal being stored. That seems like a "best effort" thing to do, and is strictly less problematic for users than the status quo.

AlexGuteniev

@wolframw

StephanTLavavej

StephanTLavavej

@StephanTLavavej

Thanks for fixing this surprisingly long-standing correctness bug! 😻 I pushed some test changes, and double-checked that the product code changes are complete.

Note for the future: we recommend against force-pushing after code review has begun, because GitHub makes it harder to see what's changed.

We merge PRs simultaneously to GitHub and our MSVC-internal repo in a semi-manual process, batched up to save time. Your PR will be part of the next batch, possibly this week but more likely next week depending on how busy I am. I'll post comments here as I prepare your PR for merging!

@wolframw

@StephanTLavavej

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

@StephanTLavavej

Thanks for fixing this bug and congratulations on your first microsoft/STL commit! 💚 😻 🎉

Labels

bug

Something isn't working