| Message ID | 20190223010220.GA32714@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR libstdc++/89446 fix null pointer dereference in char_traits | expand |
On Sat, Feb 23, 2019 at 01:02:20AM +0000, Jonathan Wakely wrote: > PR libstdc++/89446 > * include/bits/char_traits.h (__constant_char_array): Check index is > in range before dereferencing. > * testsuite/21_strings/basic_string_view/operators/char/89446.cc: > New test. > > Tested x86_64-linux, committed to gcc-8-branch and gcc-7-branch. And not trunk? The bug is still there, even when it should be usually ifdefed out because __builtin_is_constexpr_evaluated() should be supported. > commit b639a9cac6e2532eb852b03df6ac40d34f1dd28c > Author: Jonathan Wakely <jwakely@redhat.com> > Date: Fri Feb 22 20:33:16 2019 +0000 > > PR libstdc++/89446 fix null pointer dereference in char_traits > > PR libstdc++/89446 > * include/bits/char_traits.h (__constant_char_array): Check index is > in range before dereferencing. > * testsuite/21_strings/basic_string_view/operators/char/89446.cc: > New test. > > diff --git a/libstdc++-v3/include/bits/char_traits.h b/libstdc++-v3/include/bits/char_traits.h > index 1945494d7e2..a2a883f3565 100644 > --- a/libstdc++-v3/include/bits/char_traits.h > +++ b/libstdc++-v3/include/bits/char_traits.h > @@ -248,7 +248,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > __constant_char_array_p(const _CharT* __a, size_t __n) > { > size_t __i = 0; > - while (__builtin_constant_p(__a[__i]) && __i < __n) > + while (__i < __n && __builtin_constant_p(__a[__i])) > __i++; > return __i == __n; > } Jakub
On 23/02/19 02:06 +0100, Jakub Jelinek wrote: >On Sat, Feb 23, 2019 at 01:02:20AM +0000, Jonathan Wakely wrote: >> PR libstdc++/89446 >> * include/bits/char_traits.h (__constant_char_array): Check index is >> in range before dereferencing. >> * testsuite/21_strings/basic_string_view/operators/char/89446.cc: >> New test. >> >> Tested x86_64-linux, committed to gcc-8-branch and gcc-7-branch. > >And not trunk? The bug is still there, even when it should be usually >ifdefed out because __builtin_is_constexpr_evaluated() should be supported. Yes, that's a bigger patch (with some cleanup) which is coming shortly.
On 23/02/19 01:02 +0000, Jonathan Wakely wrote: >+// { dg-options "-std=gnu++17 -fexceptions -fnon-call-exceptions -O1" } >+// { dg-do run { target { powerpc*-*-linux* i?86-*-linux* x86_64-*-linux* } } } I forgot to say that this is a conservative list of targets where -fnon-call-exceptions is supported. Maybe it could run on other targets, but this should be enough to ensure we don't get a regression for this bug.
On 23/02/19 01:26 +0000, Jonathan Wakely wrote: >On 23/02/19 02:06 +0100, Jakub Jelinek wrote: >>On Sat, Feb 23, 2019 at 01:02:20AM +0000, Jonathan Wakely wrote: >>> PR libstdc++/89446 >>> * include/bits/char_traits.h (__constant_char_array): Check index is >>> in range before dereferencing. >>> * testsuite/21_strings/basic_string_view/operators/char/89446.cc: >>> New test. >>> >>>Tested x86_64-linux, committed to gcc-8-branch and gcc-7-branch. >> >>And not trunk? The bug is still there, even when it should be usually >>ifdefed out because __builtin_is_constexpr_evaluated() should be supported. > >Yes, that's a bigger patch (with some cleanup) which is coming >shortly. Here's the patch for trunk. When the size is zero we can just return, we don't need to do the __builtin_constant_p checks. Tested powerpc64le-linux, committed to trunk. commit c1f64344793df8b29ccb27d06572ba8d5b51957b Author: Jonathan Wakely <jwakely@redhat.com> Date: Sat Feb 23 01:33:02 2019 +0000 PR libstdc++/89446 fix null pointer dereference in char_traits PR libstdc++/89446 * include/bits/char_traits.h (__constant_char_array): Check index is in range before dereferencing. (char_traits<char>::compare, char_traits<char>::find) (char_traits<char8_t>::compare, char_traits<char8_t>::find): Return immediately if n is zero. (char_traits<wchar_t>::compare, char_traits<wchar_t>::find): Likewise. Remove workarounds for PR 67026. * testsuite/21_strings/basic_string_view/operators/char/89446.cc: New test. * testsuite/21_strings/basic_string_view/operators/wchar_t/89446.cc: New test. diff --git a/libstdc++-v3/include/bits/char_traits.h b/libstdc++-v3/include/bits/char_traits.h index 21099c36c3b..fd9a3c73930 100644 --- a/libstdc++-v3/include/bits/char_traits.h +++ b/libstdc++-v3/include/bits/char_traits.h @@ -260,7 +260,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return __builtin_is_constant_evaluated(); #else size_t __i = 0; - while (__builtin_constant_p(__a[__i]) && __i < __n) + while (__i < __n && __builtin_constant_p(__a[__i])) __i++; return __i == __n; #endif @@ -314,14 +314,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX17_CONSTEXPR int compare(const char_type* __s1, const char_type* __s2, size_t __n) { + if (__n == 0) + return 0; #if __cplusplus >= 201703L if (__builtin_constant_p(__n) && __constant_char_array_p(__s1, __n) && __constant_char_array_p(__s2, __n)) return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n); #endif - if (__n == 0) - return 0; return __builtin_memcmp(__s1, __s2, __n); } @@ -338,14 +338,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX17_CONSTEXPR const char_type* find(const char_type* __s, size_t __n, const char_type& __a) { + if (__n == 0) + return 0; #if __cplusplus >= 201703L if (__builtin_constant_p(__n) && __builtin_constant_p(__a) && __constant_char_array_p(__s, __n)) return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a); #endif - if (__n == 0) - return 0; return static_cast<const char_type*>(__builtin_memchr(__s, __a, __n)); } @@ -423,16 +423,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX17_CONSTEXPR int compare(const char_type* __s1, const char_type* __s2, size_t __n) { + if (__n == 0) + return 0; #if __cplusplus >= 201703L if (__builtin_constant_p(__n) && __constant_char_array_p(__s1, __n) && __constant_char_array_p(__s2, __n)) return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n); #endif - if (__n == 0) - return 0; - else - return wmemcmp(__s1, __s2, __n); + return wmemcmp(__s1, __s2, __n); } static _GLIBCXX17_CONSTEXPR size_t @@ -441,24 +440,22 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201703L if (__constant_string_p(__s)) return __gnu_cxx::char_traits<char_type>::length(__s); - else #endif - return wcslen(__s); + return wcslen(__s); } static _GLIBCXX17_CONSTEXPR const char_type* find(const char_type* __s, size_t __n, const char_type& __a) { + if (__n == 0) + return 0; #if __cplusplus >= 201703L if (__builtin_constant_p(__n) && __builtin_constant_p(__a) && __constant_char_array_p(__s, __n)) return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a); #endif - if (__n == 0) - return 0; - else - return wmemchr(__s, __a, __n); + return wmemchr(__s, __a, __n); } static char_type* @@ -532,14 +529,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX17_CONSTEXPR int compare(const char_type* __s1, const char_type* __s2, size_t __n) { + if (__n == 0) + return 0; #if __cplusplus > 201402 if (__builtin_constant_p(__n) && __constant_char_array_p(__s1, __n) && __constant_char_array_p(__s2, __n)) return __gnu_cxx::char_traits<char_type>::compare(__s1, __s2, __n); #endif - if (__n == 0) - return 0; return __builtin_memcmp(__s1, __s2, __n); } @@ -559,14 +556,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION static _GLIBCXX17_CONSTEXPR const char_type* find(const char_type* __s, size_t __n, const char_type& __a) { + if (__n == 0) + return 0; #if __cplusplus > 201402 if (__builtin_constant_p(__n) && __builtin_constant_p(__a) && __constant_char_array_p(__s, __n)) return __gnu_cxx::char_traits<char_type>::find(__s, __n, __a); #endif - if (__n == 0) - return 0; return static_cast<const char_type*>(__builtin_memchr(__s, __a, __n)); } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc new file mode 100644 index 00000000000..768ba63ddfe --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc @@ -0,0 +1,28 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -fexceptions -fnon-call-exceptions -O1" } +// { dg-do run { target { powerpc*-*-linux* i?86-*-linux* x86_64-*-linux* } } } +// { dg-require-effective-target c++17 } + +#include <string_view> + +int main() +{ + std::string_view s1, s2; + return s1 != s2; +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/wchar_t/89446.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/wchar_t/89446.cc new file mode 100644 index 00000000000..a0ecbebea92 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/wchar_t/89446.cc @@ -0,0 +1,28 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -fexceptions -fnon-call-exceptions -O1" } +// { dg-do run { target { powerpc*-*-linux* i?86-*-linux* x86_64-*-linux* } } } +// { dg-require-effective-target c++17 } + +#include <string_view> + +int main() +{ + std::wstring_view s1, s2; + return s1 != s2; +}
> I forgot to say that this is a conservative list of targets where > -fnon-call-exceptions is supported. Maybe it could run on other > targets, but this should be enough to ensure we don't get a regression > for this bug. What do you mean by supported exactly? Ada and Go set it by default and they are supported on a bunch of other targets...
On 23/02/19 10:55 +0100, Eric Botcazou wrote: >> I forgot to say that this is a conservative list of targets where >> -fnon-call-exceptions is supported. Maybe it could run on other >> targets, but this should be enough to ensure we don't get a regression >> for this bug. > >What do you mean by supported exactly? The manual for -fnon-call-exceptions says: "Note that this requires platform-specific runtime support that does not exist everywhere." I don't know exactly what that means, so the test only runs on targets where I know it works. >Ada and Go set it by default and they >are supported on a bunch of other targets... It's not supposed to be an exhaustive list of targets where it's supported. As I said, it's a conservative list. It could run on more targets, but there might not be much benefit as long as the test runs on some targets that get tested often.
diff --git a/libstdc++-v3/include/bits/char_traits.h b/libstdc++-v3/include/bits/char_traits.h index 1945494d7e2..a2a883f3565 100644 --- a/libstdc++-v3/include/bits/char_traits.h +++ b/libstdc++-v3/include/bits/char_traits.h @@ -248,7 +248,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION __constant_char_array_p(const _CharT* __a, size_t __n) { size_t __i = 0; - while (__builtin_constant_p(__a[__i]) && __i < __n) + while (__i < __n && __builtin_constant_p(__a[__i])) __i++; return __i == __n; } diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc new file mode 100644 index 00000000000..768ba63ddfe --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operators/char/89446.cc @@ -0,0 +1,28 @@ +// Copyright (C) 2019 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -fexceptions -fnon-call-exceptions -O1" } +// { dg-do run { target { powerpc*-*-linux* i?86-*-linux* x86_64-*-linux* } } } +// { dg-require-effective-target c++17 } + +#include <string_view> + +int main() +{ + std::string_view s1, s2; + return s1 != s2; +}
PR libstdc++/89446 * include/bits/char_traits.h (__constant_char_array): Check index is in range before dereferencing. * testsuite/21_strings/basic_string_view/operators/char/89446.cc: New test. Tested x86_64-linux, committed to gcc-8-branch and gcc-7-branch. commit b639a9cac6e2532eb852b03df6ac40d34f1dd28c Author: Jonathan Wakely <jwakely@redhat.com> Date: Fri Feb 22 20:33:16 2019 +0000 PR libstdc++/89446 fix null pointer dereference in char_traits PR libstdc++/89446 * include/bits/char_traits.h (__constant_char_array): Check index is in range before dereferencing. * testsuite/21_strings/basic_string_view/operators/char/89446.cc: New test.