[PATCH] D20334: [libcxx] Fix a bug in strstreambuf::overflow

[Ben Craig via cfe-commits]

bcraig added a comment.

So I definitely see a problem, and I think your fix is a reasonable solution to that problem.  I'm still unclear on how that caused the ASAN error though.  Here's my understanding of the current problem...

The initial buffer for strstreambuf is often 4096  ( _strstream> line 184, __default_alsize = 4096;).
So we have a 4096 character string, and push that into strstreambuf.  There shouldn't be any calls to overflow because of that.
Now, we push in an ends().  This causes a call to overflow().  overflow() allocates a new buffer of twice the size, then saves off the relevant put and get range sizes for the old buffer.  The old capacity ranges (einp and eout) should add up to 4096.  We then set the new range sizes using the old range sizes.  THIS SEEMS WRONG.  We have an 8K buffer, but we are only setting it with 4K sizes.  We then write ends() into the no-mans-land past the end of epptr() (well, exactly at epptr), and then advance pptr() past epptr().

I can see this causing a lot of trouble, as some code compares pptr() == epptr(), and we've broken that.

However, I would expect ASAN to require an extra call to overflow() before things explode.  strlen should still find that end() that we placed.  Maybe there's an extra overflow() that I didn't see though.

I would like to see a new test added to exercise this use case.  The test should probably live somewhere under test/std/depr/depr.str.strstreams/depr.strstreambuf.  Your example code is going to be pretty close to what you would want to put there (maybe a string.resize instead of the stringOfLength() function you have).  I believe you can run the tests using address sanitizer by setting LLVM_USE_SANITIZER="Address" to your cmake line.  "make check-libcxx" will then compile the tests (and libcxx?) with ASAN.


================
Comment at: src/strstream.cpp:174
@@ -173,3 +173,3 @@
         ptrdiff_t nout = pptr()  - pbase();
         ptrdiff_t eout = epptr() - pbase();
         if (__strmode_ & __allocated)
----------------
This is now unused.


http://reviews.llvm.org/D20334