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

C++23 prohibits constructing std::string from nullptr #8265

Closed
StephanTLavavej opened this issue Jul 7, 2021 · 4 comments
Closed

C++23 prohibits constructing std::string from nullptr #8265

StephanTLavavej opened this issue Jul 7, 2021 · 4 comments

Comments

@StephanTLavavej
Copy link

I work on MSVC's C++ Standard Library implementation, and we regularly build MAME (along with many other open-source projects) to identify compiler/library bugs that would break your project, so we can fix them before shipping. This also allows us to provide advance notice of source-breaking changes - which is the case here.

The paper P2166R1 "Prohibiting basic_string And basic_string_view Construction From nullptr" has been accepted for the upcoming C++23 Standard, and we recently implemented it by merging microsoft/STL#1995 from our contributor @sam20908. Our open-source test pass then discovered the following code in MAME that is now disallowed by C++23:

return open_choices(file, procs, nullptr, formats, flags, outcassette);
}
cassette_image::error cassette_image::open_choices(void *file, const io_procs *procs, const std::string &extension,
const Format *const *formats, int flags, ptr &outcassette)

Here, cassette_image::open() is passing nullptr for the const std::string &extension parameter of cassette_image::open_choices(). This has always been undefined behavior in C++98 through C++20 (basic_string's constructor has a precondition that the pointer is non-null); C++23 has added a deleted overload of basic_string's constructor to detect when the argument is known to be nullptr at compile-time. (Runtime values of const char * that happen to be null are still undefined behavior.)

There are several possible ways to fix this code; you could pass an empty string with "" or std::string{} or even {} (depending on whether open_choices() is overloaded).

There may be additional occurrences of this throughout MAME's codebase - this is simply the first one we encountered. Hope this helps!

@cuavas
Copy link
Member

cuavas commented Jul 8, 2021

Yeah, we shouldn’t be constructing std::string from nullptr. In this case, I think it should be passing std::string() rather than nullptr in the call to open_choices. Could someone (e.g. @Tafoid or @Robbbert) test this change?

@ajrhacker
Copy link
Contributor

Constructing strings from nullptr already causes segmentation faults or runtime assertion errors on many builds. Bugs like this (e.g. 3a04996) have persisted mostly due to obscurity.

Robbbert added a commit that referenced this issue Jul 21, 2021
Note that std::string() was tried but somehow caused another bug (tape preset as play was instead stopped).
@Robbbert
Copy link
Contributor

This bug was created almost 5 years ago and survived a number of refactors since.

I tested std::string(), however a different bug appeared which I can't explain, that being if the tape was preset to PLAY, it instead was STOPPED. So I used {} instead.

See 645aef0

@Robbbert
Copy link
Contributor

Robbbert commented Jul 22, 2021

No further comments, so closing.

Anyone who has a continuing interest in similar bugs may reopen this issue if they so wish.

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

No branches or pull requests

4 participants