[libc++] Implement P0487R1 - Fixing operator>>(basic_istream&, CharT*)

[lichray]

Summary

Avoid buffer overflow by replacing the pointer interface with an array reference interface in C++2a.
Tentatively ready on Batavia2018.

https://wg21.link/lwg2499
https://wg21.link/p0487

[lichray]

Rebased

[ldionne]

include/istream

601

Should this instead be if (__n == 0)? Can width() ever return a negative number? cppreference says this about streamsize:

Except in the constructors of std::strstreambuf, negative values of std::streamsize are never used.

I couldn't find a matching statement in the standard, however there's this footnote in 30.5.2:

streamsize is used in most places where ISO C would use size_t. Most of the uses of streamsize could use size_t, except for the strstreambuf constructors, which require negative values. It should probably be the signed type corresponding to size_t (which is what Posix.2 calls ssize_t).

This does not clearly say that negative values are never used, but it does suggest it. Maybe it's safer to still use __n <= 0.

[ldionne]

Please also update https://libcxx.llvm.org/cxx2a_status.html.

[lichray]

In D51268#1303217, @ldionne wrote:

Please also update https://libcxx.llvm.org/cxx2a_status.html.

Done.

include/istream

601

My interpretation of 27.5.3.2 is that you can retrieve the value you set through width(streamsize). And the wording here (unmodified) says "If width() is greater than zero, [...]," so the opposite should be width() <= 0.

[lichray]

Update cxx2a status.

[ldionne]

LGTM

include/istream

 template <class _CharT, class _Traits>
 basic_istream<_CharT, _Traits>&
 basic_istream<_CharT, _Traits>::operator>>(int& __n)
 {
     return _VSTD::__input_arithmetic_with_numeric_limits<int>(*this, __n);
 }
 
 template<class _CharT, class _Traits>
+_LIBCPP_INLINE_VISIBILITY
 basic_istream<_CharT, _Traits>&
-operator>>(basic_istream<_CharT, _Traits>& __is, _CharT* __s)
+__input_c_string(basic_istream<_CharT, _Traits>& __is, _CharT* __p, size_t __n)
 {
 #ifndef _LIBCPP_NO_EXCEPTIONS
     try
     {
 #endif  // _LIBCPP_NO_EXCEPTIONS
         typename basic_istream<_CharT, _Traits>::sentry __sen(__is);
         if (__sen)
         {
-            streamsize __n = __is.width();
-            if (__n <= 0)
-                __n = numeric_limits<streamsize>::max() / sizeof(_CharT) - 1;
-            streamsize __c = 0;
+            auto __s = __p;
             const ctype<_CharT>& __ct = use_facet<ctype<_CharT> >(__is.getloc());
             ios_base::iostate __err = ios_base::goodbit;
-            while (__c < __n-1)
+            while (__s != __p + (__n-1))
             {
                 typename _Traits::int_type __i = __is.rdbuf()->sgetc();
                 if (_Traits::eq_int_type(__i, _Traits::eof()))
                 {
                    __err |= ios_base::eofbit;
                    break;
                 }
                 _CharT __ch = _Traits::to_char_type(__i);
                 if (__ct.is(__ct.space, __ch))
                     break;
                 *__s++ = __ch;
-                ++__c;
                  __is.rdbuf()->sbumpc();
             }
             *__s = _CharT();
             __is.width(0);
-            if (__c == 0)
+            if (__s == __p)
                __err |= ios_base::failbit;
             __is.setstate(__err);
         }
 #ifndef _LIBCPP_NO_EXCEPTIONS
     }
     catch (...)
     {
         __is.__set_badbit_and_consider_rethrow();
     }
 #endif  // _LIBCPP_NO_EXCEPTIONS
     return __is;
 }
 
+#if _LIBCPP_STD_VER > 17
+
+template<class _CharT, class _Traits, size_t _Np>
+inline _LIBCPP_INLINE_VISIBILITY
+basic_istream<_CharT, _Traits>&
+operator>>(basic_istream<_CharT, _Traits>& __is, _CharT (&__buf)[_Np])
+{
+    auto __n = _Np;
+    if (__is.width() > 0)
+        __n = _VSTD::min(size_t(__is.width()), _Np);
+    return _VSTD::__input_c_string(__is, __buf, __n);
+}
+
+template<class _Traits, size_t _Np>
+inline _LIBCPP_INLINE_VISIBILITY
+basic_istream<char, _Traits>&
+operator>>(basic_istream<char, _Traits>& __is, unsigned char (&__buf)[_Np])
+{
+    return __is >> (char(&)[_Np])__buf;
+}
+
+template<class _Traits, size_t _Np>
+inline _LIBCPP_INLINE_VISIBILITY
+basic_istream<char, _Traits>&
+operator>>(basic_istream<char, _Traits>& __is, signed char (&__buf)[_Np])
+{
+    return __is >> (char(&)[_Np])__buf;
+}
+
+#else
+
+template<class _CharT, class _Traits>
+inline _LIBCPP_INLINE_VISIBILITY
+basic_istream<_CharT, _Traits>&
+operator>>(basic_istream<_CharT, _Traits>& __is, _CharT* __s)
+{
+    streamsize __n = __is.width();
+    if (__n <= 0)
+        __n = numeric_limits<streamsize>::max() / sizeof(_CharT) - 1;
+    return _VSTD::__input_c_string(__is, __s, size_t(__n));
+}
+
 template<class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_istream<char, _Traits>&
 operator>>(basic_istream<char, _Traits>& __is, unsigned char* __s)
 {
     return __is >> (char*)__s;
 }
 
 template<class _Traits>
 inline _LIBCPP_INLINE_VISIBILITY
 basic_istream<char, _Traits>&
 operator>>(basic_istream<char, _Traits>& __is, signed char* __s)
 {
     return __is >> (char*)__s;
 }
 
+#endif  // _LIBCPP_STD_VER > 17
+
 template<class _CharT, class _Traits>
 basic_istream<_CharT, _Traits>&
 operator>>(basic_istream<_CharT, _Traits>& __is, _CharT& __c)
 {
 #ifndef _LIBCPP_NO_EXCEPTIONS
     try
     {
 #endif  // _LIBCPP_NO_EXCEPTIONS

test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/signed_char_pointer.pass.cpp

int main()
         is.width(4);
         signed char s[20];
         is >> s;
         assert(!is.eof());
         assert(!is.fail());
         assert(std::string((char*)s) == "abc");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk    ");
+        std::istream is(&sb);
+        signed char s[4];
+        is >> s;
+        assert(!is.eof());
+        assert(!is.fail());
+        assert(std::string((char*)s) == "abc");
+    }
+#endif
     {
         testbuf<char> sb("   abcdefghijk");
         std::istream is(&sb);
         signed char s[20];
         is >> s;
         assert( is.eof());
         assert(!is.fail());
         assert(std::string((char*)s) == "abcdefghijk");
         assert(is.width() == 0);
     }
     {
         testbuf<char> sb("   abcdefghijk");
         std::istream is(&sb);
         signed char s[20];
         is.width(1);
         is >> s;
         assert(!is.eof());
         assert( is.fail());
         assert(std::string((char*)s) == "");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk");
+        std::istream is(&sb);
+        signed char s[1];
+        is >> s;
+        assert(!is.eof());
+        assert( is.fail());
+        assert(std::string((char*)s) == "");
+    }
+#endif
 }

test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/unsigned_char_pointer.pass.cpp

int main()
         is.width(4);
         unsigned char s[20];
         is >> s;
         assert(!is.eof());
         assert(!is.fail());
         assert(std::string((char*)s) == "abc");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk    ");
+        std::istream is(&sb);
+        unsigned char s[4];
+        is >> s;
+        assert(!is.eof());
+        assert(!is.fail());
+        assert(std::string((char*)s) == "abc");
+    }
+#endif
     {
         testbuf<char> sb("   abcdefghijk");
         std::istream is(&sb);
         unsigned char s[20];
         is >> s;
         assert( is.eof());
         assert(!is.fail());
         assert(std::string((char*)s) == "abcdefghijk");
         assert(is.width() == 0);
     }
     {
         testbuf<char> sb("   abcdefghijk");
         std::istream is(&sb);
         unsigned char s[20];
         is.width(1);
         is >> s;
         assert(!is.eof());
         assert( is.fail());
         assert(std::string((char*)s) == "");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk");
+        std::istream is(&sb);
+        unsigned char s[1];
+        is >> s;
+        assert(!is.eof());
+        assert( is.fail());
+        assert(std::string((char*)s) == "");
+    }
+#endif
 }

test/std/input.output/iostream.format/input.streams/istream.formatted/istream_extractors/wchar_t_pointer.pass.cpp

int main()
         testbuf<char> sb("   abcdefghijk    ");
         std::istream is(&sb);
         char s[20];
         is >> s;
         assert(!is.eof());
         assert(!is.fail());
         assert(std::string(s) == "abcdefghijk");
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk    ");
+        std::istream is(&sb);
+        char s[4];
+        is >> s;
+        assert(!is.eof());
+        assert(!is.fail());
+        assert(std::string(s) == "abc");
+    }
+#endif
     {
         testbuf<wchar_t> sb(L"   abcdefghijk    ");
         std::wistream is(&sb);
         is.width(4);
         wchar_t s[20];
         is >> s;
         assert(!is.eof());
         assert(!is.fail());
         assert(std::wstring(s) == L"abc");
         assert(is.width() == 0);
     }
     {
         testbuf<wchar_t> sb(L"   abcdefghijk");
         std::wistream is(&sb);
         wchar_t s[20];
         is >> s;
         assert( is.eof());
         assert(!is.fail());
         assert(std::wstring(s) == L"abcdefghijk");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<wchar_t> sb(L"   abcdefghijk");
+        std::wistream is(&sb);
+        wchar_t s[4];
+        is >> s;
+        assert(!is.eof());
+        assert(!is.fail());
+        assert(std::wstring(s) == L"abc");
+    }
+#endif
     {
         testbuf<char> sb("   abcdefghijk");
         std::istream is(&sb);
         char s[20];
         is.width(1);
         is >> s;
         assert(!is.eof());
         assert( is.fail());
         assert(std::string(s) == "");
         assert(is.width() == 0);
     }
+#if TEST_STD_VER > 17
+    {
+        testbuf<char> sb("   abcdefghijk");
+        std::istream is(&sb);
+        char s[1];
+        is >> s;
+        assert(!is.eof());
+        assert( is.fail());
+        assert(std::string(s) == "");
+    }
+#endif
 }

www/cxx2a_status.html

-->
   <tr><td><a href="https://wg21.link/P1025R1">P1025R1</a></td><td>CWG</td><td>Update The Reference To The Unicode Standard</td><td>Rapperswil</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1120R0">P1120R0</a></td><td>CWG</td><td>Consistency improvements for &lt;=&gt; and other comparison operators</td><td>Rapperswil</td><td></td><td></td></tr>
 
    <tr><td></td><td></td><td></td><td></td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0318R1">P0318R1</a></td><td>LWG</td><td>unwrap_ref_decay and unwrap_reference</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0356R5">P0356R5</a></td><td>LWG</td><td>Simplified partial function application</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0357R3">P0357R3</a></td><td>LWG</td><td>reference_wrapper for incomplete types</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0482R6">P0482R6</a></td><td>CWG</td><td>char8_t: A type for UTF-8 characters and strings</td><td>San Diego</td><td><i> </i></td><td></td></tr>
-  <tr><td><a href="https://wg21.link/P0487R1">P0487R1</a></td><td>LWG</td><td>Fixing operator&gt;&gt;(basic_istream&amp;, CharT*) (LWG 2499)</td><td>San Diego</td><td><i> </i></td><td></td></tr>
+  <tr><td><a href="https://wg21.link/P0487R1">P0487R1</a></td><td>LWG</td><td>Fixing operator&gt;&gt;(basic_istream&amp;, CharT*) (LWG 2499)</td><td>San Diego</td><td>Complete</td><td>8.0</td></tr>
   <tr><td><a href="https://wg21.link/P0591R4">P0591R4</a></td><td>LWG</td><td>Utility functions to implement uses-allocator construction</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0595R2">P0595R2</a></td><td>CWG</td><td>P0595R2 std::is_constant_evaluated()</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0602R4">P0602R4</a></td><td>LWG</td><td>variant and optional should propagate copy/move triviality</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0608R3">P0608R3</a></td><td>LWG</td><td>A sane variant converting constructor</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0655R1">P0655R1</a></td><td>LWG</td><td>visit&lt;R&gt;: Explicit Return Type for visit</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0771R1">P0771R1</a></td><td>LWG</td><td>std::function move constructor should be noexcept</td><td>San Diego</td><td>Complete</td><td>6.0</td></tr>
   <tr><td><a href="https://wg21.link/P0896R4">P0896R4</a></td><td>LWG</td><td>The One Ranges Proposal</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0899R1">P0899R1</a></td><td>LWG</td><td>P0899R1 - LWG 3016 is not a defect</td><td>San Diego</td><td><i>Nothing to do</i></td><td></td></tr>