3

I've seen several similar snippets of code that looked like this:

struct MyExcept : std::exception {
    explicit MyExcept(const char* m) noexcept : message{m} {}

    const char* what() const noexcept override {
        return message;
    }

    const char* message;
};

void foo() {
    std::string error;

    error += "Some";
    error += " Error";

    throw MyExcept{error.c_str()};
}

int main() {
    try {
        foo();
    } catch (const MyExcept& e) {
        // Is this okay?
        std::cout << e.message << std::endl;
    }
}

In the line following the comment Is this okay?, we read the c-style string that was allocated in the foo function using std::string. Since the string is destructed with stack unwinding, is this undefined behavior?


If it's indeed undefined behavior, what if we replace the main function with this one?

int main() {
    foo();
}

Since there is no catch, the compiler is not forced to unwind the stack, and yet output the result of what() in the console and abort the program. So is it still undefined behavior?

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141

3 Answers3

5

Yes, that's undefined behavior. You are working with a dangling pointer.

void foo() {
    std::string error;

    error += "Some";
    error += " Error";

    throw MyExcept{error.c_str()};
} // <<  error goes out of scope here and so does the pointer returned
  //     from c_str()

Since there is no catch, the compiler is not forced to unwind the stack, and yet output the result of what() in the console and abort the program. So is it still undefined behavior?

Since the default implementation will use std::terminate and in turn calling std::abort() this may be still undefined behavior because most of the standard handler implementations will try to dereference what().

You can install your own handlers though to avoid that.

user0042
  • 7,917
  • 3
  • 24
  • 39
2

Your first snippet has undefined behavior. [exception.ctor]/1:

As control passes from the point where an exception is thrown to a handler, destructors are invoked by a process, specified in this section, called stack unwinding.

Here, the destructor or error is called, causing the c_str() to become a dangling pointer. Later dereferencing it, when you use std::cout for instance, is undefined behavior.

Your second snippet is perfectly fine. There is no reason why it would be undefined behavior. You never actually call what, or do anything else that might result in undefined behavior. The only thing not defined by the Standard is if stack unwinding happens or not, [except.terminate]/2:

In the situation where no matching handler is found, it is implementation-defined whether or not the stack is unwound before std​::​terminate() is called.

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
  • Thanks Rakete1111, so it's up to GCC or any other implementation if there is undefined behavior in the second snippet? – Guillaume Racicot Jul 20 '17 at 16:38
  • 1
    _"There is no reason why it would be undefined behavior."_ Let's say that's implementation defined. It may or may be not undefined behavior depending on what the implementation does. – user0042 Jul 20 '17 at 16:39
  • @GuillaumeRacicot No, there is no undefined behavior. The only behavior that deviates from compiler to compiler is if stack unwinding happens or not, and that is implementation defined. You never actually use the pointer in question, so it's fine. – Rakete1111 Jul 20 '17 at 16:42
  • 1
    @user0042 Fine but where? The [default handler](http://eel.is/c++draft/terminate.handler#3) just calls `std::abort`, nothing more and nothing less. But ok, maybe I'm missing a quote, I'll try to find it. There is nothing saying that this is implementation defined. – Rakete1111 Jul 20 '17 at 16:43
  • @Rakete1111 Well, I have to admit I tell that from experience, don't have a reference quote either :-/ ... – user0042 Jul 20 '17 at 16:45
  • @user0042 [gcc's](https://gcc.gnu.org/onlinedocs/libstdc++/manual/termination.html) documention on the topic seems to imply that `what()` is indeed called if the exception derives from `std::exception`. Weird, I still can't find anything in the Standard about it. – Rakete1111 Jul 20 '17 at 16:47
  • @Rakete1111 IMO the only way to be sure, is to install your own terminate handlers. – user0042 Jul 20 '17 at 16:50
  • @user0042 How would you do that? [`std::terminate_handler`](http://en.cppreference.com/w/cpp/error/terminate_handler) doesn't take any parameters you could use to call `what()`. – Rakete1111 Jul 20 '17 at 16:51
2

As others stated, the code has undefined behavior, since the pointer assigned to message is left dangling.

std::runtime_error already provides a solution to this issue. Call its constructor that takes a std::string as input, and don't override what() at all:

struct MyExcept : std::runtime_error {
    explicit MyExcept(const std::string & m) noexcept : std::runtime_error(m) {}
};

void foo() {
    std::string error;

    error += "Some";
    error += " Error";

    throw MyExcept(error);
}

int main() {
    try {
        foo();
    }
    catch (const MyExcept& e) {
        std::cout << e.what() << std::endl;
    }
}

std::runtime_error has an internal std::string whose data what() returns by default, thus avoiding the dangling issue.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Indeed this is the proper way of setting up an exception's error message. Funnily enough the `std::exception::what()` method is `virtual`, but there seems to be no simple way of overriding it because it must return a `const char *`, leading to the dangling pointer issues in most cases. Maybe one day the signature will be changed to returning a `std::string`... – András Aszódi Aug 12 '21 at 16:33