On 8/6/20 8:45 AM, Jonathan Wakely via Libstdc++ wrote:
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.
Sorry. I don't see this exercise as a complete waste of time
(but I understand why it feels like that to you).
What it highlights is the fact that the warning infrastructure
we have in place is far from optimal for C++ in general (with
its heavy reliance on ilining and templates) and the standard
library in particular (especially with -Wno-system-headers).
We should make an effort to do better.
Setting aside the effort to clean up the library so that it can
be used even with -Wsystem-headers, warnings about out of bounds
accesses should trigger even with -Wno-system-headers. If one
doesn't I'd tend to view it as a bug. I added code to have some
trigger despite it but I'm pretty sure there are more places where
the middle end needs to do the same gymnastics to enable it.
Martin
