-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[libc++] Fix wraparound issue with -fsanitize=integer in string operator>> #106263
Conversation
…tor>> Fixes llvm#106261 rdar://133991190
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why...? Can't set allocator size_type to 32-bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, yeah, that's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes #106261
rdar://133991190