Access Violation when sending a 0 int literal to a const string parameter

Question

On VS2015 and VS2017, this compiles with no warning, and generates an access violation that cannot be caught and crashes the application. Obviously the int 0 is silently converted to a null pointer which is then assumed to point to a string hence crashing.

#include <string>
#include <iostream>
void crash(const std::string& s) {}
int main()
{
    try
    {
        crash(0);
    }
    catch (const std::exception& ex)
    {
        // never gets here!
        std::cout << "got" << ex.what() << std::endl;
    }
}

How can I trap and recover from such an exception? If I drop the const from the function parameter it does not compile - so that's maybe one way to guard from misuse by users, but I would lose the protection provided by the const, or would I? What's the best practice to write prototypes that avoid this problem?

_How can I trap and recover from such an exception?_ You can't. Don't do it. — , Jan 11 '18 at 15:17
What is your actual problem? Do you want to make just this one specific case fail to compile? Do you want to make this one specific case throw an exception? Or what? — David Schwartz, Jan 11 '18 at 22:48
My immediate problem was solved by Massimiliano below. I do think we need to get rid of this C relic, of having 0 being NULL and silently converted to nullptr. And also we need to never try to convert a nullprt to a std::string& or any other reference for that matter. A pointer can be null a reference should never be. These things need to be addressed by the standard committees. — Yaniv, Jan 14 '18 at 07:45
A slight nitpick - a literal `0` isn't *converted* to a null pointer - it *is* a way to write a null pointer in your source. — Toby Speight, Feb 09 '18 at 14:50
@Yaniv: Nothing is converting to a reference here: a temporary `std::string` is being **constructed** from an argument of an appropriate type (after what is here an unfortunate implicit conversion), and a reference is being bound to that in unsurprising manner. — Davis Herring, Aug 11 '21 at 19:07

Massimiliano Janes · Accepted Answer · 2018-01-11T15:33:11.940

25

For this specific case, you can get a compile time error by using C++11 std::nullptr_t, just add the following deleted overload:

void crash(std::nullptr_t) = delete;

Of course, this won't protect you against passing null (or non-null-terminated ) char* pointers ... you are violating an std::string constructor precondition, resulting in undefined behavior; this is by definition unrecoverable.

Alternatively, if you really need to catch these errors at runtime in a possibly recoverable way, you could write a const char* overload that throws if given a null pointer or invokes the std::string const& version otherwise.

If your real function takes more than a few string arguments, and overloading all possible combinations seems not feasible, you could resort writing a function template, performing all the checks over the deduced types ex-post.

edited Jan 11 '18 at 15:33

answered Jan 11 '18 at 15:17

Massimiliano Janes

5,524
1
10
22

1

My real code is really a family of methods with various overloads. On one overload there are bunch of string parameters and on the other there is an int in between similarly meaning parameters. So the library user (me 2 years later) naively changed from one method to the other and neglected to get rid of the 0 causing the application to crash. I Think both techniques you suggested are good and useful in this case and good to know in general. Thanks. – Yaniv Jan 11 '18 at 15:38
Since `0` is implicitly convertible to `char*` (which `std::string` has a constructor for), and `0` and `nullptr` are different things, wouldn't deleting the `std::nullptr_t` overload only prevent `crash(nullptr)` from compiling but not prevent `crash(0)`? – Remy Lebeau Jan 11 '18 at 22:14
@RemyLebeau [conv.ptr#1] reads *"A null pointer constant is an integer literal (2.14.2) with value zero or a prvalue of type std::nullptr_t"*; then, the nullptr_t overload is preferred over string(char*) because standard-conversions over user-defined conversions are – Massimiliano Janes Jan 11 '18 at 22:57

score 3 · Answer 2 · edited Feb 12 '18 at 09:23

namespace safer {
  template<class CharT,
    class Traits = ::std::char_traits<CharT>,
    class Allocator = ::std::allocator<CharT>,
    class Base = ::std::basic_string<CharT, Traits, Allocator>
  >
  struct basic_string:
    Base
  {
    using Base::Base;
    basic_string( CharT const* ptr ):
      Base( ptr?Base(ptr):Base() )
    {}
  };
  using string = basic_string<char>;
  using wstring = basic_string<wchar_t>;
}

This safer::string is basically identical to std::string but doesn't crash when constructed from a null pointer. Instead, it treats it as an empty string.

Simply sweep all mention of std::string from your code base and replace with safer::string, and similar for std::wstring and std::basic_string.

void crash(const safer::string& s) {}

You could choose to throw rather than silently consume the value.

We can detect 0 at compile time as well:

namespace safer {
  template<class CharT,
    class Traits = ::std::char_traits<CharT>,
    class Allocator = ::std::allocator<CharT>,
    class Base = ::std::basic_string<CharT, Traits, Allocator>
  >
  struct basic_string:
    Base
  {
    using Base::Base;
    basic_string( CharT const* ptr ):
      Base( ptr?Base(ptr):Base() )
    {}
    template<class T,
      // SFINAE only accepts prvalues of type int:
      std::enable_if_t<std::is_same<T, int>::value, bool> = true
    >
    basic_string(T&&)=delete; // block 0
  };
  using string = basic_string<char>;
  using wstring = basic_string<wchar_t>;
}

and now passing 0 gets a compile-time error, passing nullptr or a null char const* gets you an empty string.

live example.

So, there are people who get nervous about the fact I told you to inherit from a non-polymorphic type in std. There are reasons not to inherit from a non-polymorphic type, but none of them apply here.

In general, however, be careful when inheriting from types not designed for polymorphism like std::basic_string<CharT>. In this particular case, storing a safer::basic_string<T> in a std::basic_string<T>* and then calling delete on it is undefined behavior (or in a std::unique_ptr<std::basic_string<T>> which calls delete). But dynamically allocating basic_strings is usually a mistake in the first place, so that is unlikely to occur.

In addition, this inheritance has to obey the LSP without changing the behavior of any method of the base class. In this case we are adapting construction, and construction is never polymorphic. If we had any write operations that were not construction where we wanted to maintain an invariant in the descended class we would be in trouble.

Just be careful deriving from `std::basic_string`: [Why should one not derive from c++ std string class?](https://stackoverflow.com/questions/6006860/) — Remy Lebeau, Jan 11 '18 at 22:24
@remy None of those points cover this case as far as I can tell. Can you be specific? — Yakk - Adam Nevraumont, Jan 11 '18 at 23:48
I'm not @Remy, but I'll guess that the comment was general advice to people reading this that one needs to understand the implications quite well to know when subclassing standard types is a safe thing to do, rather than assuming that because this answer derives from `std::string` that it's a good practice in general. Perhaps it warrants a short sentence of warning? — Toby Speight, Feb 09 '18 at 14:59
@TobySpeight Ok, I added scary advice not to do this recklessly. — Yakk - Adam Nevraumont, Feb 09 '18 at 16:06
I like your "scary advice" - I just tidied up some typos for you. — Toby Speight, Feb 12 '18 at 09:23

Access Violation when sending a 0 int literal to a const string parameter

2 Answers2

Linked