Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Values in optional<map<string, string>> getting "corrupted" in very specific cases

Tags:

c++

Sorry for the poor title, but what I'm seeing is bizarre and hard to explain succinctly.

Basically, we have an optional<map<string, string>> in our code accessed through a getter/setter, and sometimes when we inspect the values we get very strange results. Here is simplified code which repros the issue:

#include <optional>
#include <map>
#include <iostream>

using namespace std;

optional<map<string, string>> optmap;

static void Set(optional<map<string, string>> m);
static optional<map<string, string>> Get();
static void PrintMap(map<string, string> m);

int main(int const argc, char const * const *argv)
{
        map<string, string> sample;
        sample.emplace("testtesttesttest1", "testtesttesttest1");
        sample.emplace("testtesttesttest2", "testtesttesttest2");
        sample.emplace("testtesttesttest3", "testtesttesttest3");

        cout << "sample:" << endl;
        PrintMap(sample);

        Set(sample);
        map<string, string> result = Get().value();

        cout << "result:" << endl;
        PrintMap(result);

        cout << "function call:" << endl;
        PrintMap(Get().value());

        cout << "inline iteration:" << endl;
        for (auto &item : Get().value())
        {
                cout << item.first << ", " << item.second << endl;
        }
}

static void Set(optional<map<string, string>> m)
{
        optmap = m;
}

static optional<map<string, string>> Get()
{
        return optmap;
}

static void PrintMap(map<string, string> m)
{
        for (auto &item : m)
        {
                cout << item.first << ", " << item.second << endl;
        }
}

I compiled using g++ -std=c++17 and got this output on my most recent run:

$ ./a.out 
sample:
testtesttesttest1, testtesttesttest1
testtesttesttest2, testtesttesttest2
testtesttesttest3, testtesttesttest3
result:
testtesttesttest1, testtesttesttest1
testtesttesttest2, testtesttesttest2
testtesttesttest3, testtesttesttest3
function call:
testtesttesttest1, testtesttesttest1
testtesttesttest2, testtesttesttest2
testtesttesttest3, testtesttesttest3
inline iteration:
@�M�OVtesttest1, ��M�OVtesttest1
��M�OVtesttest2, ��M�OVtesttest2
��M�OVtesttest3, testtest3

Note that the values only get "corrupted" in the last case where we iterate using for (auto &item : Get().value()). What's even stranger is that this only appears to happen for strings of a certain length. If the values are less than 16 characters long, we have no problem. If I change the map to contain the following:

sample.emplace("fifteencharokay", "15");
sample.emplace("sixteencharweird", "16");

I get this output:

$ ./a.out 
sample:
fifteencharokay, 15
sixteencharweird, 16
result:
fifteencharokay, 15
sixteencharweird, 16
function call:
fifteencharokay, 15
sixteencharweird, 16
inline iteration:
fifteencharokay, 15
harweird, 16

(Notice that "sixteencharweird" has been truncated to "harweird" in the last line)

What is happening here? Why is it that we're having issues in this one very specific case (long strings and iterating directly over the result of a function call)? Is there some sort of C++ rule I'm breaking here by iterating this way?

like image 776
wlyles Avatar asked Jan 24 '23 19:01

wlyles


1 Answers

In this loop:

for (auto &item : Get().value())

you are invoking undefined behavior, because the temporary returned by Get() will die at the end of the full expression, and the .value() that your range for loop will be iterating over refers to memory that no longer exists.

The strange behavior that you notice with strings less than 16 chars long, is possibly due to small-string-optimization. Since the string holds on to the internal buffer for short strings, you can still see the memory there. Of course, this is still UB, and you can't rely on it.

You can fix this issue by doing:

auto const &g = Get();
for (auto &item : g.value())

Here's a demo.

In fact, c++20 adds the range-for with initializer construct for exactly this purpose:

for (auto const &g = Get(); auto &item : g.value())
like image 101
cigien Avatar answered May 18 '23 19:05

cigien