[committed] libstdc++: Replace operator>>(istream&, char*) [LWG 2499]

[Jonathan Wakely]

On 06/08/20 15:01 +0100, Jonathan Wakely wrote:
>On 06/08/20 15:26 +0200, Jakub Jelinek via Libstdc++ wrote:
>>On Thu, Aug 06, 2020 at 02:14:48PM +0100, Jonathan Wakely wrote:
>>>  template<typename _CharT, typename _Traits>
>>>    __attribute__((__nonnull__(2), __access__(__write_only__, 2)))
>>>    inline basic_istream<_CharT, _Traits>&
>>>    operator>>(basic_istream<_CharT, _Traits>& __in, _CharT* __s)
>>>    {
>>>      size_t __n = __builtin_object_size(__s, 0);
>>>      if (__builtin_expect(__n < sizeof(_CharT), false))
>>>	{
>>>	  // not even space for null terminator
>>>	  __glibcxx_assert(__n >= sizeof(_CharT));
>>>	  __in.width(0);
>>>	  __in.setstate(ios_base::failbit);
>>>	}
>>>      else
>>>	{
>>>	  if (__n == (size_t)-1)
>>>	    __n = __gnu_cxx::__numeric_traits<streamsize>::__max;
>>>	  std::__istream_extract(__in, __s, __n / sizeof(_CharT));
>>>	}
>>>      return __in;
>>>    }
>>>
>>>This will give a -Wstringop-overflow warning at -O0 and then overflow
>>>the buffer, with undefined behaviour. And it will give no warning but
>>>avoid the overflow when optimising. This isn't my preferred outcome,
>>>I'd prefer to always get a warning, *and* be able to avoid the
>>>overflow when optimising and the size is known.
>>
>>A way to get warning even at -O2 would be to call some external function
>>in the if (__bos0 < sizeof(_CharT)) block, which wouldn't be optimized away
>>and would have __attribute__((warning ("..."))) on it.
>>See e.g. how glibc uses __warndecl e.g. in
>>/usr/include/bits/string_fortified.h.
>>One can use alias attribute to have different warnings for the same external
>>call (which could do e.g. what part of __glibcxx_assert does, call vprintf
>>+ abort.
>
>Every time I've tried that I've found the requirement for an external
>function to be frustrating. It means adding a new symbol to the
>library, because it doesn't work for inline functions or function
>templates, even with __attribute__((noinline)).
>
>And we don't necessarily want it to abort, because that depends on a
>macro defined by users, which isn't visible inside the library.
>
>It shouldn't be this hard.

The function with __attribute__(__warning__(""))) only warns when
-Wsystem-headers is on, which makes it useless. And when it's on, it
warns twice for a single call:

In file included from /home/jwakely/gcc/11/include/c++/11.0.0/sstream:38,
                  from of.cc:1:
In function 'std::basic_istream<_CharT, _Traits>& std::operator>>(std::basic_istream<_CharT, _Traits>&, _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]',
     inlined from 'std::basic_istream<_CharT, _Traits>& std::operator>>(std::basic_istream<_CharT, _Traits>&, _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]' at /home/jwakely/gcc/11/include/c++/11.0.0/istream:808:5,
     inlined from 'void test01(std::istream&)' at of.cc:7:16:
/home/jwakely/gcc/11/include/c++/11.0.0/istream:814:26: warning: call to 'std::__diag_overflow' declared with attribute warning: buffer overflow detected [-Wattribute-warning]
   814 |           __diag_overflow();
       |           ~~~~~~~~~~~~~~~^~
In function 'std::basic_istream<_CharT, _Traits>& std::operator>>(std::basic_istream<_CharT, _Traits>&, _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]',
     inlined from 'std::basic_istream<_CharT, _Traits>& std::operator>>(std::basic_istream<_CharT, _Traits>&, _CharT*) [with _CharT = char; _Traits = std::char_traits<char>]' at /home/jwakely/gcc/11/include/c++/11.0.0/istream:808:5,
     inlined from 'void test01(std::istream&)' at of.cc:7:16,
     inlined from 'int main()' at of.cc:13:9:
/home/jwakely/gcc/11/include/c++/11.0.0/istream:814:26: warning: call to 'std::__diag_overflow' declared with attribute warning: buffer overflow detected [-Wattribute-warning]
   814 |           __diag_overflow();
       |           ~~~~~~~~~~~~~~~^~


Adding attributes to __istream_extract is useless, because that's only
called by the library, so again, needs -Wsystem-headers to do
anything.

Adding attributes to operator>> works well, but only at -O0 because
otherwise it gets inlined and the attributes are ignored. The
functions that get called by the inlined function don't warn because
they're in system headers.

This is unusable, and a waste of a day.