This revision was automatically updated to reflect the committed changes.
Closed by commit rL274132: [libcxx] Fix a bug in strstreambuf::overflow.
(authored by ahatanak).
Changed prior to commit:
http://reviews.llvm.org/D20334?vs=58740&id=62225#toc
Repository:
rL LLVM
http://reviews.llvm.or
mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.
This looks fine to me.
http://reviews.llvm.org/D20334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi
I agree with Ben that this looks good.
Marshall and Eric, do you want Akira to hold off, or are you happy deferring to
Ben and me?
> On 2016-May-26, at 18:31, Akira Hatanaka via cfe-commits
> wrote:
>
> ahatanak updated this revision to Diff 58740.
> ahatanak added a comment.
>
> Remove unus
dexonsmith added a comment.
I agree with Ben that this looks good.
Marshall and Eric, do you want Akira to hold off, or are you happy deferring to
Ben and me?
http://reviews.llvm.org/D20334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h
(Ignore me, I was looking at an old version.)
> On 2016-Jun-28, at 17:56, Duncan P. N. Exon Smith via cfe-commits
> wrote:
>
> The fix looks fairly obvious, but you haven't added a testcase. Would you
> please do so?
>
>> On 2016-May-17, at 12:20, Akira Hatanaka via cfe-commits
>> wrote:
>
The fix looks fairly obvious, but you haven't added a testcase. Would you
please do so?
> On 2016-May-17, at 12:20, Akira Hatanaka via cfe-commits
> wrote:
>
> ahatanak created this revision.
> ahatanak added reviewers: mclow.lists, EricWF, howard.hinnant.
> ahatanak added a subscriber: cfe-c
ahatanak added a comment.
Thanks!
Marshall, I've filed a PR for this bug:
https://llvm.org/bugs/show_bug.cgi?id=27915
http://reviews.llvm.org/D20334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listi
bcraig added a comment.
LGTM. You may want to wait on Marshall's approval though. Thanks for being
patient with me, and thanks for the patch!
http://reviews.llvm.org/D20334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm
mclow.lists added a comment.
Let's file a bug on this too - http://llvm.org/bugs (make it easier to find in
the future)
http://reviews.llvm.org/D20334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/li
ahatanak marked an inline comment as done.
ahatanak added a comment.
I spent some time debugging the code and here is what I found.
The initial buffer size is 0 when strstreambuf is constructed and all six
pointers are null initially. When the first character is pushed,
strstreambuf::overflow a
ahatanak updated this revision to Diff 58740.
ahatanak added a comment.
Remove unused variable and add test case.
http://reviews.llvm.org/D20334
Files:
src/strstream.cpp
test/std/depr/depr.str.strstreams/depr.strstreambuf/depr.strstreambuf.members/overflow.pass.cpp
Index:
test/std/depr/d
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>
ahatanak added a comment.
In http://reviews.llvm.org/D20334#439248, @bcraig wrote:
> ASAN is complaining about an excessively large read. If the problem was in
> overflow, I would expect ASAN to complain about an out-of-bounds write
> instead.
According to the example shown in the link below
bcraig added a comment.
In http://reviews.llvm.org/D20334#438052, @ahatanak wrote:
> My understanding is that typically std::ends is explicitly appended to
> null-terminate the stream buffer. The test case in the example does that.
>
> http://en.cppreference.com/w/cpp/io/ostrstream/str
Doh! Y
ahatanak added a comment.
My understanding is that typically std::ends is explicitly appended to
null-terminate the stream buffer. The test case in the example does that.
http://en.cppreference.com/w/cpp/io/ostrstream/str
http://reviews.llvm.org/D20334
__
bcraig added a subscriber: bcraig.
bcraig added a comment.
I don't believe this is a libcxx bug, but it is a bug in the test code.
oss.str(); isn't required to return a null terminated string. std::cout <<
(char *) requires a null terminated string though.
http://reviews.llvm.org/D20334
_
ping.
> On May 17, 2016, at 12:20 PM, Akira Hatanaka via cfe-commits
> wrote:
>
> ahatanak created this revision.
> ahatanak added reviewers: mclow.lists, EricWF, howard.hinnant.
> ahatanak added a subscriber: cfe-commits.
>
> The end pointer should point to one past the end of the newly alloc
ahatanak created this revision.
ahatanak added reviewers: mclow.lists, EricWF, howard.hinnant.
ahatanak added a subscriber: cfe-commits.
The end pointer should point to one past the end of the newly allocated buffer.
Without this fix, asan reports an error when the following code is compiled and
18 matches
Mail list logo
