1

I have the following (simplified) code:


class py_string {
    PyObject* v;

public:
    py_string(const std::string& str) {
        // See https://docs.python.org/3/c-api/unicode.html#c.PyUnicode_FromStringAndSize
        // The string data is copied into the returned object
        v = PyUnicode_FromStringAndSize(str.data(), str.size());
    }
    ~py_string() {
        Py_XDECREF(v);
    }
    py_string(const py_string& other) {
        v = other.v;
        Py_XINCREF(v);
    }
    py_string(py_string&& other) {
        v = other.v;
        other.v = nullptr;
    }
};


static py_string generate_stylesheet() {  
    std::ostringstream html;              // html_styles.cc:80
    html << "<style>"; 
    html << ...;                          // html_styles.cc:82
    html << "</style>";
    return py_string(html.str());         // html_styles.cc:133
}

void emit() {
    auto style = generate_stylesheet();   // html_styles.cc:138
    ...
}

When running under Valgrind, the following error is reported:

==5882== Invalid read of size 4
==5882==    at 0x544028: ??? (in /usr/bin/python3.6)
==5882==    by 0x6BBBFC2: emit() (html_styles.cc:138)
==5882==    by 0x6D4AB0F: ...
==5882==  Address 0x669b020 is 1,904 bytes inside a block of size 2,049 free'd
==5882==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5882==    by 0x6BBBB82: deallocate (new_allocator.h:125)
==5882==    by 0x6BBBB82: deallocate (alloc_traits.h:462)
==5882==    by 0x6BBBB82: _M_destroy (basic_string.h:226)
==5882==    by 0x6BBBB82: _M_dispose (basic_string.h:221)
==5882==    by 0x6BBBB82: ~basic_string (basic_string.h:647)
==5882==    by 0x6BBBB82: ~basic_stringbuf (sstream:65)
==5882==    by 0x6BBBB82: ~basic_ostringstream (sstream:590)
==5882==    by 0x6BBBB82: generate_stylesheet() (html_styles.cc:80)
==5882==    by 0x6BBBF4F: emit() (html_styles.cc:138)
==5882==    by 0x6D4AB0F: ...
==5882==  Block was alloc'd at
==5882==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5882==    by 0x715207C: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::reserve(unsigned long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==5882==    by 0x7145CA7: std::__cxx11::basic_stringbuf<char, std::char_traits<char>, std::allocator<char> >::overflow(int) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==5882==    by 0x71504AA: std::basic_streambuf<char, std::char_traits<char> >::xsputn(char const*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==5882==    by 0x7140B83: std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.25)
==5882==    by 0x6BBB86A: operator<< <std::char_traits<char> > (ostream:561)
==5882==    by 0x6BBB86A: generate_stylesheet() (html_styles.cc:82)
==5882==    by 0x6BBBF4F: emit() (html_styles.cc:138)

So, it appears Valgrind is telling me the following story: in line 138 (function emit()) there is an attempt to read data from a string that no longer exists, but previously belonged to object html inside function generate_stylesheet() and then was freed when the function exited.

It was always my understanding that ostringstream::str() creates and returns a copy of a string that contains the stream's data. But that object, even though it is a temporary, must survive until the end of full expression, and before that happens the string will have been copied into a PyObject*.

So, what's happening here?

Pasha
  • 6,298
  • 2
  • 22
  • 34
  • @JerryJeremiah Already did, as comments – Pasha Jul 14 '20 at 01:55
  • Can you show the "obvious" implementation of your copy constructor, move constructor, destructor, etc? Valgrind's messages indicate that you do not copy the content of `v` in your copy constructor. – MSpiller Jul 14 '20 at 07:11
  • @M.Spiller I've updated the question with the 2 constructors. But really, `v` is just a plain C pointer, there is nothing special about copying it other than the incref/decref semantics. – Pasha Jul 14 '20 at 07:34
  • 2
    Maybe you can comment out the lines between `` to confirm that the issue is not among those. – H Krishnan Jul 14 '20 at 09:27

0 Answers0