6

According to an article (here and there) this code is an erroneous use-after free example:

#include <iostream>
#include <string>
#include <string_view>

int main() {
  std::string s = "Hellooooooooooooooo ";
  std::string_view sv = s + "World\n";
  std::cout << sv;
}

In the article it is stated that the string s will be freed when the string_view is used! This goes against my debugging experience. But I'm asking you to confirm/verify/check this.

In my experience, stack/scope variables are freed (calling the destructor would be a far more correct wording) at the exit of the scope. This means that in this case, this would happen AFTER the std::cout << sv;

However I have never used string_view, so I don't know about any internal mechanics of this object.

If indeed it is a dangerous behaviour, could you explain it? Otherwise, I'd be glad to read the confirmation that scope variables destructors are called only at the exit of the current scope, naturally, or when an exception is thrown, interrupting the thread in the current scope.


EDIT: After the first two answers, it is really a use-after-free usage.

Subsidiary question: Do you think we could add a move constructor with the delete keyword in the definition of string_view so as to forbid that?

Stephane Rolland
  • 38,876
  • 35
  • 121
  • 169
  • 2
    The things is that the scope of unnamed temporary object created with `s + "World\n"` is limited to that one statement, not to the end of `main`. – user7860670 Apr 22 '19 at 07:13
  • 1
    Yes, `string::operator string_view` is a huge mistake. Why was it made applicable to rvalue references? Heads should be rolling. – n. m. could be an AI Apr 22 '19 at 07:20
  • @n.m. yes exactly, why does it accepts `rvalue` reference ? That's what I was wondering about after the interesting answers... So as to prevent this, do you think we could add a move constructor with the delete keyword in the definition of string_view ? – Stephane Rolland Apr 22 '19 at 07:23
  • 3
    @StephaneRolland I suspect the reason is that `std::string_view` is expected to be used as aa function parameter so that temporaries can be passed into a function (like with a const ref). Obviously the lifetine is fine for that scenario. – Galik Apr 22 '19 at 07:28
  • 1
    @Galik Yes this is a legit scenario, I retract my objection ;) But perhaps then string_view should *only* be used as a function parameter. – n. m. could be an AI Apr 22 '19 at 07:54

2 Answers2

7

The problem with this code...

std::string_view sv = s + "World\n";

... is that sv is not set to s but to a nameless temporary created by the expression s + "world\n". That temporary is destroyed immediately after the whole expression ends (at the semicolon).

So yes, this is a "use after free" type error.

If you want to extend the life of that temporary you have to assign it to a variable that will maintain it - like a new std::string object:

std::string sv = s + "World\n"; // copy the temporary to new storage in sv

A std::string_view is merely a "view" onto a string, it is not a string in itself. It is only valid as long as the string it "looks" at is valid.

There is another quirk here too. You can also bind the temporary to a const reference which extends the life of temporaries:

std::string const& sv = s + "World\n"; // just keep the temporary living

Why is initializing a std::string_view from a temporary allowed?

I can not speak for the standards committee, but my suspicion would be that std::string_view is expected to be used as a function parameter so that temporaries can be passed into a function (like with a const ref). Obviously the lifetime is fine for that scenario.

If we forbade initialization from temporaries then a major use of std::string_view would be negated. You would be forced to create a new std::string (or binding to a const ref) before calling a function making the process awkward.

Galik
  • 47,303
  • 4
  • 80
  • 117
  • 1
    `keep the temporary living` - that's one horrible thing to do – user7860670 Apr 22 '19 at 07:13
  • Okay. Had I known string_view was taking a pointer/reference as input, I might have sensed the danger. Thanks for the explanation: symptom + remedy. I agree that this is tricky / error prone. – Stephane Rolland Apr 22 '19 at 07:14
  • @Galik Do you think we could add a move constructor with the delete keyword in the definition of string_view so as to forbid that? – Stephane Rolland Apr 22 '19 at 07:28
  • 1
    @StephaneRolland I added some comments at the end. – Galik Apr 22 '19 at 07:34
  • @StephaneRolland No, the correct way is obvious, don't let there be an implicit conversion. This has been discussed 20 years ago on the conversion to `char*`, they are literally the same problem. – Passer By Apr 22 '19 at 07:35
  • @PasserBy Not so obvious because it had not sparkled in my mind until you said it. I like your proposal. – Stephane Rolland Apr 22 '19 at 07:39
3

The expression s + “World\n” results in a temporary object. The lifetime of this temporary std::string is just long enough to initialise sv. The memory that sv refers to is freed immediately after it is initialised (when the temporary object is destroyed).

Indiana Kernick
  • 5,041
  • 2
  • 20
  • 50