[libc++] Fix wraparound issue with -fsanitize=integer in string operator>> #106263
Conversation
…tor>> Fixes llvm#106261 rdar://133991190
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesFixes #106261 Full diff: https://github.com/llvm/llvm-project/pull/106263.diff 1 Files Affected:
diff --git a/libcxx/include/istream b/libcxx/include/istream
index d2b577a9ad9efc..84cb0ac9d9f252 100644
--- a/libcxx/include/istream
+++ b/libcxx/include/istream
@@ -1211,12 +1211,12 @@ operator>>(basic_istream<_CharT, _Traits>& __is, basic_string<_CharT, _Traits, _
try {
#endif
__str.clear();
- streamsize __n = __is.width();
- if (__n <= 0)
- __n = __str.max_size();
- if (__n <= 0)
- __n = numeric_limits<streamsize>::max();
- streamsize __c = 0;
+ using _Size = typename basic_string<_CharT, _Traits, _Allocator>::size_type;
+ static_assert(numeric_limits<_Size>::max() >= numeric_limits<streamsize>::max(),
+ "Stream width could be too large to be represented in the string's size_type");
+ streamsize const __width = __is.width();
+ _Size const __n = __width <= 0 ? __str.max_size() : static_cast<_Size>(__width);
+ _Size __c = 0;
const ctype<_CharT>& __ct = std::use_facet<ctype<_CharT> >(__is.getloc());
while (__c < __n) {
typename _Traits::int_type __i = __is.rdbuf()->sgetc();
|
libcxx/include/istream
Outdated
| __n = numeric_limits<streamsize>::max(); | ||
| streamsize __c = 0; | ||
| using _Size = typename basic_string<_CharT, _Traits, _Allocator>::size_type; | ||
| static_assert(numeric_limits<_Size>::max() >= numeric_limits<streamsize>::max(), |
There was a problem hiding this comment.
Why...? Can't set allocator size_type to 32-bit?
There was a problem hiding this comment.
The other alternative would be to use streamsize for the counter and do something like:
streamsize const __width = __is.width();
streamsize const __n = __width <= 0 ? std::clamp(__str.max_size(), 0, numeric_limits<streamsize>::max()) : __width;
WDYT?
There was a problem hiding this comment.
__str.max_size() may not be representable in streamsize, either. And we'd better not let __n exceed max_size() in practice, so it looks reasonable to count by size_type. If __is.width() is larger than max_size() at runtime, stores max_size().
There was a problem hiding this comment.
Let me know what you think about the latest version.
libcxx/include/istream
Outdated
| using _Size = typename basic_string<_CharT, _Traits, _Allocator>::size_type; | ||
| streamsize const __width = __is.width(); | ||
| _Size const __max_size = __str.max_size(); | ||
| _Size const __n = __width <= 0 ? __max_size : std::min(__max_size, static_cast<_Size>(__width)); |
There was a problem hiding this comment.
If width is <= 0, we use max_size() as-is. Otherwise, we use the smallest of width and max_size(), which accounts for the case where max_size() would be e.g. a 32 bit integer.
There was a problem hiding this comment.
static_cast<_Size>(__width) still wrap around, right? I'm thinking
_Size const __n = [](streamsize __width, _Size __max_size) {
if (__width <= 0)
return __max_size;
else
{
auto const __asked = static_cast<std::make_unsigned<streamsize>::type>(__width);
if (__asked < __max_size)
return static_cast<_Size>(__asked);
else
return __max_size;
}
}(__is.width(), __str.max_size());There was a problem hiding this comment.
Ugh, yeah, that's better.
Fixes #106261
rdar://133991190