Bug 97415 - Invalid pointer comparison in stringbuf::str() (reported by pointer-compare AddressSanitizer)
Summary: Invalid pointer comparison in stringbuf::str() (reported by pointer-compare A...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 10.2.0
: P3 normal
Target Milestone: 11.0
Assignee: Jonathan Wakely
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-14 10:08 UTC by Paweł Bylica
Modified: 2020-11-10 19:41 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-10-14 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paweł Bylica 2020-10-14 10:08:35 UTC
When my application is instrumented with -fsanitize=address,pointer-compare
and running under ASAN_OPTIONS=detect_invalid_pointer_pairs=2,
I get for following failure in basic_stringbuf::str()

==3879==ERROR: AddressSanitizer: invalid-pointer-pair: 0x7ffcdf273b66 0x000000000000
    #0 0x5597a6c6d786 in std::__cxx11::basic_stringbuf<char, std::char_traits<char>, std::allocator<char> >::str() const /usr/include/c++/10/sstream:184
    #1 0x5597a6c6d786 in std::__cxx11::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >::str() const /usr/include/c++/10/sstream:678
    #2 0x5597a6c6d786 in std::basic_ostream<char, std::char_traits<char> >& std::__detail::operator<< <char, std::char_traits<char>, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&>(std::basic_ostream<char, std::char_traits<char> >&, std::__detail::_Quoted_string<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, char> const&) /usr/include/c++/10/bits/quoted_string.h:130
    #3 0x5597a6c6d786 in std::basic_ostream<char, std::char_traits<char> >& std::filesystem::__cxx11::operator<< <char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, std::filesystem::__cxx11::path const&) /usr/include/c++/10/bits/fs_path.h:441
    #4 0x5597a6c6d786 in log_total /home/builder/project/test/spectests/spectests.cpp:675
    #5 0x5597a6c48939 in run_tests_from_dir /home/builder/project/test/spectests/spectests.cpp:708
    #6 0x5597a6c48939 in main /home/builder/project/test/spectests/spectests.cpp:750

Here is the implementation of basic_stringbuf::str() used for compilation:

      __string_type
      str() const
      {
	__string_type __ret(_M_string.get_allocator());
	if (this->pptr())
	  {
	    // The current egptr() may not be the actual string end.
	    if (this->pptr() > this->egptr())
	      __ret.assign(this->pbase(), this->pptr());
	    else
	      __ret.assign(this->pbase(), this->egptr());
	  }
	else
	  __ret = _M_string;
	return __ret;
      }

In the line `if (this->pptr() > this->egptr())`,
the `this->egptr()` may be nullptr and therefore AddressSanitizer complains about this comparison.

I don't have handy repro code for the issue, but I can try to build one if desired.

GCC version: cpp (Debian 10.2.0-15) 10.2.0
Comment 1 Jonathan Wakely 2020-10-14 13:27:16 UTC
This should reproduce it, but doesn't for some reason:

#include <bits/c++config.h>
#undef _GLIBCXX_EXTERN_TEMPLATE
#include <sstream>

int main()
{
  std::ostringstream s;
  s << ".";
  return s.str().length();
}
Comment 2 Jonathan Wakely 2020-10-14 13:33:00 UTC
Oh, it does if I spell the environment variable correctly.
Comment 3 CVS Commits 2020-10-14 17:56:26 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:78198b6021a9695054dab039340202170b88423c

commit r11-3889-g78198b6021a9695054dab039340202170b88423c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 14 18:55:14 2020 +0100

    libstdc++: Fix unspecified comparison to null pointer [PR 97415]
    
    The standard doesn't guarantee that null pointers compare less than
    non-null pointers. AddressSanitizer complains about the pptr()> egptr()
    comparison in basic_stringbuf::str() when egptr() is null.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/97415
            * include/std/sstream (basic_stringbuf::str()): Check for
            null egptr() before comparing to non-null pptr().
Comment 4 Jonathan Wakely 2020-10-15 11:32:29 UTC
Fixed on trunk so far. I'm undecided whether it needs to be backported. Although the comparison with null is formally unspecified, I think all the compilers we support behave as expected.
Comment 5 Martin Liška 2020-10-15 15:29:27 UTC
(In reply to Jonathan Wakely from comment #4)
> Fixed on trunk so far. I'm undecided whether it needs to be backported.
> Although the comparison with null is formally unspecified, I think all the
> compilers we support behave as expected.

I wouldn't backport it as pointer-compare is quite experimental feature of the AddressSanitizer.
Comment 6 Jonathan Wakely 2020-10-15 17:03:35 UTC
OK thanks, let's call it done then.
Comment 7 CVS Commits 2020-11-10 19:41:11 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:ced70ebaa372945ec8d73703d81e4a10d6d51c9b

commit r11-4887-gced70ebaa372945ec8d73703d81e4a10d6d51c9b
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue Nov 10 15:46:02 2020 +0000

    libstdc++: Fix more unspecified comparisons to null pointer [PR 97415]
    
    This adds some more null checks to avoid a relational comparison with a
    null pointer, similar to 78198b6021a9695054dab039340202170b88423c.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/97415
            * include/std/sstream (basic_stringbuf::_M_update_egptr)
            (basic_stringbuf::__xfer_bufptrs::__xfer_bufptrs): Check for
            null before comparing pointers.