[patch] Make vector::at() assertion message more useful (try #2)
Greetings,
This is a followup to:
http://gcc.gnu.org/ml/libstdc++/2013-08/msg00096.html
Without this patch, the user on vector::at out of bounds sees:
terminate called after throwing an instance of 'std::out_of_range'
what(): vector::_M_range_check
Aborted (core dumped)
With the patch:
terminate called after throwing an instance of 'std::out_of_range'
what(): vector::_M_range_check: __n (which is 3) >= this->size() (which is 2)
Aborted (core dumped)
I am not at all sure the names I choose here are good ones. Suggestions
welcome.
I also shudder at the idea of repeating _M_range_check code in
e.g. string::at(), and elsewhere. Perhaps we need a snprintf_lite, that
only understands '%zu' and literal characters, e.g.:
snprintf_lite(__s, sizeof(__s),
_N("vector::_M_range_check: __n (which is %zu) >= "
"this->size() (which is %zu)"), __n, this->size());
[The patch also doesn't include libstdc++-v3/libsupc++/Makefile.in,
which I'll regenerate before submitting.]
[Please CC me on any replies.]
--
Paul Pluzhnikov
2013-09-04 Paul Pluzhnikov
* libstdc++-v3/config/abi/pre/gnu.ver: Add
_ZN9__gnu_cxx13concat_size_tEPcm
* libstdc++-v3/include/bits/stl_vector.h (_M_range_check): Print
additional assertion details
* libstdc++-v3/libsupc++/Makefile.am: Add support.cc
* libstdc++-v3/libsupc++/support.cc: New
*
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewise.
Index: libstdc++-v3/config/abi/pre/gnu.ver
===
--- libstdc++-v3/config/abi/pre/gnu.ver (revision 202262)
+++ libstdc++-v3/config/abi/pre/gnu.ver (working copy)
@@ -1365,6 +1365,9 @@
# std::get_unexpected()
_ZSt14get_unexpectedv;
+# __gnu_cxx::concat_size_t(char*, unsigned long)
+_ZN9__gnu_cxx13concat_size_tEPcm;
+
} GLIBCXX_3.4.19;
# Symbols in the support library (libsupc++) have their own tag.
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 202262)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -63,6 +63,10 @@
#include
#endif
+namespace __gnu_cxx {
+ int concat_size_t(char *, size_t);
+}
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_CONTAINER
@@ -791,7 +795,26 @@
_M_range_check(size_type __n) const
{
if (__n >= this->size())
- __throw_out_of_range(__N("vector::_M_range_check"));
+ {
+ const char __p[] = __N("vector::_M_range_check: __n (which is ");
+ const char __q[] = __N(") >= this->size() (which is ");
+
+ // Enough space for __p, __q, size and __n (in decimal).
+ const int __alloc_size =
+ sizeof(__p) + sizeof(__q) + 3 * 2 * sizeof(size_type) + 10;
+
+ char *__s = static_cast(__builtin_alloca(__alloc_size));
+ char *__ps = __s;
+ __builtin_memcpy(__ps, __p, sizeof(__p));
+ __ps += sizeof(__p) - 1;
+ __ps += __gnu_cxx::concat_size_t(__ps, __n);
+ __builtin_memcpy(__ps, __q, sizeof(__q));
+ __ps += sizeof(__q) - 1;
+ __ps += __gnu_cxx::concat_size_t(__ps, this->size());
+ *(__ps++) = __N(')');
+ *(__ps++) = '\0';
+ __throw_out_of_range(__s);
+ }
}
public:
Index: libstdc++-v3/libsupc++/Makefile.am
===
--- libstdc++-v3/libsupc++/Makefile.am (revision 202262)
+++ libstdc++-v3/libsupc++/Makefile.am (working copy)
@@ -91,6 +91,7 @@
pointer_type_info.cc \
pure.cc \
si_class_type_info.cc \
+ support.cc \
tinfo.cc \
tinfo2.cc \
vec.cc \
Index: libstdc++-v3/libsupc++/support.cc
===
--- libstdc++-v3/libsupc++/support.cc (revision 0)
+++ libstdc++-v3/libsupc++/support.cc (revision 0)
@@ -0,0 +1,53 @@
+// Debugging support -*- C++ -*-
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of GCC.
+//
+// GCC 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.
+//
+// GCC is distributed in the hope that it will be useful,
+// but
Re: [patch] Make vector::at() assertion message more useful (try #2)
On Wed, Sep 4, 2013 at 2:10 PM, Daniel Krügler
wrote:
> I expect that this kind of index violation is a rather often occurring
> pattern and should be isolated. IMO the _M_range
> _check now pessimisms the normal, non-violating case.
Did you mean "pessimises code size", or something else?
> Why not simply
> replacing it by something along the lines of
>
> _M_range_check(size_type __n) const
>{
> if (__n >= this->size())
>
> __throw_out_of_range(__index_out_of_range_msg(__N("vector::_M_range_check"),
>__n, this->size()));
>}
>
> where __index_out_of_range_msg is a reusable function that uses
> something like snprintf_lite internally?
Some disadvantages to doing that:
1. The actual message is now "magically" transformed inside
__index_out_of_range_msg into the final message, making translation
more difficult.
2. __index_out_of_range_msg() would have to return a string, which is heavier
weight (in try#1 I just used snprintf, which was considered "too heavy").
Thanks,
--
Paul Pluzhnikov
Re: [patch] Make vector::at() assertion message more useful (try #2)
On Wed, Sep 4, 2013 at 4:26 PM, Paolo Carlini wrote:
> For sure concat_size would not be Ok, isn't uglified.
I didn't uglify it because it's inside __gnu_cxx namespace.
Does it still need uglification?
>>snprintf_lite(__s, sizeof(__s),
>> _N("vector::_M_range_check: __n (which is %zu) >= "
>> "this->size() (which is %zu)"), __n, this->size());
>
> That seems worth exploring, I agree.
Should snprintf_lite be in __gnu_cxx namespace, or be global and be called
__snprintf_lite(), or ...?
Is the location of the out-of-line code in libstdc++-v3/libsupc++/ ok?
(Would probably be called snprintf_lite.cc or some such.)
Is the version I've assigned to the symbol -- GLIBCXX_3.4.20 -- ok?
Thanks,
--
Paul Pluzhnikov
Re: [patch] Make cxxfilt demangle internal-linkage templates
Ping x2? Original message: http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html On Fri, Aug 16, 2013 at 6:10 PM, Paul Pluzhnikov wrote: > Ping? -- Paul Pluzhnikov
Re: [patch] Make vector::at() assertion message more useful (try #2)
On Wed, Sep 4, 2013 at 9:55 PM, Daniel Krügler
wrote:
>> Did you mean "pessimises code size", or something else?
>
> Yes.
Daniel's idea proved a good one, and I now have a patch that I am
happy with, and that will be easy to extend to string::at(), and other
__throw_... functions.
I've added the new snprintf.cc to c++11/ rather than c++98/ as Paolo
suggested, because the only current caller is in c++11/functexcept.cc
Thanks,
--
Paul Pluzhnikov
libstdc++-v3/ChangeLog:
2013-09-12 Paul Pluzhnikov
* src/c++11/Makefile.am: Add snprintf.cc
* src/c++11/Makefile.in: Regenerate.
* include/bits/functexcept.h (__throw_out_of_range): Adjust.
* src/c++11/functexcept.cc (__throw_out_of_range): New overload.
* src/c++11/snprintf.cc: New.
* config/abi/pre/gnu.ver: Add _ZSt20__throw_out_of_rangePKcz.
* include/bits/stl_vector.h (_M_range_check): Print
additional assertion details.
* testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewise.
Index: libstdc++-v3/src/c++11/Makefile.am
===
--- libstdc++-v3/src/c++11/Makefile.am (revision 202545)
+++ libstdc++-v3/src/c++11/Makefile.am (working copy)
@@ -42,6 +42,7 @@
random.cc \
regex.cc \
shared_ptr.cc \
+ snprintf.cc \
system_error.cc \
thread.cc
Index: libstdc++-v3/src/c++11/functexcept.cc
===
--- libstdc++-v3/src/c++11/functexcept.cc (revision 202545)
+++ libstdc++-v3/src/c++11/functexcept.cc (working copy)
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#ifdef _GLIBCXX_USE_NLS
# include
@@ -39,6 +40,12 @@
# define _(msgid) (msgid)
#endif
+namespace __gnu_cxx
+{
+ int __snprintf_lite(char *__buf, size_t __bufsize, const char *__fmt,
+ va_list __ap);
+}
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -75,11 +82,27 @@
__throw_length_error(const char* __s __attribute__((unused)))
{ _GLIBCXX_THROW_OR_ABORT(length_error(_(__s))); }
+ // Left only to maintain backward-compatible ABI.
void
__throw_out_of_range(const char* __s __attribute__((unused)))
{ _GLIBCXX_THROW_OR_ABORT(out_of_range(_(__s))); }
void
+ __throw_out_of_range(const char* __fmt, ...)
+ {
+const size_t __len = __builtin_strlen(__fmt);
+// enough for expanding up to 5 size_t's in the format.
+const size_t __alloca_size = __len + 100;
+char *const __s = static_cast(alloca(__alloca_size));
+va_list __ap;
+
+va_start(__ap, __fmt);
+__gnu_cxx::__snprintf_lite(__s, __alloca_size, __fmt, __ap);
+_GLIBCXX_THROW_OR_ABORT(out_of_range(_(__s)));
+va_end(__ap); // Not reached.
+ }
+
+ void
__throw_runtime_error(const char* __s __attribute__((unused)))
{ _GLIBCXX_THROW_OR_ABORT(runtime_error(_(__s))); }
Index: libstdc++-v3/src/c++11/snprintf.cc
===
--- libstdc++-v3/src/c++11/snprintf.cc (revision 0)
+++ libstdc++-v3/src/c++11/snprintf.cc (revision 0)
@@ -0,0 +1,103 @@
+// Debugging support -*- C++ -*-
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of GCC.
+//
+// GCC 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.
+//
+// GCC 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.
+//
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
+// <http://www.gnu.org/licenses/>.
+
+#include
+#include
+
+namespace std {
+ template
+ int
+ __int_to_char(_CharT* __bufend, _ValueT __v, const _CharT* __lit,
+ios_base::fmtflags __flags, bool __dec);
+}
+
+namespace __gnu_cxx {
+
+ // Private routine to append decimal representation of VAL to the given
+ // BUFFER, but not more than BUFSIZE characters.
+ // Does not NUL-terminate the output
Re: [patch] Make cxxfilt demangle internal-linkage templates
Ping x3? 2013/9/11 Paul Pluzhnikov : > Ping x2? > > Original message: > http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html Thanks, -- Paul Pluzhnikov
Re: [PATCH] Fix PR58417 -- r202700 appears to be causing ICEs
tiate_scev): Construct global instead of local cache. (resolve_mixers): Likewise. * gcc.dg/torture/pr58417.c: New testcase. Thanks, -- Paul Pluzhnikov
Re: [patch] Make vector::at() assertion message more useful (try #2)
On Fri, Sep 13, 2013 at 3:02 AM, Paolo Carlini wrote:
> - The game with the variadic and the non-variadic __throw_out_of_range makes
> me a little nervous. Let's just name the new one differently, like
> __throw_out_of_range_var.
Replaced with __throw_out_of_range_fmt.
> - Please consistently use __builtin_alloca everywhere, alloca isn't a
> standard function.
Done.
> - I would rather call the file itself snprintf_lite.cc, in order not to fool
> somebody that it actually implements the whole snprintf.
Done.
> - I'm a bit confused about __concat_size_t returning -1. Since it only
> formats integers, I think we can be *sure* that the buffer is big enough.
How so? The caller could do this:
__snprintf_lite("aaa: %zu, 8, 12345678);
By the time we get to __concat_size_t, we only have 3 characters left.
> Then, if it returns -1 something is going *very badly* wrong, shouldn't we
> __builtin_abort() or something similar?
I've re-worked this part -- if this ever happens, throw a logic_error with
a message to file a bug.
> - In terms of buffer sizes, this comment:
>
> // enough for expanding up to 5 size_t's in the format.
>
> and then the actual code in __snprintf_lite makes me a little nervous.
> Agreed, we are not going to overflow the buffer, but truncating with no
> diagnostic whatsoever seems rather gross. We can probably sort out this
> later, new ideas welcome, anyway.
Re-worked.
> - While we are at it, shouldn't we use the new facility at least in array,
> vector and deque too? For consistency over the containers.
Done.
I also expanded snprintf_lite to handle '%s', as that comes handy inside
string _M_check.
Thanks,
P.S. In the process of updating callers of __throw_out_of_range, I've
discovered that two of them had already used __builtin_sprintf.
P.P.S. Sorry this patch grew ... I can split it into parts if that's easier
to review.
--
Paul Pluzhnikov
libstdc++-v3/ChangeLog:
* include/bits/functexcept.h (__throw_out_of_range_fmt): New.
* src/c++11/functexcept.cc (__throw_out_of_range_fmt): New.
* src/c++11/snprintf_lite.cc: New.
* src/c++11/Makefile.am: Add snprintf_lite.cc.
* src/c++11/Makefile.in: Regenerate.
* config/abi/pre/gnu.ver: Add _ZSt24__throw_out_of_range_fmtPKcz.
* include/std/array (at): Use __throw_out_of_range_fmt.
* include/debug/array (at): Likewise.
* include/profile/array (at): Likewise.
* include/std/bitset (_M_check_initial_position, _M_check): New.
(bitset::bitset): Use _M_check_initial_position.
(set, reset, flip, test): Use _M_check.
* include/ext/vstring.h (_M_check, at): Use __throw_out_of_range_fmt.
* include/bits/stl_vector.h (_M_range_check): Likewise.
* include/bits/stl_bvector.h (_M_range_check): Likewise.
* include/bits/stl_deque.h (_M_range_check): Likewise.
* include/bits/basic_string.h (_M_check, at): Likewise.
* testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust.
* testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewise.
*
testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewise.
* testsuite/23_containers/deque/requirements/dr438/assign_neg.cc:
Likewise.
* testsuite/23_containers/deque/requirements/dr438/insert_neg.cc:
Likewise.
* testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc:
Likewise.
* testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc:
Likewise.
* testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc:
Likewise.
* testsuite/23_containers/array/tuple_interface/get_neg.cc: Likewise.
* testsuite/util/exception/safety.h (generate): Use
__throw_out_of_range_fmt.
Index: libstdc++-v3/include/bits/functexcept.h
===
--- libstdc++-v3/include/bits/functexcept.h (revision 202709)
+++ libstdc++-v3/include/bits/functexcept.h (working copy)
@@ -75,6 +75,10 @@
__throw_out_of_range(const char*) __attribute__((__noreturn__));
void
+ __throw_out_of_range_fmt(const char*, ...) __attribute__((__noreturn__))
+__attribute__((__format__(__printf__, 1, 2)));
+
+ void
__throw_runtime_error(const char*) __attribute__((__noreturn__));
void
Index: libstdc++-v3/src/c++11/functexcept.cc
===
--- libstdc++-v3/src/c++11/functexcept.cc (revision 202709)
+++ libstdc++-v3/src/c++11/functexcept.cc (working copy)
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#ifdef _GLIBCXX_USE_NLS
# include
@@ -39,6 +40,12 @@
# define _
Re: [patch] Make vector::at() assertion message more useful (try #2)
On Wed, Sep 18, 2013 at 3:00 PM, Paolo Carlini wrote: > In the meanwhile, since you are also touching debug-mode and > profile-mode, make sure to run check-debug and check-profile too. Thanks for mentioning that. Several more tests needed line number adjustments. There are also tests that are failing there independently of my patch. Committed as r202818. Thanks, -- Paul Pluzhnikov
Re: [patch] Make vector::at() assertion message more useful (try #2)
On 9/23/13 4:26 AM, Paolo Carlini wrote:
m68k-linux/./libstdc++-v3/src/.libs/libstdc++.so: undefined reference
to `int std::__int_to_char(char*, unsigned int,
char const*, std::_Ios_Fmtflags, bool)'
Reproduced on i686.
Sorry about the trouble ...
I would say, either make sure to use only those two in the new
code
Testing this patch:
Index: libstdc++-v3/src/c++11/snprintf_lite.cc
===
--- libstdc++-v3/src/c++11/snprintf_lite.cc (revision 202830)
+++ libstdc++-v3/src/c++11/snprintf_lite.cc (working copy)
@@ -70,9 +70,10 @@
int __concat_size_t(char *__buf, size_t __bufsize, size_t __val)
{
// Long enough for decimal representation.
-int __ilen = 3 * sizeof(__val);
+unsigned long long __val_ull = __val;
+int __ilen = 3 * sizeof(__val_ull);
char *__cs = static_cast(__builtin_alloca(__ilen));
-size_t __len = std::__int_to_char(__cs + __ilen, __val,
+size_t __len = std::__int_to_char(__cs + __ilen, __val_ull,
std::__num_base::_S_atoms_out,
std::ios_base::dec, true);
if (__bufsize < __len)
Re: [patch] Make vector::at() assertion message more useful (try #2)
On 9/23/13 7:48 AM, Paul Pluzhnikov wrote:
Testing this patch:
libstdc++ tests finished with
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}'
Committed as r202832.
Sorry about the trouble.
--
2013-09-23 Paul Pluzhnikov
* src/c++11/snprintf_lite.cc (__concat_size_t): Use only
std::__int_to_char()
Index: libstdc++-v3/src/c++11/snprintf_lite.cc
===
--- libstdc++-v3/src/c++11/snprintf_lite.cc (revision 202830)
+++ libstdc++-v3/src/c++11/snprintf_lite.cc (working copy)
@@ -70,9 +70,10 @@
int __concat_size_t(char *__buf, size_t __bufsize, size_t __val)
{
// Long enough for decimal representation.
-int __ilen = 3 * sizeof(__val);
+unsigned long long __val_ull = __val;
+int __ilen = 3 * sizeof(__val_ull);
char *__cs = static_cast(__builtin_alloca(__ilen));
-size_t __len = std::__int_to_char(__cs + __ilen, __val,
+size_t __len = std::__int_to_char(__cs + __ilen, __val_ull,
std::__num_base::_S_atoms_out,
std::ios_base::dec, true);
if (__bufsize < __len)
Re: [patch] Make vector::at() assertion message more useful (try #2)
On 9/23/13 7:54 AM, Paolo Carlini wrote:
Testing this patch:
In fact, however, that unsigned long long instantiation isn't
*unconditionally* available, see locale-inst.cc.
Thanks,
I think we have to use
unsigned long as a fall back controlled by the same macro. And please
add a comment explaining those contortions having to do with the
available instantiations. Patch pre-approved.
Does this look right?
Index: libstdc++-v3/src/c++11/snprintf_lite.cc
===
--- libstdc++-v3/src/c++11/snprintf_lite.cc (revision 202832)
+++ libstdc++-v3/src/c++11/snprintf_lite.cc (working copy)
@@ -69,11 +69,17 @@
// Returns number of characters appended, or -1 if BUFSIZE is too small.
int __concat_size_t(char *__buf, size_t __bufsize, size_t __val)
{
+// __int_to_char is explicitly instantiated and available only for
+// some, but not all, types.
+#ifdef _GLIBCXX_USE_LONG_LONG
+unsigned long long __val2 = __val;
+#else
+unsigned long __val2 = __val;
+#endif
// Long enough for decimal representation.
-unsigned long long __val_ull = __val;
-int __ilen = 3 * sizeof(__val_ull);
+int __ilen = 3 * sizeof(__val2);
char *__cs = static_cast(__builtin_alloca(__ilen));
-size_t __len = std::__int_to_char(__cs + __ilen, __val_ull,
+size_t __len = std::__int_to_char(__cs + __ilen, __val2,
std::__num_base::_S_atoms_out,
std::ios_base::dec, true);
if (__bufsize < __len)
Re: [patch] Make vector::at() assertion message more useful (try #2)
On 9/23/13 9:24 AM, Paolo Carlini wrote:
Does this look right?
Yes. Nit, in the comment I would also explicitly mention locale-inst.cc.
Done, submitted as r202836.
2013-09-23 Paul Pluzhnikov
* src/c++11/snprintf_lite.cc (__concat_size_t): Use
unsigned long long conditionally.
Index: libstdc++-v3/src/c++11/snprintf_lite.cc
===
--- libstdc++-v3/src/c++11/snprintf_lite.cc (revision 202832)
+++ libstdc++-v3/src/c++11/snprintf_lite.cc (working copy)
@@ -69,11 +69,17 @@
// Returns number of characters appended, or -1 if BUFSIZE is too small.
int __concat_size_t(char *__buf, size_t __bufsize, size_t __val)
{
+// __int_to_char is explicitly instantiated and available only for
+// some, but not all, types. See locale-inst.cc.
+#ifdef _GLIBCXX_USE_LONG_LONG
+unsigned long long __val2 = __val;
+#else
+unsigned long __val2 = __val;
+#endif
// Long enough for decimal representation.
-unsigned long long __val_ull = __val;
-int __ilen = 3 * sizeof(__val_ull);
+int __ilen = 3 * sizeof(__val2);
char *__cs = static_cast(__builtin_alloca(__ilen));
-size_t __len = std::__int_to_char(__cs + __ilen, __val_ull,
+size_t __len = std::__int_to_char(__cs + __ilen, __val2,
std::__num_base::_S_atoms_out,
std::ios_base::dec, true);
if (__bufsize < __len)
[google integration,gcc-4_8] Additional lightweight debug checks for std::deque
Greetings,
I've committed the patch below to google/integration (r202856) and
gcc-4_8 (r202857) branches. Google ref: b/10872448.
This cought approximately 10 bugs in our code.
See also r195357 (similar checks for std::vector) and PR 56109.
Thanks,
2013-09-23 Paul Pluzhnikov
* libstdc++-v3/include/bits/stl_deque.h (operator[], front,
back, pop_front, pop_back): Add conditional range checks.
Index: libstdc++-v3/include/bits/stl_deque.h
===
--- libstdc++-v3/include/bits/stl_deque.h (revision 202851)
+++ libstdc++-v3/include/bits/stl_deque.h (working copy)
@@ -1242,7 +1242,12 @@
*/
reference
operator[](size_type __n)
- { return this->_M_impl._M_start[difference_type(__n)]; }
+ {
+#if __google_stl_debug_deque
+ _M_range_check(__n);
+#endif
+ return this->_M_impl._M_start[difference_type(__n)];
+ }
/**
* @brief Subscript access to the data contained in the %deque.
@@ -1257,7 +1262,12 @@
*/
const_reference
operator[](size_type __n) const
- { return this->_M_impl._M_start[difference_type(__n)]; }
+ {
+#if __google_stl_debug_deque
+ _M_range_check(__n);
+#endif
+ return this->_M_impl._M_start[difference_type(__n)];
+ }
protected:
/// Safety check used only from at().
@@ -1311,7 +1321,12 @@
*/
reference
front()
- { return *begin(); }
+ {
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("front() on empty deque");
+#endif
+ return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -1319,7 +1334,12 @@
*/
const_reference
front() const
- { return *begin(); }
+ {
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("front() on empty deque");
+#endif
+ return *begin();
+ }
/**
* Returns a read/write reference to the data at the last element of the
@@ -1328,6 +1348,9 @@
reference
back()
{
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("back() on empty deque");
+#endif
iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1340,6 +1363,9 @@
const_reference
back() const
{
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("back() on empty deque");
+#endif
const_iterator __tmp = end();
--__tmp;
return *__tmp;
@@ -1420,6 +1446,9 @@
void
pop_front()
{
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("pop_front() on empty deque");
+#endif
if (this->_M_impl._M_start._M_cur
!= this->_M_impl._M_start._M_last - 1)
{
@@ -1441,6 +1470,9 @@
void
pop_back()
{
+#if __google_stl_debug_deque
+ if (empty()) __throw_logic_error("pop_back() on empty deque");
+#endif
if (this->_M_impl._M_finish._M_cur
!= this->_M_impl._M_finish._M_first)
{
Re: [google integration,gcc-4_8] Additional lightweight debug checks for std::deque
On Mon, Sep 23, 2013 at 6:29 PM, Paul Pluzhnikov wrote:
> I've committed the patch below to google/integration (r202856) and
> gcc-4_8 (r202857) branches. Google ref: b/10872448.
I've committed r202880 to adjust line numbers for libstdc++ breakage
on google/gcc-4_8 branch.
Thanks,
2013-09-24 Paul Pluzhnikov
* testsuite/23_containers/deque/requirements/dr438/assign_neg.cc:
Adjust.
* testsuite/23_containers/deque/requirements/dr438/insert_neg.cc:
Adjust.
* testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc:
Likewise.
* testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc:
Likewise.
Index: testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
===
--- testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
(revision 202876)
+++ testsuite/23_containers/deque/requirements/dr438/assign_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1698 }
+// { dg-error "no matching" "" { target *-*-* } 1730 }
#include
Index: testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
===
--- testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
(revision 202876)
+++ testsuite/23_containers/deque/requirements/dr438/insert_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1782 }
+// { dg-error "no matching" "" { target *-*-* } 1814 }
#include
@@ -32,4 +32,3 @@
std::deque d;
d.insert(d.begin(), 10, 1);
}
-
Index: testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
===
--- testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
(revision 202876)
+++ testsuite/23_containers/deque/requirements/dr438/constructor_1_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1631 }
+// { dg-error "no matching" "" { target *-*-* } 1663 }
#include
Index: testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
===
--- testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
(revision 202876)
+++ testsuite/23_containers/deque/requirements/dr438/constructor_2_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1631 }
+// { dg-error "no matching" "" { target *-*-* } 1663 }
#include
#include
[google gcc-4_8] Applied partial backport of r202818, r202832 and r202836.
Greetings,
I committed partial backport of r202818, r202832 and r202836 to
google/gcc-4_8 branch as r202927.
2013-09-25 Paul Pluzhnikov
* libstdc++-v3/config/abi/pre/gnu.ver: Add
_ZSt24__throw_out_of_range_fmtPKcz
* libstdc++-v3/src/c++11/Makefile.am: Add snprintf_lite.
* libstdc++-v3/src/c++11/Makefile.in: Regenerate.
* libstdc++-v3/src/c++11/snprintf_lite.cc: New.
* libstdc++-v3/src/c++11/functexcept.cc (__throw_out_of_range_fmt): New.
Index: libstdc++-v3/config/abi/pre/gnu.ver
===
--- libstdc++-v3/config/abi/pre/gnu.ver (revision 202926)
+++ libstdc++-v3/config/abi/pre/gnu.ver (working copy)
@@ -1357,6 +1357,13 @@
} GLIBCXX_3.4.18;
+GLIBCXX_3.4.20 {
+
+ # std::__throw_out_of_range_fmt(char const*, ...)
+ _ZSt24__throw_out_of_range_fmtPKcz;
+
+} GLIBCXX_3.4.19;
+
# Symbols in the support library (libsupc++) have their own tag.
CXXABI_1.3 {
Index: libstdc++-v3/src/c++11/Makefile.am
===
--- libstdc++-v3/src/c++11/Makefile.am (revision 202926)
+++ libstdc++-v3/src/c++11/Makefile.am (working copy)
@@ -42,6 +42,7 @@
random.cc \
regex.cc \
shared_ptr.cc \
+ snprintf_lite.cc \
system_error.cc \
thread.cc
Index: libstdc++-v3/src/c++11/functexcept.cc
===
--- libstdc++-v3/src/c++11/functexcept.cc (revision 202926)
+++ libstdc++-v3/src/c++11/functexcept.cc (working copy)
@@ -31,6 +31,7 @@
#include
#include
#include
+#include
#ifdef _GLIBCXX_USE_NLS
# include
@@ -39,6 +40,12 @@
# define _(msgid) (msgid)
#endif
+namespace __gnu_cxx
+{
+ int __snprintf_lite(char *__buf, size_t __bufsize, const char *__fmt,
+ va_list __ap);
+}
+
namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -80,6 +87,22 @@
{ _GLIBCXX_THROW_OR_ABORT(out_of_range(_(__s))); }
void
+ __throw_out_of_range_fmt(const char* __fmt, ...)
+ {
+const size_t __len = __builtin_strlen(__fmt);
+// We expect at most 2 numbers, and 1 short string. The additional
+// 512 bytes should provide more than enough space for expansion.
+const size_t __alloca_size = __len + 512;
+char *const __s = static_cast(__builtin_alloca(__alloca_size));
+va_list __ap;
+
+va_start(__ap, __fmt);
+__gnu_cxx::__snprintf_lite(__s, __alloca_size, __fmt, __ap);
+_GLIBCXX_THROW_OR_ABORT(out_of_range(_(__s)));
+va_end(__ap); // Not reached.
+ }
+
+ void
__throw_runtime_error(const char* __s __attribute__((unused)))
{ _GLIBCXX_THROW_OR_ABORT(runtime_error(_(__s))); }
Index: libstdc++-v3/src/c++11/Makefile.in
===
--- libstdc++-v3/src/c++11/Makefile.in (revision 202926)
+++ libstdc++-v3/src/c++11/Makefile.in (working copy)
@@ -70,7 +70,8 @@
am__objects_1 = chrono.lo condition_variable.lo debug.lo \
functexcept.lo functional.lo future.lo hash_c++0x.lo \
hashtable_c++0x.lo limits.lo mutex.lo placeholders.lo \
- random.lo regex.lo shared_ptr.lo system_error.lo thread.lo
+ random.lo regex.lo shared_ptr.lo snprintf_lite.lo \
+ system_error.lo thread.lo
@ENABLE_EXTERN_TEMPLATE_TRUE@am__objects_2 = fstream-inst.lo \
@ENABLE_EXTERN_TEMPLATE_TRUE@ string-inst.lo wstring-inst.lo
am_libc__11convenience_la_OBJECTS = $(am__objects_1) $(am__objects_2)
@@ -319,6 +320,7 @@
random.cc \
regex.cc \
shared_ptr.cc \
+ snprintf_lite.cc \
system_error.cc \
thread.cc
Index: libstdc++-v3/src/c++11/snprintf_lite.cc
===
--- libstdc++-v3/src/c++11/snprintf_lite.cc (revision 0)
+++ libstdc++-v3/src/c++11/snprintf_lite.cc (revision 0)
@@ -0,0 +1,159 @@
+// Debugging support -*- C++ -*-
+
+// Copyright (C) 2013 Free Software Foundation, Inc.
+//
+// This file is part of GCC.
+//
+// GCC 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.
+//
+// GCC 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.
+//
+// Under Section 7 of GPL version 3, you are granted additional
+// permissions described in the GCC Runtime Library Exception, version
+// 3.1, as published by the Free Software Foundation.
+
+// You should have received a copy of the GNU General Public License and
+// a copy of the GCC Runtime Library Exception along with this program;
+// see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
[google gcc-4_8,integration][patch] Set abi-version to 0
Greetings, For Google b/10860844, I've committed the patch below on google gcc-4_8 (r203381) and integration (r203382) branches. This forces abi-version to be the latest available. 2013-10-10 Paul Pluizhnikov * gcc/common.opt (flag_abi_version): Set to 0. Index: gcc/common.opt === --- gcc/common.opt (revision 203380) +++ gcc/common.opt (working copy) @@ -866,7 +866,7 @@ ; Additional positive integers will be assigned as new versions of ; the ABI become the default version of the ABI. fabi-version= -Common Joined RejectNegative UInteger Var(flag_abi_version) Init(2) +Common Joined RejectNegative UInteger Var(flag_abi_version) Init(0) faggressive-loop-optimizations Common Report Var(flag_aggressive_loop_optimizations) Optimization Init(1)
Re: [patch] Make cxxfilt demangle internal-linkage templates
Ian? Ping x4. On Wed, Sep 18, 2013 at 4:24 PM, Paolo Carlini wrote: >>> Original message: >>> http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html >> >> > I'm adding Ian in CC. -- Paul Pluzhnikov
[Google][gcc-4_8] Completed backport of r202818 from trunk to google/gcc-4_8 as r206071.
Greetings,
I've finished backport of r202818 from trunk to google/gcc-4_8 as r206071.
Thanks,
--
Paul Pluzhnikov
Index: libstdc++-v3/include/debug/array
===
--- libstdc++-v3/include/debug/array(revision 206038)
+++ libstdc++-v3/include/debug/array(working copy)
@@ -165,7 +165,10 @@
at(size_type __n)
{
if (__n >= _Nm)
- std::__throw_out_of_range(__N("array::at"));
+ std::__throw_out_of_range_fmt(__N("array::at: __n "
+ "(which is %zu) >= _Nm "
+ "(which is %zu)"),
+ __n, _Nm);
return _AT_Type::_S_ref(_M_elems, __n);
}
@@ -175,7 +178,9 @@
// Result of conditional expression must be an lvalue so use
// boolean ? lvalue : (throw-expr, lvalue)
return __n < _Nm ? _AT_Type::_S_ref(_M_elems, __n)
- : (std::__throw_out_of_range(__N("array::at")),
+ : (std::__throw_out_of_range_fmt(__N("array::at: __n (which is %zu) "
+ ">= _Nm (which is %zu)"),
+ __n, _Nm),
_AT_Type::_S_ref(_M_elems, 0));
}
Index: libstdc++-v3/include/std/bitset
===
--- libstdc++-v3/include/std/bitset (revision 206038)
+++ libstdc++-v3/include/std/bitset (working copy)
@@ -752,7 +752,27 @@
typedef _Base_bitset<_GLIBCXX_BITSET_WORDS(_Nb)> _Base;
typedef unsigned long _WordT;
+ template
void
+ _M_check_initial_position(const std::basic_string<_CharT, _Traits,
_Alloc>& __s,
+ size_t __position) const
+ {
+ if (__position > __s.size())
+ __throw_out_of_range_fmt(__N("bitset::bitset: __position "
+ "(which is %zu) > __s.size() "
+ "(which is %zu)"),
+ __position, __s.size());
+ }
+
+ void _M_check(size_t __position, const char *__s) const
+ {
+ if (__position >= _Nb)
+ __throw_out_of_range_fmt(__N("%s: __position (which is %zu) "
+ ">= _Nb (which is %zu)"),
+ __s, __position, _Nb);
+ }
+
+ void
_M_do_sanitize() _GLIBCXX_NOEXCEPT
{
typedef _Sanitize<_Nb % _GLIBCXX_BITSET_BITS_PER_WORD> __sanitize_type;
@@ -867,9 +887,7 @@
size_t __position = 0)
: _Base()
{
- if (__position > __s.size())
- __throw_out_of_range(__N("bitset::bitset initial position "
-"not valid"));
+ _M_check_initial_position(__s, __position);
_M_copy_from_string(__s, __position,
std::basic_string<_CharT, _Traits, _Alloc>::npos,
_CharT('0'), _CharT('1'));
@@ -890,9 +908,7 @@
size_t __position, size_t __n)
: _Base()
{
- if (__position > __s.size())
- __throw_out_of_range(__N("bitset::bitset initial position "
-"not valid"));
+ _M_check_initial_position(__s, __position);
_M_copy_from_string(__s, __position, __n, _CharT('0'), _CharT('1'));
}
@@ -904,9 +920,7 @@
_CharT __zero, _CharT __one = _CharT('1'))
: _Base()
{
- if (__position > __s.size())
- __throw_out_of_range(__N("bitset::bitset initial position "
-"not valid"));
+ _M_check_initial_position(__s, __position);
_M_copy_from_string(__s, __position, __n, __zero, __one);
}
@@ -1067,8 +1081,7 @@
bitset<_Nb>&
set(size_t __position, bool __val = true)
{
- if (__position >= _Nb)
- __throw_out_of_range(__N("bitset::set"));
+ this->_M_check(__position, __N("bitset::set"));
return _Unchecked_set(__position, __val);
}
@@ -1092,8 +1105,7 @@
bitset<_Nb>&
reset(size_t __position)
{
- if (__position >= _Nb)
- __throw_out_of_range(__N("bitset::reset"));
+ this->_M_check(__position, __N("bitset::reset"));
return _Unchecked_reset(__position);
}
@@ -1116,8 +1128,7 @@
bitset<_Nb>&
flip(size_t __position)
{
- if (__position >= _Nb)
- __throw_out_of_range(__N("
[google][4.8] Add more inexpensive debug checks to vector, bitvector, deque
Greetings,
For Google b/9127283, I've committed attached patch on google/gcc-4_8 branch.
Related: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56109
This caught ~10 bugs for us. erase(end()) and pop_back() on empty
vector appear to be most common.
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 206330)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -1050,6 +1050,10 @@
void
pop_back()
{
+#if __google_stl_debug_vector
+ if (this->empty())
+ __throw_logic_error(__N("pop_back() on empty vector"));
+#endif
--this->_M_impl._M_finish;
_Alloc_traits::destroy(this->_M_impl, this->_M_impl._M_finish);
}
@@ -1135,7 +1139,13 @@
*/
void
insert(iterator __position, size_type __n, const value_type& __x)
- { _M_fill_insert(__position, __n, __x); }
+ {
+#if __google_stl_debug_vector
+ if (__position < this->begin() || __position > this->end())
+ __throw_out_of_range(__N("insert() at invalid position"));
+#endif
+ _M_fill_insert(__position, __n, __x);
+ }
/**
* @brief Inserts a range into the %vector.
@@ -1157,13 +1167,23 @@
void
insert(iterator __position, _InputIterator __first,
_InputIterator __last)
-{ _M_insert_dispatch(__position, __first, __last, __false_type()); }
+{
+#if __google_stl_debug_vector
+ if (__position < this->begin() || __position > this->end())
+ __throw_out_of_range(__N("insert() at invalid position"));
+#endif
+ _M_insert_dispatch(__position, __first, __last, __false_type());
+ }
#else
template
void
insert(iterator __position, _InputIterator __first,
_InputIterator __last)
{
+#if __google_stl_debug_vector
+ if (__position < this->begin() || __position > this->end())
+ __throw_out_of_range(__N("insert() at invalid position"));
+#endif
// Check whether it's an integral type. If so, it's not an iterator.
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_insert_dispatch(__position, __first, __last, _Integral());
Index: libstdc++-v3/include/bits/stl_deque.h
===
--- libstdc++-v3/include/bits/stl_deque.h (revision 206330)
+++ libstdc++-v3/include/bits/stl_deque.h (working copy)
@@ -1552,7 +1552,13 @@
*/
void
insert(iterator __position, size_type __n, const value_type& __x)
- { _M_fill_insert(__position, __n, __x); }
+ {
+#if __google_stl_debug_deque
+ if (__position < this->begin() || __position > this->end())
+ __throw_logic_error("insert() at invalid position");
+#endif
+ _M_fill_insert(__position, __n, __x);
+ }
/**
* @brief Inserts a range into the %deque.
@@ -1570,7 +1576,13 @@
void
insert(iterator __position, _InputIterator __first,
_InputIterator __last)
-{ _M_insert_dispatch(__position, __first, __last, __false_type()); }
+{
+#if __google_stl_debug_deque
+ if (__position < this->begin() || __position > this->end())
+ __throw_logic_error("insert() at invalid position");
+#endif
+ _M_insert_dispatch(__position, __first, __last, __false_type());
+ }
#else
template
void
Index: libstdc++-v3/include/bits/vector.tcc
===
--- libstdc++-v3/include/bits/vector.tcc(revision 206330)
+++ libstdc++-v3/include/bits/vector.tcc(working copy)
@@ -107,6 +107,10 @@
vector<_Tp, _Alloc>::
insert(iterator __position, const value_type& __x)
{
+#if __google_stl_debug_vector
+ if (__position < this->begin() || __position > this->end())
+ __throw_out_of_range(__N("insert() at invalid position"));
+#endif
const size_type __n = __position - begin();
if (this->_M_impl._M_finish != this->_M_impl._M_end_of_storage
&& __position == end())
@@ -134,6 +138,10 @@
vector<_Tp, _Alloc>::
erase(iterator __position)
{
+#if __google_stl_debug_vector
+ if (__position < this->begin() || __position >= this->end())
+ __throw_out_of_range(__N("erase() at invalid position"));
+#endif
if (__position + 1 != end())
_GLIBCXX_MOVE3(__position + 1, end(), __position);
--this->_M_impl._M_finish;
@@ -146,6 +154,10 @@
vector<_Tp, _Alloc>::
erase(iterator __first, iterator __last)
{
+#if __goo
[google][gcc-4.9][committed] Add inexpensive bounds checks to std::array
Greetings,
For google b/9650176, attached patch adds bounds checks to std::array
and updates expected line numbers in tests.
This is similar to the checks that we do for std::vector.
See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56109
Committed as r226465.
Thanks,
--
Paul Pluzhnikov
Index: libstdc++-v3/include/std/array
===
--- libstdc++-v3/include/std/array (revision 226462)
+++ libstdc++-v3/include/std/array (working copy)
@@ -50,7 +50,18 @@
static constexpr _Tp*
_S_ptr(const _Type& __t, std::size_t __n) noexcept
+#if __google_stl_debug_array
+ {
+ return __n < _Nm
+ ? const_cast<_Tp*>(std::__addressof(__t[__n]))
+ : (std::__throw_out_of_range_fmt(__N("array::_S_ptr: __n "
+ "(which is %zu) >= size() "
+ "(which is %zu)"),
+ __n, _Nm), nullptr);
+ }
+#else
{ return const_cast<_Tp*>(std::__addressof(__t[__n])); }
+#endif
};
template
Index: libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
===
--- libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
(revision 226462)
+++ libstdc++-v3/testsuite/23_containers/array/tuple_interface/get_neg.cc
(working copy)
@@ -28,6 +28,6 @@
int n2 = std::get<1>(std::move(a));
int n3 = std::get<1>(ca);
-// { dg-error "static assertion failed" "" { target *-*-* } 274 }
-// { dg-error "static assertion failed" "" { target *-*-* } 283 }
-// { dg-error "static assertion failed" "" { target *-*-* } 291 }
+// { dg-error "static assertion failed" "" { target *-*-* } 285 }
+// { dg-error "static assertion failed" "" { target *-*-* } 294 }
+// { dg-error "static assertion failed" "" { target *-*-* } 302 }
Index:
libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
===
---
libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
(revision 226462)
+++
libstdc++-v3/testsuite/23_containers/array/tuple_interface/tuple_element_neg.cc
(working copy)
@@ -23,4 +23,4 @@
typedef std::tuple_element<1, std::array>::type type;
-// { dg-error "static assertion failed" "" { target *-*-* } 320 }
+// { dg-error "static assertion failed" "" { target *-*-* } 331 }
[google gcc-4_8] Reverted r203873 and r203036
Greetings, I've reverted r203873 and r203036 in r204860 on google/gcc-4_8 branch. The r203036 changes output of std::sort when input has equivalent elements, and this breaks "golden output" tests, which we'll need to clean up. Thanks, -- Paul Pluzhnikov
Ok to backport r210653 (fix for PR58930) to gcc-4_9-branch?
Greetings, On Fri, Jun 27, 2014 at 2:18 AM, paolo.carlini at oracle dot com wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58930 > > --- Comment #6 from Paolo Carlini --- > The patch isn't trivial but I understand that it fixes quite a few issues. > Thus > certainly no objections from me. Just ask on the mailing list: if Jason > agrees, > please go ahead. Thanks. Ok to backport r210653 (fix for PR58930) to gcc-4_9-branch? (If not, I'll backport to google/gcc-4_9 instead.) Thanks, -- Paul Pluzhnikov
Re: Ok to backport r210653 (fix for PR58930) to gcc-4_9-branch?
On Fri, Jun 27, 2014 at 12:51 PM, Jason Merrill wrote:
> On 06/27/2014 11:04 AM, Paul Pluzhnikov wrote:
>>
>> Ok to backport r210653 (fix for PR58930) to gcc-4_9-branch?
>
>
> OK, yes.
Thanks. Committed attached patch as r212207.
Tested on Linux/x86_64, no regressions.
--
Paul Pluzhnikov
Index: gcc/testsuite/g++.dg/cpp0x/nsdmi-template11.C
===
--- gcc/testsuite/g++.dg/cpp0x/nsdmi-template11.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nsdmi-template11.C (revision 212207)
@@ -0,0 +1,30 @@
+// PR c++/58930
+// { dg-do compile { target c++11 } }
+
+struct SampleModule
+{
+ explicit SampleModule (int);
+};
+
+template < typename >
+struct BaseHandler
+{
+ SampleModule module_ { 0 };
+};
+
+BaseHandler a;
+// PR c++/58930
+// { dg-do compile { target c++11 } }
+
+struct SampleModule
+{
+ explicit SampleModule (int);
+};
+
+template < typename >
+struct BaseHandler
+{
+ SampleModule module_ { 0 };
+};
+
+BaseHandler a;
Index: gcc/testsuite/g++.dg/cpp0x/nsdmi-template13.C
===
--- gcc/testsuite/g++.dg/cpp0x/nsdmi-template13.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nsdmi-template13.C (revision 212207)
@@ -0,0 +1,22 @@
+// PR c++/58704
+// { dg-do compile { target c++11 } }
+
+struct A {};
+
+template struct B
+{
+ A a[1] = { };
+};
+
+B b;
+// PR c++/58704
+// { dg-do compile { target c++11 } }
+
+struct A {};
+
+template struct B
+{
+ A a[1] = { };
+};
+
+B b;
Index: gcc/testsuite/g++.dg/cpp0x/nsdmi-template12.C
===
--- gcc/testsuite/g++.dg/cpp0x/nsdmi-template12.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/nsdmi-template12.C (revision 212207)
@@ -0,0 +1,34 @@
+// PR c++/58753
+// { dg-do compile { target c++11 } }
+
+#include
+
+template
+struct X {X(std::initializer_list) {}};
+
+template
+class T {
+ X x{1};
+};
+
+int main()
+{
+ T t;
+}
+// PR c++/58753
+// { dg-do compile { target c++11 } }
+
+#include
+
+template
+struct X {X(std::initializer_list) {}};
+
+template
+class T {
+ X x{1};
+};
+
+int main()
+{
+ T t;
+}
Index: gcc/cp/init.c
===
--- gcc/cp/init.c (revision 212206)
+++ gcc/cp/init.c (revision 212207)
@@ -522,6 +522,49 @@
}
}
+/* Return the non-static data initializer for FIELD_DECL MEMBER. */
+
+tree
+get_nsdmi (tree member, bool in_ctor)
+{
+ tree init;
+ tree save_ccp = current_class_ptr;
+ tree save_ccr = current_class_ref;
+ if (!in_ctor)
+inject_this_parameter (DECL_CONTEXT (member), TYPE_UNQUALIFIED);
+ if (DECL_LANG_SPECIFIC (member) && DECL_TEMPLATE_INFO (member))
+{
+ /* Do deferred instantiation of the NSDMI. */
+ init = (tsubst_copy_and_build
+ (DECL_INITIAL (DECL_TI_TEMPLATE (member)),
+ DECL_TI_ARGS (member),
+ tf_warning_or_error, member, /*function_p=*/false,
+ /*integral_constant_expression_p=*/false));
+
+ init = digest_nsdmi_init (member, init);
+}
+ else
+{
+ init = DECL_INITIAL (member);
+ if (init && TREE_CODE (init) == DEFAULT_ARG)
+ {
+ error ("constructor required before non-static data member "
+"for %qD has been parsed", member);
+ DECL_INITIAL (member) = error_mark_node;
+ init = NULL_TREE;
+ }
+ /* Strip redundant TARGET_EXPR so we don't need to remap it, and
+so the aggregate init code below will see a CONSTRUCTOR. */
+ if (init && TREE_CODE (init) == TARGET_EXPR
+ && !VOID_TYPE_P (TREE_TYPE (TARGET_EXPR_INITIAL (init
+ init = TARGET_EXPR_INITIAL (init);
+ init = break_out_target_exprs (init);
+}
+ current_class_ptr = save_ccp;
+ current_class_ref = save_ccr;
+ return init;
+}
+
/* Initialize MEMBER, a FIELD_DECL, with INIT, a TREE_LIST of
arguments. If TREE_LIST is void_type_node, an empty initializer
list was given; if NULL_TREE no initializer was given. */
@@ -535,31 +578,7 @@
/* Use the non-static data member initializer if there was no
mem-initializer for this field. */
if (init == NULL_TREE)
-{
- if (DECL_LANG_SPECIFIC (member) && DECL_TEMPLATE_INFO (member))
- /* Do deferred instantiation of the NSDMI. */
- init = (tsubst_copy_and_build
- (DECL_INITIAL (DECL_TI_TEMPLATE (member)),
-DECL_TI_ARGS (member),
-tf_warning_or_error, member, /*function_p=*/false,
-/*integral_constant_expression_p=*/false));
- else
- {
- init = DECL_INITIAL (member);
- if (init && TREE_CODE (init) == DEFAULT_ARG)
- {
- error ("constructor required befor
Re: Ok to backport r210653 (fix for PR58930) to gcc-4_9-branch?
On Tue, Jul 1, 2014 at 12:06 PM, Paolo Carlini wrote: > If this is what you actually committed Yes. > something went wrong with the testcases... Thanks. The patch applied twice, causing doubling of the test :-( Now I have to see how that didn't cause new failures ... In the mean time, r212208 deleted the doubling. Thanks, -- Paul Pluzhnikov
[patch][google/gcc-4_7] Extend expiration date for pr54127 (issue6817091)
Greetings, This patch is for google/gcc-4_7 branch. Thanks, 2012-11-05 Paul Pluzhnikov * contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail: extend expiration date for pr54127. Index: contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail === --- contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail (revision 193176) +++ contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail (working copy) @@ -1,9 +1,9 @@ # Temporarily ignore gcc pr54127. -expire=20121031 | FAIL: gcc.dg/torture/pr53589.c -O3 -g (test for excess errors) -expire=20121031 | FAIL: gcc.dg/torture/pr53589.c -O3 -g (internal compiler error) +expire=20121231 | FAIL: gcc.dg/torture/pr53589.c -O3 -g (test for excess errors) +expire=20121231 | FAIL: gcc.dg/torture/pr53589.c -O3 -g (internal compiler error) # Temporarily ignore Google ref b/6983319. -expire=20121031 | FAIL: gcc.target/powerpc/regnames-1.c (test for excess errors) -expire=20121031 | FAIL: gcc.target/powerpc/regnames-1.c (internal compiler error) +expire=20121231 | FAIL: gcc.target/powerpc/regnames-1.c (test for excess errors) +expire=20121231 | FAIL: gcc.target/powerpc/regnames-1.c (internal compiler error) FAIL: gfortran.dg/bessel_6.f90 -O0 execution test FAIL: gfortran.dg/bessel_6.f90 -O1 execution test -- This patch is available for review at http://codereview.appspot.com/6817091
[google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector
Greetings,
This patch adds (relatively) cheap bounds and dangling checks to
vector, similar to the checks I added to vector in r195373,
r195356, etc.
Ok for google branches (gcc-4_7, gcc-4_8, integration) ?
Thanks,
--
Index: libstdc++-v3/include/bits/stl_bvector.h
===
--- libstdc++-v3/include/bits/stl_bvector.h (revision 199261)
+++ libstdc++-v3/include/bits/stl_bvector.h (working copy)
@@ -438,11 +438,31 @@
#endif
~_Bvector_base()
- { this->_M_deallocate(); }
+ {
+this->_M_deallocate();
+#if __google_stl_debug_bvector
+__builtin_memset(this, 0xcd, sizeof(*this));
+#endif
+ }
protected:
_Bvector_impl _M_impl;
+#if __google_stl_debug_bvector
+ bool _M_is_valid() const
+ {
+ return (this->_M_impl._M_start._M_p == 0
+ && this->_M_impl._M_finish._M_p == 0
+ && this->_M_impl._M_end_of_storage == 0)
+ || (this->_M_impl._M_start._M_p <= this->_M_impl._M_finish._M_p
+ && this->_M_impl._M_finish._M_p <= this->_M_impl._M_end_of_storage
+ && (this->_M_impl._M_start._M_p < this->_M_impl._M_end_of_storage
+ || (this->_M_impl._M_start._M_p ==
this->_M_impl._M_end_of_storage
+ && this->_M_impl._M_start._M_offset == 0
+ && this->_M_impl._M_finish._M_offset == 0)));
+ }
+#endif
+
_Bit_type*
_M_allocate(size_t __n)
{ return _M_impl.allocate(_S_nword(__n)); }
@@ -571,6 +591,10 @@
vector&
operator=(const vector& __x)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
if (&__x == this)
return *this;
if (__x.size() > capacity())
@@ -587,6 +611,10 @@
vector&
operator=(vector&& __x)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
// NB: DR 1204.
// NB: DR 675.
this->clear();
@@ -608,12 +636,22 @@
// or not the type is an integer.
void
assign(size_type __n, const bool& __x)
-{ _M_fill_assign(__n, __x); }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("assign on corrupt (dangling?) vector");
+#endif
+ _M_fill_assign(__n, __x);
+}
template
void
assign(_InputIterator __first, _InputIterator __last)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("assign() on corrupt (dangling?) vector");
+#endif
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_assign_dispatch(__first, __last, _Integral());
}
@@ -626,19 +664,43 @@
iterator
begin() _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
const_iterator
begin() const _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
iterator
end() _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_finish; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_finish;
+}
const_iterator
end() const _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_finish; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_finish;
+}
reverse_iterator
rbegin() _GLIBCXX_NOEXCEPT
@@ -659,11 +721,23 @@
#ifdef __GXX_EXPERIMENTAL_CXX0X__
const_iterator
cbegin() const noexcept
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("cbegin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
const_iterator
cend() const noexcept
-{ return this->_M_impl._M_finish; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("cend() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_finish;
+}
const_reverse_iterator
crbegin() const noexcept
@@ -681,6 +755,10 @@
size_type
max_size() const _GLIBCXX_NOEXCEPT
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("max_size() on corrupt (dangling?) vector");
+
Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector
+cc libstd...@gcc.gnu.org
On Thu, May 23, 2013 at 8:51 AM, Paul Pluzhnikov wrote:
> Greetings,
>
> This patch adds (relatively) cheap bounds and dangling checks to
> vector, similar to the checks I added to vector in r195373,
> r195356, etc.
>
> Ok for google branches (gcc-4_7, gcc-4_8, integration) ?
>
> Thanks,
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_bvector.h
===
--- libstdc++-v3/include/bits/stl_bvector.h (revision 199261)
+++ libstdc++-v3/include/bits/stl_bvector.h (working copy)
@@ -438,11 +438,31 @@
#endif
~_Bvector_base()
- { this->_M_deallocate(); }
+ {
+this->_M_deallocate();
+#if __google_stl_debug_bvector
+__builtin_memset(this, 0xcd, sizeof(*this));
+#endif
+ }
protected:
_Bvector_impl _M_impl;
+#if __google_stl_debug_bvector
+ bool _M_is_valid() const
+ {
+ return (this->_M_impl._M_start._M_p == 0
+ && this->_M_impl._M_finish._M_p == 0
+ && this->_M_impl._M_end_of_storage == 0)
+ || (this->_M_impl._M_start._M_p <= this->_M_impl._M_finish._M_p
+ && this->_M_impl._M_finish._M_p <= this->_M_impl._M_end_of_storage
+ && (this->_M_impl._M_start._M_p < this->_M_impl._M_end_of_storage
+ || (this->_M_impl._M_start._M_p ==
this->_M_impl._M_end_of_storage
+ && this->_M_impl._M_start._M_offset == 0
+ && this->_M_impl._M_finish._M_offset == 0)));
+ }
+#endif
+
_Bit_type*
_M_allocate(size_t __n)
{ return _M_impl.allocate(_S_nword(__n)); }
@@ -571,6 +591,10 @@
vector&
operator=(const vector& __x)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
if (&__x == this)
return *this;
if (__x.size() > capacity())
@@ -587,6 +611,10 @@
vector&
operator=(vector&& __x)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("op=() on corrupt (dangling?) vector");
+#endif
// NB: DR 1204.
// NB: DR 675.
this->clear();
@@ -608,12 +636,22 @@
// or not the type is an integer.
void
assign(size_type __n, const bool& __x)
-{ _M_fill_assign(__n, __x); }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("assign on corrupt (dangling?) vector");
+#endif
+ _M_fill_assign(__n, __x);
+}
template
void
assign(_InputIterator __first, _InputIterator __last)
{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("assign() on corrupt (dangling?) vector");
+#endif
typedef typename std::__is_integer<_InputIterator>::__type _Integral;
_M_assign_dispatch(__first, __last, _Integral());
}
@@ -626,19 +664,43 @@
iterator
begin() _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
const_iterator
begin() const _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
iterator
end() _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_finish; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_finish;
+}
const_iterator
end() const _GLIBCXX_NOEXCEPT
-{ return this->_M_impl._M_finish; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_finish;
+}
reverse_iterator
rbegin() _GLIBCXX_NOEXCEPT
@@ -659,11 +721,23 @@
#ifdef __GXX_EXPERIMENTAL_CXX0X__
const_iterator
cbegin() const noexcept
-{ return this->_M_impl._M_start; }
+{
+#if __google_stl_debug_bvector
+ if (!this->_M_is_valid())
+ __throw_logic_error("cbegin() on corrupt (dangling?) vector");
+#endif
+ return this->_M_impl._M_start;
+}
const_iterator
cend() const noexcept
-{ return this->_M_impl._M_finish; }
+{
+#if __google_st
Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector
On Thu, May 23, 2013 at 9:14 AM, Jonathan Wakely wrote: > I was wondering the other day whether we should put these checks on > trunk and enable them automatically when !defined(__OPTIMIZE__) FWIW, we keep this under a separate macro so we can turn it on or off independent of other build options. Our current code looks like this: #if !defined(__google_stl_debug_vector) # if !defined(NDEBUG) # define __google_stl_debug_vector 1 # endif #endif Keying off NDEBUG rather than __OPTIMIZE__ seems like a more consistent approach -- if you want assert()s, then you probably also want these checks. -- Paul Pluzhnikov
Re: [google gcc-4_7,gcc-4_8,integration] Add bounds checks to vector
Jonathan, On Thu, May 23, 2013 at 10:13 AM, Paul Pluzhnikov wrote: > On Thu, May 23, 2013 at 9:14 AM, Jonathan Wakely > wrote: > >> I was wondering the other day whether we should put these checks on >> trunk and enable them automatically when !defined(__OPTIMIZE__) > > FWIW, we keep this under a separate macro so we can turn it on or off > independent of other build options. > > Our current code looks like this: > > #if !defined(__google_stl_debug_vector) > # if !defined(NDEBUG) > # define __google_stl_debug_vector 1 > # endif > #endif > > > Keying off NDEBUG rather than __OPTIMIZE__ seems like a more > consistent approach -- if you want assert()s, then you probably also > want these checks. I'll be happy to try to push these changes into trunk, if you'd like me to. The question is what macro(s) should guard the checks, and under what conditions should they be turned on. BTW, this is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56109 Thanks, -- Paul Pluzhnikov
[google gcc-4_7,gcc-4_8,integration] Relax vector validity checks
Greetings,
The vector validity checks introduced here:
http://gcc.gnu.org/ml/gcc-patches/2011-09/msg00415.html
proved too strict: older versions of GCC used to do this:
_M_start = _M_end = _M_end_of_storage = new_allocator(sizeof(T) * n)
even when n == 0, and we have code compiled by such version linked in
with new code, and failing the check.
Google ref b/9198806
Attached patch relaxes the check, while still catching dangling vector
accesses.
Ok for google/gcc_4-7, gcc-4_8 and integration branches?
Thanks,
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 199461)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -244,12 +244,27 @@
bool _M_is_valid() const
{
-return (this->_M_impl._M_end_of_storage == 0
- && this->_M_impl._M_start == 0
- && this->_M_impl._M_finish == 0)
- || (this->_M_impl._M_start <= this->_M_impl._M_finish
- && this->_M_impl._M_finish <= this->_M_impl._M_end_of_storage
- && this->_M_impl._M_start < this->_M_impl._M_end_of_storage);
+if (this->_M_impl._M_end_of_storage == 0
+ && this->_M_impl._M_start == 0
+ && this->_M_impl._M_finish == 0)
+ return true;
+
+ if (this->_M_impl._M_start <= this->_M_impl._M_finish
+ && this->_M_impl._M_finish <= this->_M_impl._M_end_of_storage)
+ {
+ if (this->_M_impl._M_start < this->_M_impl._M_end_of_storage)
+ return true;
+ else if (this->_M_impl._M_start == this->_M_impl._M_end_of_storage
+&& this->_M_impl._M_start == this->_M_impl._M_finish)
+ {
+ pointer _0xcdcd;
+
+ __builtin_memset(&_0xcdcd, 0xcd, sizeof(_0xcdcd));
+ return this->_M_impl._M_finish != _0xcdcd;
+ }
+ }
+
+ return false;
}
public:
Re: [google gcc-4_7,gcc-4_8,integration] Relax vector validity checks
On Thu, May 30, 2013 at 6:48 PM, Diego Novillo wrote: > OK. Is this applicable to trunk and/or release branches? No: the "cheap" vector and string checks are on google branches only. -- Paul Pluzhnikov
[google gcc-4_8 commited] Adjust testsuite line numbers for r199468.
I've committed attached patch on google/gcc-4_8 branch to fix testsuite
failures broken by r199468.
Thanks,
--
Paul Pluzhnikov
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
===
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
(revision 199470)
+++
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1346 }
+// { dg-error "no matching" "" { target *-*-* } 1361 }
#include
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
===
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
(revision 199470)
+++
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1387 }
+// { dg-error "no matching" "" { target *-*-* } 1402 }
#include
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
===
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
(revision 199470)
+++
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1272 }
+// { dg-error "no matching" "" { target *-*-* } 1287 }
#include
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
===
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
(revision 199470)
+++
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
(working copy)
@@ -18,7 +18,7 @@
// <http://www.gnu.org/licenses/>.
// { dg-do compile }
-// { dg-error "no matching" "" { target *-*-* } 1272 }
+// { dg-error "no matching" "" { target *-*-* } 1287 }
#include
#include
Re: [google gcc-4_8 commited] Adjust testsuite line numbers for r199468.
On Thu, May 30, 2013 at 10:00 PM, Paul Pluzhnikov wrote: > I've committed attached patch on google/gcc-4_8 branch to fix testsuite > failures broken by r199468. Also applied to google/integration branch. -- Paul Pluzhnikov
[google/4_8] Merged r200809 from gcc-4_8-branch to google/gcc-4_8
-- Paul Pluzhnikov
[google gcc-4_8] Merge r201068 (fix for PR57878) from gcc-4_8-branch
Greetings, I committed merge of r201068 (backport of the fix for PR57878 from trunk) as r201071 to google/gcc-4_8 branch. Thanks, -- Paul Pluzhnikov
[patch] Make cxxfilt demangle internal-linkage templates
Greetings,
I've been redirected here from binutils:
http://sourceware.org/ml/binutils/2013-08/msg00052.html
http://sourceware.org/ml/binutils/2013-08/msg00056.html
The following source:
template static void f();
void g() { f(); }
results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.
Richard Smith says:
The ABI doesn't cover manglings for local symbols ...
... and c++filt is not able to cope with the L prefix here.
I'm having a hard time seeing how this isn't a g++ bug and a matching
c++filt bug.
It's hard for me to argue that this is a 'g++' bug (since there is no
ABI violation here), but c++filt should be able to handle this, and does
with attached patch.
Ok for trunk?
Thanks,
Google ref: b/10137049
---
Paul Pluzhnikov
2013-08-07 Paul Pluzhnikov
* cp-demangle.c (d_name): Handle internal-linkage templates.
* testsuite/demangle-expected: New case.
Index: libiberty/testsuite/demangle-expected
===
--- libiberty/testsuite/demangle-expected (revision 201577)
+++ libiberty/testsuite/demangle-expected (working copy)
@@ -4291,3 +4291,6 @@
--format=gnu-v3
_Z1nIM1AKFvvREEvT_
void n(void (A::*)() const &)
+--format=gnu-v3
+_ZL1fIiEvv
+void f()
Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c (revision 201577)
+++ libiberty/cp-demangle.c (working copy)
@@ -1276,7 +1276,6 @@
case 'Z':
return d_local_name (di);
-case 'L':
case 'U':
return d_unqualified_name (di);
@@ -1323,6 +1322,7 @@
return dc;
}
+case 'L':
default:
dc = d_unqualified_name (di);
if (d_peek_char (di) == 'I')
Re: [patch] Make cxxfilt demangle internal-linkage templates
On Wed, Aug 7, 2013 at 11:34 AM, Andrew Pinski wrote: > I think this should also be send to the GCC List as libiberty is > officially maintained by GCC. http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00394.html Thanks, -- Paul Pluzhnikov
[patch google/gcc-4_8] Applied r201755 to google/gcc-4_8 branch
Greetings, I've committed r201755 on google/gcc-4_8 branch as r201761.
Re: [patch] Make cxxfilt demangle internal-linkage templates
Ping?
On Wed, Aug 7, 2013 at 11:51 AM, Paul Pluzhnikov wrote:
> The following source:
>
> template static void f();
> void g() { f(); }
>
> results in "_Z1fIiEvv" under g++, but in "_ZL1fIiEvv" under clang.
>
> Richard Smith says:
>
> The ABI doesn't cover manglings for local symbols ...
> ... and c++filt is not able to cope with the L prefix here.
>
> I'm having a hard time seeing how this isn't a g++ bug and a matching
> c++filt bug.
--
Paul Pluzhnikov
[google gcc-4_7, integration] Build more of libstdc++ with frame pointers
Greetings, Google ref b/8187733 Build libstdc++-v3/src/c++11/debug.cc with -fno-omit-frame-pointer, so frame-based unwinder can step through it. Tested: bootstrap build and verified debug.cc is built with -fno-omit-frame-pointer. Ok for google/gcc-4_7 and google/integration? Thanks, -- Paul Pluzhnikov Index: libstdc++-v3/src/c++11/Makefile.am === --- libstdc++-v3/src/c++11/Makefile.am (revision 196316) +++ libstdc++-v3/src/c++11/Makefile.am (working copy) @@ -124,3 +124,4 @@ # Google-specific pessimization functexcept.lo_no_omit_frame_pointer = -fno-omit-frame-pointer +debug.lo_no_omit_frame_pointer = -fno-omit-frame-pointer Index: libstdc++-v3/src/c++11/Makefile.in === --- libstdc++-v3/src/c++11/Makefile.in (revision 196316) +++ libstdc++-v3/src/c++11/Makefile.in (working copy) @@ -391,6 +391,7 @@ # Google-specific pessimization functexcept.lo_no_omit_frame_pointer = -fno-omit-frame-pointer +debug.lo_no_omit_frame_pointer = -fno-omit-frame-pointer all: all-am .SUFFIXES:
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Wed, Feb 27, 2013 at 12:17 PM, Andrew Pinski wrote: > Just an aside which program does not understand dwarf2 unwinding? All Google executables currently link in a frame-based unwinder. This allows us to report crashes and record runtime statistics from the executable itself, in ways that are convenient when dealing with thousands of executables spread across millions of machines, and which are much harder to achieve using external tools. There is of course libunwind, but it turns out that it's very hard to beat speed and simplicity of a frame-based unwinder (which matters when you collect stack traces across live production services). I hope this answers your question. -- Paul Pluzhnikov
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Wed, Feb 27, 2013 at 1:46 PM, Andrew Pinski wrote: > libunwind is not needed since there is already a dwarf2 based unwinder > that is accessible in libgcc already. Last I checked, libgcc dwarf handling code called malloc, and thus wasn't suitable when you want to instrument malloc *itself* to collect stack traces, nor for SIGPROF profiling. Is libgcc dwarf code async-signal safe now? If not, will it ever be? > I don't know why people still > promote libunwind when libgcc already has similar facilities. Because these facilities are not (or at least were not in the recent past) suitable for the unwinder requirements that people have? Thanks, -- Paul Pluzhnikov
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Wed, Feb 27, 2013 at 5:52 PM, Ian Lance Taylor wrote: > I think that the libgcc unwinder only calls malloc if somebody calls > __register_frame_info. So I looked. Indeed it doesn't call malloc in our environment, but does call dl_iterate_phdr, which is just as deadly. To be fair, libunwind does that as well, and we have to provide a workaround for that. Still, out-of-the-box libgcc unwinder is not suitable for our unwind requirements. > I thought a more serious issue is that the libgcc unwinder has no way > of dealing with a corrupt stack--it will simply crash. That too ... Thanks, -- Paul Pluzhnikov
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Thu, Feb 28, 2013 at 9:28 AM, Ian Lance Taylor wrote: >> does call dl_iterate_phdr, which is just as deadly. > > Hmmm, I guess I can't remember why. Let me refresh your memory. You've seen this deadlock before: Thread 1Thread 2 dlopen malloc ... takes loader lock ... takes malloc lock malloc _Unwind_Backtrace ... needs malloc lock dl_iterate_phdr held by T2... needs loader lock held by T1 -- Paul Pluzhnikov
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Thu, Feb 28, 2013 at 9:07 AM, Xinliang David Li wrote: > Any insight about the relative performance of the two implementations? We have a benchmark for the speed of unwinder. Here are results. The number /1, /2, etc. is the number of levels in the stack trace. Using frame-based unwinder: Benchmark Time(ns)CPU(ns) Iterations BM_GetStackTrace/1 29 29 24137931 BM_GetStackTrace/2 38 38 17948718 BM_GetStackTrace/3 43 44 16279070 BM_GetStackTrace/4 57 57 1000 BM_GetStackTrace/5 62 62 1000 BM_GetStackTrace/6 65 65 1000 BM_GetStackTrace/7 65 64 1000 BM_GetStackTrace/8 65 65 1000 BM_GetStackTrace/9 65 65 1000 BM_GetStackTrace/10 65 65 1000 Using libgcc: Benchmark Time(ns)CPU(ns) Iterations BM_GetStackTrace/11543 1543 47 BM_GetStackTrace/22042 2057 35 BM_GetStackTrace/32378 2366 291667 BM_GetStackTrace/42754 2720 25 BM_GetStackTrace/53212 3200 218750 BM_GetStackTrace/63655 3651 19 BM_GetStackTrace/74039 4000 175000 BM_GetStackTrace/84009 4000 175000 BM_GetStackTrace/94002 4000 175000 BM_GetStackTrace/10 4017 4000 175000 Using libunwind: Benchmark Time(ns)CPU(ns) Iterations BM_GetStackTrace/1 1091086363636 BM_GetStackTrace/2 121122583 BM_GetStackTrace/3 130130500 BM_GetStackTrace/4 133132500 BM_GetStackTrace/5 148148467 BM_GetStackTrace/6 1621624375000 BM_GetStackTrace/7 1741754117647 BM_GetStackTrace/8 185185389 BM_GetStackTrace/9 1881873684211 BM_GetStackTrace/101881873684211 Conclusions: - frame based unwinder is hard to beat (re-confirmed :-) - libunwind is getting really close at 3x the overhead - libgcc is nowhere close at 50x. -- Paul Pluzhnikov
Re: [google gcc-4_7, integration] Build more of libstdc++ with frame pointers
On Thu, Feb 28, 2013 at 11:59 AM, Jakub Jelinek wrote: > On Thu, Feb 28, 2013 at 11:57:48AM -0800, Cary Coutant wrote: >> Similarly, couldn't dlopen drop the loader lock while calling malloc? > > It can't, but perhaps it could call some alternative malloc instead > (the simpler malloc version in ld.so or similar). ld-linux starts calling the simpler malloc in dl-minimal.c, then switches to "real" libc.so.6 malloc later on. This behavior causes a lot of pain to anyone who tries to interpose malloc and use dlsym(RTLD_NEXT,...) or similar from the interposer. Roland explained to me ~15 years ago why it is that way (it had something to do with Hurd); an explanation I can't find at the moment. -- Paul Pluzhnikov
Re: [google][4.7]Using CPU mocks to test code coverage of multiversioned functions
+cc libc-alpha On Mon, Mar 18, 2013 at 9:05 AM, Xinliang David Li wrote: > Interesting idea about lazy IFUNC relocation. > On Mon, Mar 18, 2013 at 2:02 AM, Richard Biener > wrote: >> On Fri, Mar 15, 2013 at 10:55 PM, Sriraman Tallam >> wrote: >>>This patch is meant for google/gcc-4_7 but I want this to be >>> considered for trunk when it opens again. This patch makes it easy to >>> test for code coverage of multiversioned functions. Here is a >>> motivating example: >> Err. As we are using IFUNCs isn't it simply possible to do this in >> the dynamic loader - for example by simlply pre-loading a library >> with the IFUNC relocators implemented differently? Thus, shouldn't >> we simply provide such library as a convenience? A similar need exists in glibc itself: it too has multiversioned functions, and lack of testing has led to recent bugs in some of them. HJ has added a framework to test IFUNCs to glibc late last year, but it would be nice to have a more general IFUNC control, so I could e.g. run a binary on SSE4-capable machine A as that binary would run on SSE2-only capable machine B. (We've had a few bugs recently, were the crash would only show on machine B and not A. These are a pain to debug, as I may not have access to B.) If such a controller is implemented, I'd think it would have to be part of GLIBC (or part of the ld-linux itself), and not of libgcc. LD_CPU_FEATURES=sse,sse2 ./a.out # run as if only sse and sse2 are available Thanks, -- Paul Pluzhnikov
Re: [google][4.7]Using CPU mocks to test code coverage of multiversioned functions
On Mon, Mar 18, 2013 at 10:18 AM, Richard Biener wrote: > "H.J. Lu" wrote: >>We can pass environment variables to IFUNC selector. Maybe we can >>enable it for debug build. Enabling this for just debug builds would not cover my use case. If the environment variable is used at loader initialization time to override CPUID output, then the runtime cost of that code would be minuscule, and it can be available in production glibc builds. > I was asking for the ifunc selector to be > Overridable by ld_preload or a similar mechanism at dynamic load time. Yes, that's how I understood you. I don't believe it would be easy to implement such interposer (if possible at all), and it would be very much tied to glibc internals. Overriding CPUID at loader initialization time sounds simpler (but I haven't looked at the code yet :-). -- Paul Pluzhnikov
[patch][google/gcc-4_7] Suppress failure in gcc.dg/lto/20100430-1 on powerpc and powerpc64
Greetings, I've commited the patch below on google/gcc-4_7 branch (r198071) to XFAILs gcc.dg/lto/20100430-1 on powerpc*. Index: contrib/testsuite-management/powerpc64-grtev3-linux-gnu.xfail === --- contrib/testsuite-management/powerpc64-grtev3-linux-gnu.xfail (revision 198055) +++ contrib/testsuite-management/powerpc64-grtev3-linux-gnu.xfail (working copy) @@ -1,3 +1,6 @@ +# ICE to be reported upstream +FAIL: gcc.dg/lto/20100430-1 c_lto_20100430-1_0.o-c_lto_20100430-1_0.o link, -O2 -fprofile-arcs -flto -r -nostdlib (internal compiler error) +UNRESOLVED: gcc.dg/lto/20100430-1 c_lto_20100430-1_0.o-c_lto_20100430-1_0.o execute -O2 -fprofile-arcs -flto -r -nostdlib # Marked test failures for v16-powerpc64 toolchain built from # v16 release branch @64346, and v16 dev branch @64533. # *** gcc: Index: contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail === --- contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail (revision 198055) +++ contrib/testsuite-management/powerpc-grtev3-linux-gnu.xfail (working copy) @@ -1,3 +1,6 @@ +# ICE to be reported upstream +FAIL: gcc.dg/lto/20100430-1 c_lto_20100430-1_0.o-c_lto_20100430-1_0.o link, -O2 -fprofile-arcs -flto -r -nostdlib (internal compiler error) +UNRESOLVED: gcc.dg/lto/20100430-1 c_lto_20100430-1_0.o-c_lto_20100430-1_0.o execute -O2 -fprofile-arcs -flto -r -nostdlib # Ignore r194995 for gcc pr55852. FAIL: gfortran.dg/intrinsic_size_3.f90 -O scan-tree-dump-times original "iszs = \\(integer\\(kind=2\\)\\) MAX_EXPR <\\(D.->dim.0..ubound - D.->dim.0..lbound\\) \\+ 1, 0>;" 1 # Ignore gcc pr54127.
Re: [google/gcc-4_8] Add -fskeleton-type-units flag.
> Add -f[no-]skeleton-type-units flag to control whether GCC > generates skeleton type units. > > This patch is for the google/gcc-4_8 branch. Ok for Google branch. Thanks, -- Paul Pluzhnikov
Re: [google gcc-4_8][x86_64]Optimize access to globals in "-fpie -pie" builds with copy relocations
On Thu, May 22, 2014 at 4:36 PM, Sriraman Tallam wrote: > This patch is pending review for trunk. Please see if this is ok to > commit to google/gcc-4_8 branch. Please see this for more details: > > https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01215.html +mld-pie-copyrelocs +Target Report Var(ix86_ld_pie_copyrelocs) Init(0) +Use linker copy relocs for pie They are not "linker copy relocs", they are simply "copy relocations". So I would call the option "-mcopy-relocs" and be done with that. But this is just bike-shedding :-) LGTM. -- Paul Pluzhnikov
Re: detecting "container overflow" bugs in std::vector
On Mon, May 26, 2014 at 8:19 AM, Konstantin Serebryany wrote: > > On Mon, May 26, 2014 at 6:12 PM, Jonathan Wakely wrote: > > I see that the patch on the Google branch removes some of the > > __google_stl_debug_vector checks -- are they considered no longer > > necessary/useful with asan? > > This was some unrelated change. Paul? That appears to have been a merge mistake on my part. I'll add them back. Thanks for noticing. -- Paul Pluzhnikov
Is testing libgomp outside of the build tree supported?
Greetings, We test GCC without access to the build tree (we only have convenient access to install and source trees). Building libgomp.c/affinity-1.c and libgomp.c++/affinity-1.C fails in such testing, because of '#include "config.h"' which is nowhere to be found. Is that a bug? Should I open a bugzilla issue for it? Thanks,
[patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
Greetings,
The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
gold, but not when linked with BFD ld.
The problem appears to be off-by-one error causing array out of bounds
access, fixed by attached patch.
OK for trunk?
Thanks,
gcc/testsuite/ChangeLog:
2014-02-04 Paul Pluzhnikov
* gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
overflow.
Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
===
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -15,7 +15,7 @@
int i;
/* Not vectorizable due to data dependence: dependence distance 1. */
- for (i = N - 1; i >= 0; i--)
+ for (i = N - 2; i >= 0; i--)
{
ia[i] = ia[i+1] * 4;
}
@@ -28,7 +28,7 @@
}
/* Vectorizable. Dependence distance -1. */
- for (i = N - 1; i >= 0; i--)
+ for (i = N - 2; i >= 0; i--)
{
ib[i+1] = ib[i] * 4;
}
Re: [patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
+cc jakub
On Tue, Feb 4, 2014 at 4:59 PM, Paul Pluzhnikov wrote:
> Greetings,
>
> The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
> gold, but not when linked with BFD ld.
>
> The problem appears to be off-by-one error causing array out of bounds
> access, fixed by attached patch.
Alternate fix (used in no-vfa-vect-depend-3.c):
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -5,8 +5,8 @@
#define N 17
-int ia[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
-int ib[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ia[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ib[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
int res[N] = {48,192,180,168,156,144,132,120,108,96,84,72,60,48,36,24,12};
__attribute__ ((noinline))
>
> OK for trunk?
>
> Thanks,
>
>
> gcc/testsuite/ChangeLog:
>
> 2014-02-04 Paul Pluzhnikov
>
> * gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
> overflow.
>
>
> Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
> ===
> --- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
> +++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
> @@ -15,7 +15,7 @@
>int i;
>
>/* Not vectorizable due to data dependence: dependence distance 1. */
> - for (i = N - 1; i >= 0; i--)
> + for (i = N - 2; i >= 0; i--)
> {
>ia[i] = ia[i+1] * 4;
> }
> @@ -28,7 +28,7 @@
> }
>
>/* Vectorizable. Dependence distance -1. */
> - for (i = N - 1; i >= 0; i--)
> + for (i = N - 2; i >= 0; i--)
> {
>ib[i+1] = ib[i] * 4;
> }
--
Paul Pluzhnikov
Re: [patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
Ping?
On Tue, Feb 4, 2014 at 5:08 PM, Paul Pluzhnikov wrote:
> +cc jakub
>
> On Tue, Feb 4, 2014 at 4:59 PM, Paul Pluzhnikov
> wrote:
>> Greetings,
>>
>> The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
>> gold, but not when linked with BFD ld.
>>
>> The problem appears to be off-by-one error causing array out of bounds
>> access, fixed by attached patch.
>
> Alternate fix (used in no-vfa-vect-depend-3.c):
>
> --- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
> +++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
> @@ -5,8 +5,8 @@
>
> #define N 17
>
> -int ia[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
> -int ib[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
> +int ia[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
> +int ib[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
> int res[N] = {48,192,180,168,156,144,132,120,108,96,84,72,60,48,36,24,12};
>
> __attribute__ ((noinline))
>
>
>>
>> OK for trunk?
>>
>> Thanks,
>>
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2014-02-04 Paul Pluzhnikov
>>
>> * gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
>> overflow.
>>
>>
>> Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
>> ===
>> --- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
>> +++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
>> @@ -15,7 +15,7 @@
>>int i;
>>
>>/* Not vectorizable due to data dependence: dependence distance 1. */
>> - for (i = N - 1; i >= 0; i--)
>> + for (i = N - 2; i >= 0; i--)
>> {
>> ia[i] = ia[i+1] * 4;
>> }
>> @@ -28,7 +28,7 @@
>> }
>>
>>/* Vectorizable. Dependence distance -1. */
>> - for (i = N - 1; i >= 0; i--)
>> + for (i = N - 2; i >= 0; i--)
>> {
>>ib[i+1] = ib[i] * 4;
>> }
>
>
>
> --
> Paul Pluzhnikov
--
Paul Pluzhnikov
Re: [patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
Jakub,
On Mon, Feb 10, 2014 at 12:09 AM, Jakub Jelinek wrote:
> On Tue, Feb 04, 2014 at 04:59:14PM -0800, Paul Pluzhnikov wrote:
>> gcc/testsuite/ChangeLog:
>>
>> 2014-02-04 Paul Pluzhnikov
>>
>> * gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
>> overflow.
>
> Ok, thanks.
Sorry, did you vouch for this:
- for (i = N - 1; i >= 0; i--)
+ for (i = N - 2; i >= 0; i--)
or that:
-int ia[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ia[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
Thanks,
--
Paul Pluzhnikov
[patch] Fix PR59295 -- remove useless warning
Greetings,
Attached patch deletes code to warn about repeated friend declaration.
The warning has apparently been present since at least 1997, but it's
unlikely to have ever prevented any actual bugs.
Ok for trunk once it opens in stage1?
Tested on Linux/x86_64 with no regressions.
Thanks,
Google ref: b/11542609
--
2014-03-20 Paul Pluzhnikov
PR c++/59295
* gcc/cp/friend.c (add_friend): Remove complain parameter.
(do_friend): Adjust.
* gcc/cp/cp-tree.h (add_friend): Adjust.
* gcc/cp/pt.c (instantiate_class_template_1): Adjust.
gcc/testsuite/ChangeLog:
2014-03-20 Paul Pluzhnikov
PR c++/59295
* g++.old-deja/g++.benjamin/warn02.C: Adjust.
Index: gcc/cp/friend.c
===
--- gcc/cp/friend.c (revision 208738)
+++ gcc/cp/friend.c (working copy)
@@ -116,14 +116,10 @@
}
/* Add a new friend to the friends of the aggregate type TYPE.
- DECL is the FUNCTION_DECL of the friend being added.
+ DECL is the FUNCTION_DECL of the friend being added. */
- If COMPLAIN is true, warning about duplicate friend is issued.
- We want to have this diagnostics during parsing but not
- when a template is being instantiated. */
-
void
-add_friend (tree type, tree decl, bool complain)
+add_friend (tree type, tree decl)
{
tree typedecl;
tree list;
@@ -146,12 +142,7 @@
for (; friends ; friends = TREE_CHAIN (friends))
{
if (decl == TREE_VALUE (friends))
- {
- if (complain)
- warning (0, "%qD is already a friend of class %qT",
-decl, type);
- return;
- }
+ return;
}
maybe_add_class_template_decl_list (type, decl, /*friend_p=*/1);
@@ -374,20 +365,12 @@
if (TREE_CODE (friend_type) == TEMPLATE_DECL)
{
if (friend_type == probe)
- {
- if (complain)
- warning (0, "%qD is already a friend of %qT", probe, type);
- break;
- }
+ break;
}
else if (TREE_CODE (probe) != TEMPLATE_DECL)
{
if (same_type_p (probe, friend_type))
- {
- if (complain)
- warning (0, "%qT is already a friend of %qT", probe, type);
- break;
- }
+ break;
}
}
@@ -511,7 +494,7 @@
decl = DECL_TI_TEMPLATE (decl);
if (decl)
- add_friend (current_class_type, decl, /*complain=*/true);
+ add_friend (current_class_type, decl);
}
else
error ("member %qD declared as friend before type %qT defined",
@@ -602,8 +585,7 @@
return error_mark_node;
add_friend (current_class_type,
- is_friend_template ? DECL_TI_TEMPLATE (decl) : decl,
- /*complain=*/true);
+ is_friend_template ? DECL_TI_TEMPLATE (decl) : decl);
DECL_FRIEND_P (decl) = 1;
}
Index: gcc/cp/cp-tree.h
===
--- gcc/cp/cp-tree.h(revision 208738)
+++ gcc/cp/cp-tree.h(working copy)
@@ -5402,7 +5402,7 @@
/* friend.c */
extern int is_friend (tree, tree);
extern void make_friend_class (tree, tree, bool);
-extern void add_friend (tree, tree, bool);
+extern void add_friend (tree, tree);
extern tree do_friend (tree, tree, tree, tree, enum
overload_flags, bool);
/* in init.c */
Index: gcc/cp/pt.c
===
--- gcc/cp/pt.c (revision 208738)
+++ gcc/cp/pt.c (working copy)
@@ -9315,7 +9315,7 @@
}
r = tsubst_friend_function (t, args);
- add_friend (type, r, /*complain=*/false);
+ add_friend (type, r);
if (TREE_CODE (t) == TEMPLATE_DECL)
{
pop_deferring_access_checks ();
Index: gcc/testsuite/g++.old-deja/g++.benjamin/warn02.C
===
--- gcc/testsuite/g++.old-deja/g++.benjamin/warn02.C(revision 208736)
+++ gcc/testsuite/g++.old-deja/g++.benjamin/warn02.C(working copy)
@@ -20,14 +20,6 @@
int b;
};
-class C
-{
- friend int foo(const char *);
- friend int foo(const char *); // { dg-warning "" }
- int foo2() {return b;}
- int b;
-};
-
class D
{
public:
Re: [patch] Fix PR59295 -- remove useless warning
On Fri, Mar 21, 2014 at 1:58 AM, Fabien Chêne wrote: > Why not making this warning suppressable, instead of removing it ? > Shouldn't it fall under -W(no)-redundant-decls ? Thanks. I'll revise the patch to do that. -- Paul Pluzhnikov
[patch] Fix PR59295 -- move redundant friend decl warning under -Wredundant-decls
Greetings,
To fix PR59295, this patch moves (generally useless) warning about
repeated / redundant friend declarations under -Wredundant-decls.
Tested on Linux/x86_64 with no regressions.
Ok for trunk once it opens in stage 1?
Thanks,
--
2014-03-21 Paul Pluzhnikov
PR c++/59295
* gcc/cp/friend.c (add_friend, make_friend_class): Move repeated
friend warning under Wredundant_decls.
Index: gcc/cp/friend.c
===
--- gcc/cp/friend.c (revision 208748)
+++ gcc/cp/friend.c (working copy)
@@ -148,7 +148,8 @@
if (decl == TREE_VALUE (friends))
{
if (complain)
- warning (0, "%qD is already a friend of class %qT",
+ warning (OPT_Wredundant_decls,
+"%qD is already a friend of class %qT",
decl, type);
return;
}
@@ -376,7 +377,8 @@
if (friend_type == probe)
{
if (complain)
- warning (0, "%qD is already a friend of %qT", probe, type);
+ warning (OPT_Wredundant_decls,
+"%qD is already a friend of %qT", probe, type);
break;
}
}
@@ -385,7 +387,8 @@
if (same_type_p (probe, friend_type))
{
if (complain)
- warning (0, "%qT is already a friend of %qT", probe, type);
+ warning (OPT_Wredundant_decls,
+"%qT is already a friend of %qT", probe, type);
break;
}
}
Re: [patch] Fix PR59295 -- move redundant friend decl warning under -Wredundant-decls
Ping?
On Fri, Mar 21, 2014 at 9:16 AM, Paul Pluzhnikov wrote:
> Greetings,
>
> To fix PR59295, this patch moves (generally useless) warning about
> repeated / redundant friend declarations under -Wredundant-decls.
>
> Tested on Linux/x86_64 with no regressions.
>
> Ok for trunk once it opens in stage 1?
Which has just happened.
>
> Thanks,
>
> --
>
> 2014-03-21 Paul Pluzhnikov
>
> PR c++/59295
> * gcc/cp/friend.c (add_friend, make_friend_class): Move repeated
> friend warning under Wredundant_decls.
>
> Index: gcc/cp/friend.c
> ===
> --- gcc/cp/friend.c (revision 208748)
> +++ gcc/cp/friend.c (working copy)
> @@ -148,7 +148,8 @@
> if (decl == TREE_VALUE (friends))
> {
> if (complain)
> - warning (0, "%qD is already a friend of class %qT",
> + warning (OPT_Wredundant_decls,
> +"%qD is already a friend of class %qT",
> decl, type);
> return;
> }
> @@ -376,7 +377,8 @@
> if (friend_type == probe)
> {
> if (complain)
> - warning (0, "%qD is already a friend of %qT", probe, type);
> + warning (OPT_Wredundant_decls,
> +"%qD is already a friend of %qT", probe, type);
> break;
> }
> }
> @@ -385,7 +387,8 @@
> if (same_type_p (probe, friend_type))
> {
> if (complain)
> - warning (0, "%qT is already a friend of %qT", probe, type);
> + warning (OPT_Wredundant_decls,
> +"%qT is already a friend of %qT", probe, type);
> break;
> }
> }
--
Paul Pluzhnikov
Re: [google 4_7] backport "std::unique_ptr improvements"
On Thu, Jan 3, 2013 at 3:46 PM, Lawrence Crowl wrote: > Relax the restrictions on argument types to a unique_ptr > instantiated on an array type. This patch is a backport > of Jonathan Wakely's "std::unique_ptr improvements" > http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01271.html to the > google 4.7 branch. Approved for google/gcc-4_7 branch. Thanks, -- Paul Pluzhnikov
[google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
Back-port revision 194909 to google/gcc-4_7 branch:
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=194909
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54526
Google ref: b/7427993
Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C
===
--- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909)
+++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy)
@@ -31,5 +31,5 @@
};
void foo::x() throw(bar)
{
- if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda
expressions|expected" } parse error
+ if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda
expressions|expected|invalid" } parse error
}
Index: libcpp/lex.c
===
--- libcpp/lex.c(revision 194909)
+++ libcpp/lex.c(working copy)
@@ -2224,6 +2224,17 @@
{
if (*buffer->cur == ':')
{
+ /* C++11 [2.5/3 lex.pptoken], "Otherwise, if the next
+three characters are <:: and the subsequent character
+is neither : nor >, the < is treated as a preprocessor
+token by itself". */
+ if (CPP_OPTION (pfile, cplusplus)
+ && (CPP_OPTION (pfile, lang) == CLK_CXX11
+ || CPP_OPTION (pfile, lang) == CLK_GNUCXX11)
+ && buffer->cur[1] == ':'
+ && buffer->cur[2] != ':' && buffer->cur[2] != '>')
+ break;
+
buffer->cur++;
result->flags |= DIGRAPH;
result->type = CPP_OPEN_SQUARE;
--
This patch is available for review at http://codereview.appspot.com/7028052
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 9:41 AM, Xinliang David Li wrote:
> ok.
The patch as sent caused some breakage, I had to adjust test cases a bit.
Submitting attached patch.
Thanks,
--
Paul Pluzhnikov
Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C
===
--- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909)
+++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy)
@@ -31,5 +31,5 @@
};
void foo::x() throw(bar)
{
- if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda
expressions|expected" } parse error
+ if (!b) throw bar (static_cast<::N::X*>(this)); // { dg-error "lambda
expressions|expected|invalid" } parse error
}
Index: gcc/testsuite/g++.dg/parse/error12.C
===
--- gcc/testsuite/g++.dg/parse/error12.C(revision 194909)
+++ gcc/testsuite/g++.dg/parse/error12.C(working copy)
@@ -8,6 +8,6 @@
template
struct Foo {};
-Foo<::B> foo; // { dg-bogus "error" "error in place of warning" }
-// { dg-warning "4: '<::' cannot begin a template-argument list" "warning <::"
{ target *-*-* } 11 }
-// { dg-message "4:'<:' is an alternate spelling for '.'. Insert whitespace
between '<' and '::'" "note <:" { target *-*-* } 11 }
+Foo<::B> foo; // { dg-bogus "error" "error in place of warning" { target
c++98 } }
+// { dg-warning "4: '<::' cannot begin a template-argument list" "warning <::"
{ target c++98 } 11 }
+// { dg-message "4:'<:' is an alternate spelling for '.'. Insert whitespace
between '<' and '::'" "note <:" { target c++98 } 11 }
Index: gcc/testsuite/g++.dg/parse/error11.C
===
--- gcc/testsuite/g++.dg/parse/error11.C(revision 194909)
+++ gcc/testsuite/g++.dg/parse/error11.C(working copy)
@@ -16,22 +16,22 @@
};
void method(void) {
-typename Foo<::B>::template Nested<::B> n; // { dg-error "17:'<::' cannot
begin" "17-begin" }
-// { dg-message "17:'<:' is an alternate spelling" "17-alt" { target *-*-* }
19 }
-// { dg-error "39:'<::' cannot begin" "39-begin" { target *-*-* } 19 }
-// { dg-message "39:'<:' is an alternate spelling" "39-alt" { target *-*-* }
19 }
+typename Foo<::B>::template Nested<::B> n; // { dg-error "17:'<::' cannot
begin" "17-begin" { target c++98 } }
+// { dg-message "17:'<:' is an alternate spelling" "17-alt" { target c++98 }
19 }
+// { dg-error "39:'<::' cannot begin" "39-begin" { target c++98 } 19 }
+// { dg-message "39:'<:' is an alternate spelling" "39-alt" { target c++98 }
19 }
n.template Nested::method();
-n.template Nested<::B>::method(); // { dg-error "22:'<::' cannot begin"
"error" }
-// { dg-message "22:'<:' is an alternate" "note" { target *-*-* } 24 }
+n.template Nested<::B>::method(); // { dg-error "22:'<::' cannot begin"
"error" { target c++98 } }
+// { dg-message "22:'<:' is an alternate" "note" { target c++98 } 24 }
Nested::method();
-Nested<::B>::method(); // { dg-error "11:'<::' cannot begin" "error" }
-// { dg-message "11:'<:' is an alternate" "note" { target *-*-* } 27 }
+Nested<::B>::method(); // { dg-error "11:'<::' cannot begin" "error" {
target c++98 } }
+// { dg-message "11:'<:' is an alternate" "note" { target c++98 } 27 }
}
};
template struct Foo2 {};
-template struct Foo2<::B>; // { dg-error "21:'<::' cannot begin" "begin" }
-// { dg-message "21:'<:' is an alternate" "alt" { target *-*-* } 33 }
+template struct Foo2<::B>; // { dg-error "21:'<::' cannot begin" "begin" {
target c++98 } }
+// { dg-message "21:'<:' is an alternate" "alt" { target c++98 } 33 }
// { dg-message "25:type/value mismatch" "mismatch" { target *-*-* } 33 }
// { dg-error "25:expected a constant" "const" { target *-*-* } 33 }
@@ -39,11 +39,11 @@
void func(void)
{
- Foo<::B> f; // { dg-error
Re: [google 4_7] Backport r194909 (<:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 3:27 PM, Xinliang David Li wrote:
> I saw only one test case fix patch from Paolo in trunk. Does this
> patch include more local fixes?
There was an earlier patch which touched g++.dg/parse/error1{1,2}.C:
r191712 | paolo | 2012-09-25 07:44:52 -0700 (Tue, 25 Sep 2012) | 15 lines
/cp
2012-09-25 Paolo Carlini
PR c++/54526
* parser.c (cp_parser_template_id): In C++11 mode simply accept
X<::A>.
/testsuite
2012-09-25 Paolo Carlini
PR c++/54526
* g++.dg/cpp0x/parse2.C: New.
* g++.dg/parse/error11.C: Adjust.
* g++.dg/parse/error12.C: Likewise.
The r194909 reverted parser.c change, but didn't revert error1{1,2}.C changes.
Current state of trunk vs. google/gcc-4_7 branch:
diff -u ${trunk}/gcc/testsuite/g++.dg/parse/error11.C
${gcc-4_7}/gcc/testsuite/g++.dg/parse/error11.C
@@ -68,4 +68,4 @@
// On the first error message, an additional note about the use of
// -fpermissive should be present
-// { dg-message "17:\\(if you use '-fpermissive' or '-std=c\\+\\+11',
or '-std=gnu\\+\\+11' G\\+\\+ will accept your code\\)" "-fpermissive"
{ target c++98 } 19 }
+// { dg-message "17:\\(if you use '-fpermissive' G\\+\\+ will accept
your code\\)" "-fpermissive" { target c++98 } 19 }
(Minor adjustment to the expected error message).
error12.C is identical on trunk and google/gcc-4_7.
Thanks,
--
Paul Pluzhnikov
[patch] PR55982 Fix buglet in __strncat_chk
Greetings, In libssp/strncat-chk.c, the loop was unrolled 5 times (apparently by accident). Ok for trunk? Thanks, -- Paul Pluzhnikov 2013-01-14 Paul Pluzhnikov PR 55982 * strncat-chk.c (__strncat_chk): Fix loop unroll. Index: libssp/strncat-chk.c === --- libssp/strncat-chk.c(revision 195181) +++ libssp/strncat-chk.c(working copy) @@ -87,12 +87,6 @@ __strncat_chk (char *__restrict__ dest, const char *++dest = c; if (c == '\0') return s; - if (slen-- == 0) -__chk_fail (); - c = *src++; - *++dest = c; - if (c == '\0') -return s; } while (--n4 > 0); n &= 3; }
[google gcc-4_7] Backport r195207 (fix for PR55982) into google/gcc-4_7
Ok for google/gcc-4_7 ? Ref b/8003094 Thanks, -- Paul Pluzhnikov 2013-01-15 Paul Pluzhnikov PR 55982 * strncat-chk.c (__strncat_chk): Fix loop unroll. Index: libssp/strncat-chk.c === --- libssp/strncat-chk.c(revision 195212) +++ libssp/strncat-chk.c(working copy) @@ -87,12 +87,6 @@ __strncat_chk (char *__restrict__ dest, const char *++dest = c; if (c == '\0') return s; - if (slen-- == 0) -__chk_fail (); - c = *src++; - *++dest = c; - if (c == '\0') -return s; } while (--n4 > 0); n &= 3; }
[google gcc-4_7, integration] Add lightweight checks for front()/back() on empty vector
This patch adds lightweight checks for front()/back() on empty vector.
Google ref b/7939186
Ok for google/gcc-4_7 and google/integration branches?
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 195313)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -878,7 +878,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front()
- { return *begin(); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("begin() on empty vector");
+#endif
+return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -886,7 +891,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const
- { return *begin(); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("begin() on empty vector");
+#endif
+return *begin();
+ }
/**
* Returns a read/write reference to the data at the last
@@ -894,7 +904,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
back()
- { return *(end() - 1); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("back() on empty vector");
+#endif
+return *(end() - 1);
+ }
/**
* Returns a read-only (constant) reference to the data at the
@@ -902,7 +917,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
back() const
- { return *(end() - 1); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("back() on empty vector");
+#endif
+return *(end() - 1);
+ }
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 464. Suggestion for new member functions in standard containers.
@@ -917,7 +937,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
pointer
#endif
data() _GLIBCXX_NOEXCEPT
- { return std::__addressof(front()); }
+ {
+#if __google_stl_debug_vector
+if (empty()) return 0;
+#endif
+return std::__addressof(front());
+ }
#ifdef __GXX_EXPERIMENTAL_CXX0X__
const _Tp*
@@ -925,7 +950,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
const_pointer
#endif
data() const _GLIBCXX_NOEXCEPT
- { return std::__addressof(front()); }
+ {
+#if __google_stl_debug_vector
+if (empty()) return 0;
+#endif
+return std::__addressof(front());
+ }
// [23.2.4.3] modifiers
/**
Re: [google gcc-4_7, integration] Add lightweight checks for front()/back() on empty vector
On Sun, Jan 20, 2013 at 3:16 AM, Gerald Pfeifer wrote:
> Isn't the error message wrong, then?
Thanks for catching that! Updated patch attached.
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 195313)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -878,7 +878,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
front()
- { return *begin(); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("front() on empty vector");
+#endif
+return *begin();
+ }
/**
* Returns a read-only (constant) reference to the data at the first
@@ -886,7 +891,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
front() const
- { return *begin(); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("front() on empty vector");
+#endif
+return *begin();
+ }
/**
* Returns a read/write reference to the data at the last
@@ -894,7 +904,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
reference
back()
- { return *(end() - 1); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("back() on empty vector");
+#endif
+return *(end() - 1);
+ }
/**
* Returns a read-only (constant) reference to the data at the
@@ -902,7 +917,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
*/
const_reference
back() const
- { return *(end() - 1); }
+ {
+#if __google_stl_debug_vector
+if (empty()) __throw_logic_error("back() on empty vector");
+#endif
+return *(end() - 1);
+ }
// _GLIBCXX_RESOLVE_LIB_DEFECTS
// DR 464. Suggestion for new member functions in standard containers.
@@ -917,7 +937,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
pointer
#endif
data() _GLIBCXX_NOEXCEPT
- { return std::__addressof(front()); }
+ {
+#if __google_stl_debug_vector
+if (empty()) return 0;
+#endif
+return std::__addressof(front());
+ }
#ifdef __GXX_EXPERIMENTAL_CXX0X__
const _Tp*
@@ -925,7 +950,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
const_pointer
#endif
data() const _GLIBCXX_NOEXCEPT
- { return std::__addressof(front()); }
+ {
+#if __google_stl_debug_vector
+if (empty()) return 0;
+#endif
+return std::__addressof(front());
+ }
// [23.2.4.3] modifiers
/**
[google gcc-4_7, integration] Add more lightweight checks for dangling vectors
This patch sets destructed vector into an invalid state, such that
subsequent calls to begin(), end(), size(), etc. all throw logic_error.
Google ref b/7248326
Ok for google/gcc-4_7 and google/integration?
Thanks,
--
Paul Pluzhnikov
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 195356)
+++ libstdc++-v3/include/bits/stl_vector.h (working copy)
@@ -159,7 +159,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
~_Vector_base()
{ _M_deallocate(this->_M_impl._M_start, this->_M_impl._M_end_of_storage
- - this->_M_impl._M_start); }
+ - this->_M_impl._M_start);
+#if __google_stl_debug_dangling_vector
+this->_M_impl._M_start = 0;
+this->_M_impl._M_finish = reinterpret_cast<_Tp*>(~0UL);
+#endif
+ }
public:
_Vector_impl _M_impl;
[google gcc-4_7, integration] Scribble on destructed strings to catch invalid accesses.
This patch allows us to catch use of destructed strings.
Google ref: b/5430313
Ok for google/gcc-4_7 and google/integration?
--
Paul Pluzhnikov
Index: libstdc++-v3/include/ext/sso_string_base.h
===
--- libstdc++-v3/include/ext/sso_string_base.h (revision 195417)
+++ libstdc++-v3/include/ext/sso_string_base.h (working copy)
@@ -215,7 +215,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
const _Alloc& __a);
~__sso_string_base()
- { _M_dispose(); }
+ {
+ _M_dispose();
+#ifdef __google_stl_debug_dangling_string
+ __builtin_memset(this, 0xcd, sizeof(*this));
+#endif
+ }
_CharT_alloc_type&
_M_get_allocator()
Re: [google gcc-4_7, integration] Scribble on destructed strings to catch invalid accesses.
On Thu, Jan 24, 2013 at 4:15 PM, Jonathan Wakely wrote: > This does look like something that could be included in debug mode, > but probably not until we're back in Stage 1. Please either put it in > Bugzilla (and CC me) or if you're diligent enough to remember please > email us again during Stage 1 :-) http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56109 > Googlers, please include the libstdc++ list on patches to the > libstdc++ code, even if it's only to a google branch. Will do. Sorry about that. Thanks, -- Paul Pluzhnikov
Re: [google/gcc-4_7] Fix ICE in should_move_die_to_comdat
On Wed, Aug 22, 2012 at 5:11 PM, Cary Coutant wrote: > This patch is for the google/gcc-4_7 branch. Approved for google/gcc-4_7 branch. Thanks, -- Paul Pluzhnikov
Re: [google/gcc-4_7] Backport patch for comdat types problem
On Wed, Sep 5, 2012 at 7:46 PM, Cary Coutant wrote: > This patch is for the google/gcc-4_7 branch. Approved for google/gcc-4_7 branch. Thanks, -- Paul Pluzhnikov
Re: [google] remove versioned symbols from libstdc++.a
On Wed, Sep 5, 2012 at 6:26 PM, Ollie Wild wrote: > Okay for google/integration and google/gcc-4_7? Approved for google/* branches. Thanks, -- Paul Pluzhnikov
Re: [google][gcc-4.7]Revert Old CPU Runtime Detection Implementation (issue6458081)
On Fri, Aug 3, 2012 at 2:56 PM, Sriraman Tallam wrote: > This patch reverts the original implementation of CPU runtime detection Ok for google/gcc-4_7 branch -- Paul Pluzhnikov
[patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
Greetings,
This patch adds a lightweight self-consistency check to many vector
operations. Google issue 5246356.
Ok for google/integration branch?
Thanks,
--
2011-09-06 Paul Pluzhnikov
* include/bits/stl_vector.h (__is_valid): New function.
(begin, end, size, capacity, swap, clear): Call it.
* include/bits/vector.tcc (operator=): Likewise.
Index: include/bits/stl_vector.h
===
--- include/bits/stl_vector.h (revision 178493)
+++ include/bits/stl_vector.h (working copy)
@@ -234,6 +234,16 @@
using _Base::_M_impl;
using _Base::_M_get_Tp_allocator;
+ bool __is_valid() const
+ {
+return (this->_M_impl._M_end_of_storage == 0
+ && this->_M_impl._M_start == 0
+ && this->_M_impl._M_finish == 0)
+ || (this->_M_impl._M_start <= this->_M_impl._M_finish
+ && this->_M_impl._M_finish <= this->_M_impl._M_end_of_storage
+ && this->_M_impl._M_start < this->_M_impl._M_end_of_storage);
+ }
+
public:
// [23.2.4.1] construct/copy/destroy
// (assign() and get_allocator() are also listed in this section)
@@ -531,7 +541,13 @@
*/
iterator
begin() _GLIBCXX_NOEXCEPT
- { return iterator(this->_M_impl._M_start); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return iterator(this->_M_impl._M_start);
+ }
/**
* Returns a read-only (constant) iterator that points to the
@@ -540,7 +556,13 @@
*/
const_iterator
begin() const _GLIBCXX_NOEXCEPT
- { return const_iterator(this->_M_impl._M_start); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("begin() on corrupt (dangling?) vector");
+#endif
+ return const_iterator(this->_M_impl._M_start);
+ }
/**
* Returns a read/write iterator that points one past the last
@@ -549,7 +571,13 @@
*/
iterator
end() _GLIBCXX_NOEXCEPT
- { return iterator(this->_M_impl._M_finish); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return iterator(this->_M_impl._M_finish);
+ }
/**
* Returns a read-only (constant) iterator that points one past
@@ -558,7 +586,13 @@
*/
const_iterator
end() const _GLIBCXX_NOEXCEPT
- { return const_iterator(this->_M_impl._M_finish); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("end() on corrupt (dangling?) vector");
+#endif
+ return const_iterator(this->_M_impl._M_finish);
+ }
/**
* Returns a read/write reverse iterator that points to the
@@ -638,7 +672,13 @@
/** Returns the number of elements in the %vector. */
size_type
size() const _GLIBCXX_NOEXCEPT
- { return size_type(this->_M_impl._M_finish - this->_M_impl._M_start); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("size() on corrupt (dangling?) vector");
+#endif
+ return size_type(this->_M_impl._M_finish - this->_M_impl._M_start);
+ }
/** Returns the size() of the largest possible %vector. */
size_type
@@ -718,7 +758,12 @@
*/
size_type
capacity() const _GLIBCXX_NOEXCEPT
- { return size_type(this->_M_impl._M_end_of_storage
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("capacity() on corrupt (dangling?) vector");
+#endif
+ return size_type(this->_M_impl._M_end_of_storage
- this->_M_impl._M_start); }
/**
@@ -1112,6 +1157,10 @@
noexcept(_Alloc_traits::_S_nothrow_swap())
#endif
{
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid() || !__x.__is_valid())
+ __throw_logic_error("swap() on corrupt (dangling?) vector");
+#endif
this->_M_impl._M_swap_data(__x._M_impl);
_Alloc_traits::_S_on_swap(_M_get_Tp_allocator(),
__x._M_get_Tp_allocator());
@@ -1125,7 +1174,13 @@
*/
void
clear() _GLIBCXX_NOEXCEPT
- { _M_erase_at_end(this->_M_impl._M_start); }
+ {
+#if __google_stl_debug_dangling_vector
+if (!this->__is_valid())
+ __throw_logic_error("clear() on corrupt (dangling?) vector");
+#endif
+ _M_
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 9:28 AM, Paul Pluzhnikov wrote: > This patch adds a lightweight self-consistency check to many vector > operations. Google issue 5246356. Sorry, forgot to mention: tested by doing bootstrap and make check on Linux/x86_64. -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 9:44 AM, Diego Novillo wrote: > OK. Any reason not to send this (or a variant) to mainline? AFAIU, mainline is not interested -- there is already a debug mode (enabled by _GLIBCXX_DEBUG), which catches many of the same bugs (and more), and introduction of "parallel" debug modes is undesirable. Unfortunately, _GLIBCXX_DEBUG makes no performance guarantees (making some normally constant-time operations O(N), etc.) and so we can't just turn it on in Google. Thanks, -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 10:46 AM, Diego Novillo wrote: > On Tue, Sep 6, 2011 at 12:54, Paul Pluzhnikov wrote: >> On Tue, Sep 6, 2011 at 9:44 AM, Diego Novillo wrote: >> >>> OK. Any reason not to send this (or a variant) to mainline? >> >> AFAIU, mainline is not interested -- there is already a debug mode (enabled >> by _GLIBCXX_DEBUG), which catches many of the same bugs (and more), and >> introduction of "parallel" debug modes is undesirable. >> >> Unfortunately, _GLIBCXX_DEBUG makes no performance guarantees (making some >> normally constant-time operations O(N), etc.) and so we can't just turn >> it on in Google. > > Right. That's why I thought of a variant. Maybe we want to have > levels of checking, or a _GLBICXX_DEBUG_FAST. Which would introduce a "parallel" debug mode ... which has been rejected in the past. > But this is something to discuss with libstdc++ (CC'd). Sure. If the "parallel" debug mode is more tenable now, I am all for it. To give some context, in a large code base (> 1e6 lines of C++ code), the checks added in this patch found 20 bugs. Most (though not all) of these bugs could also have been found with Valgrind and (probably) with _GLIBCXX_DEBUG, but the runtime cost of running such heavy-weight checks over the entire code base is prohibitive. Thanks, -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 12:44 PM, Jonathan Wakely wrote: > On 6 September 2011 20:23, Jonathan Wakely wrote: >> >> What's a dangling vector anyway? One that has been moved from? > > Apparently not, since a moved-from vector would pass __valid() (as > indeed it should) > > So I'm quite curious what bugs this catches. The garden variety "use after free": vector<> *v = new vector<>; ... delete v; ... for (it = v->begin(); it != v->end(); ++it) // Oops! > The existing debug mode > catches some fairly subtle cases of user error such as comparing > iterators from different containers, or dereferencing past-the-end > iterators. This patch seems to catch user errors related to ... what? > Heap corruption? Using a vector after its destructor runs? Those > are pretty serious, un-subtle errors and not specific to vector, so > why add code to vector (code which isn't 100% reliable if it only > catches corruption after the memory is reused and new data written to > it) It is 100% percent reliable for us, due to use of a debugging allocator which scribbles on all free()d memory. But yes, that's one more reason why this patch is not really good for upstream. > to detect more general problems that can happen to any type? We can't (easily) catch the general problem. This patch allows us to easily catch this particular instance of it. > The fact the patch did catch real bugs doesn't mean it's a good idea, > as you say, those bugs probably could have been caught in other ways. Sure -- we have other ways to catch the these bugs. They are not very practical at the moment due to their runtime overhead. As for your other suggestion: enabling _GLIBCXX_DEBUG just for vector, that didn't occur to me and is something I'd like to explore. Thanks, -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 1:33 PM, Jonathan Wakely wrote: >> for (it = v->begin(); it != v->end(); ++it) // Oops! > > Eurgh, the occurrence of "delete" in anything except a destructor is a > code smell that should have led someone to find those bugs anyway! > Obviously the code above is a trivial example, but it's unforgivable > to write that I can assure you that the actual code that exhibited these bugs was much subtler than that, and the bugs were not immediately obvious. Sometimes they were not immediately obvious even after running Valgrind on the buggy code and getting allocation/deletion/access stack traces with source corrdinates. >> We can't (easily) catch the general problem. This patch allows us to easily >> catch this particular instance of it. > > Sure, but so does adding "assert(this);" at the top of every member Sorry, no. Adding "assert(this);" does not catch any new bugs (at least not on Linux) -- the code would have immediately crashed on zero-page dereference anyway. Thanks, -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Tue, Sep 6, 2011 at 2:51 PM, Jonathan Wakely wrote: > I don't mean for vector::begin and the other functions in that patch, > I mean in general for member functions of any type. There are plenty > of functions that wouldn't crash when called through a null pointer. > But even std:vector has member functions like that, such as max_size. Right. (We might tweak the compiler to automagically insert that assert in non-omitimized builds ;-) > Anyway, I think we've concluded the patch is not suitable for general > use, as it has limited value without a debugging allocator that makes > pages dirty after free. Agreed. I'll rename __is_valid to _M_is_valid to match the rest of the file, and submit to google/integration only. Thanks for your comments, -- Paul Pluzhnikov
Re: [patch][google/integration] Add lightweight checks to vector::begin et. al. (issue4973065)
On Wed, Sep 7, 2011 at 2:34 AM, Pedro Alves wrote:
> Zeroing out would hide bugs; there's lots of code that does
>
> delete ptr;
> ...
> if (ptr)
> {
> ptr->...
> }
>
> You'd not see the bug that way. Making 'delete v' clobber the pointer
> with 0xdeadbeef or ~0 instead would be better.
Right. In practice, I don't believe I've ever seen this bug in such a
"pure" form though.
What I often see is
ptr = new Foo;
DoSomethingInAnotherThread(ptr);
...
delete ptr; // Oops. Didn't wait for another thread to finish
}
Or
ptr = new Foo;
DoSomethingThatDeletes(ptr);
ptr->x++; // Oops. Use after free
AFAICT, neither of these would be helped by delete stomping on the pointer.
--
Paul Pluzhnikov
[patch][google/integration] Don't force tls-model to initial-exec when building libgomp (issue6107046)
Greetings,
The patch below is needed for google/integration branch:
we want to be able build libgomp.a with -fPIC, be able to link it into a
shared library, and be able to dlopen that library without running out of
static TLS space (-ftls-model=initial-exec precludes that last part).
Google ref b/6368405
Google ref b/6156799
Tested: make && make check
2012-04-22 Paul Pluzhnikov
* libgomp/configure.tgt: Don't force initial-exec.
Index: libgomp/configure.tgt
===
--- libgomp/configure.tgt (revision 186636)
+++ libgomp/configure.tgt (working copy)
@@ -10,16 +10,6 @@
# XCFLAGS Add extra compile flags to use.
# XLDFLAGSAdd extra link flags to use.
-# Optimize TLS usage by avoiding the overhead of dynamic allocation.
-if test $gcc_cv_have_tls = yes ; then
- case "${target}" in
-
-*-*-linux*)
- XCFLAGS="${XCFLAGS} -ftls-model=initial-exec"
- ;;
- esac
-fi
-
# Since we require POSIX threads, assume a POSIX system by default.
config_path="posix"
--
This patch is available for review at http://codereview.appspot.com/6107046
Re: [patch][google/integration] Don't force tls-model to initial-exec when building libgomp (issue6107046)
On Sun, Apr 22, 2012 at 10:09 PM, Andrew Pinski wrote: > IIRC the main reason is because the slow down from not using > initial-exec model for GOMP is a lot. Given a choice between "slows down a lot" and "doesn't work at all", we prefer the former ;-) I see that we are not alone: http://old.nabble.com/-patch--libgomp%3A-removing-nodlopen-flag-for-portability-td10286039.html Generally we don't use OpenMP, but some of our third-party libraries do depend on it. Thanks for the heads-up. -- Paul Pluzhnikov
[google/integration] Enable lightweight debug checks (issue4402041)
This patch adds lightweight debug checks (if enabled by macros).
To be applied only to google/integration branch.
Tested by bootstrapping and running "make check".
2011-04-12 Paul Pluzhnikov
* libstdc++-v3/include/ext/vstring.h: Enable debug checks when
__google_stl_debug_string is 1.
* libstdc++-v3/include/ext/sso_string_base.h: Scribble on
logically-dangling storage when __google_stl_debug_string_dangling
is 1.
* libstdc++-v3/include/bits/stl_vector.h: Enable debug checks when
__google_stl_debug_vector is 1.
*
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
Adjust line number.
*
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
Likewize.
*
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
Likewize.
*
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
Likewize.
Index: libstdc++-v3/include/ext/vstring.h
===
--- libstdc++-v3/include/ext/vstring.h (revision 172341)
+++ libstdc++-v3/include/ext/vstring.h (working copy)
@@ -37,6 +37,21 @@
#include
#include
+#if __google_stl_debug_string && !defined(_GLIBCXX_DEBUG)
+# undef _GLIBCXX_DEBUG_ASSERT
+# undef _GLIBCXX_DEBUG_PEDASSERT
+// Perform additional checks (but only in this file).
+# define _GLIBCXX_DEBUG_ASSERT(_Condition) \
+ if (! (_Condition)) {\
+char buf[512]; \
+__builtin_snprintf(buf, sizeof(buf), \
+ "%s:%d: %s: Assertion '%s' failed.\n", \
+ __FILE__, __LINE__, __func__, # _Condition); \
+std::__throw_runtime_error(buf); \
+ }
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition) _GLIBCXX_DEBUG_ASSERT(_Condition)
+#endif
+
namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -2793,4 +2808,12 @@
#include "vstring.tcc"
+#if __google_stl_debug_string && !defined(_GLIBCXX_DEBUG)
+// Undo our defines, so they don't affect anything else.
+# undef _GLIBCXX_DEBUG_ASSERT
+# undef _GLIBCXX_DEBUG_PEDASSERT
+# define _GLIBCXX_DEBUG_ASSERT(_Condition)
+# define _GLIBCXX_DEBUG_PEDASSERT(_Condition)
+#endif
+
#endif /* _VSTRING_H */
Index: libstdc++-v3/include/ext/sso_string_base.h
===
--- libstdc++-v3/include/ext/sso_string_base.h (revision 172341)
+++ libstdc++-v3/include/ext/sso_string_base.h (working copy)
@@ -86,6 +86,13 @@
{
if (!_M_is_local())
_M_destroy(_M_allocated_capacity);
+#if __google_stl_debug_string_dangling
+ else {
+ // Wipe local storage for destructed string with 0xCD.
+ // This mimics what DebugAllocation does to free()d memory.
+ __builtin_memset(_M_local_data, 0xcd, sizeof(_M_local_data));
+}
+#endif
}
void
@@ -169,15 +176,29 @@
_M_leak() { }
void
- _M_set_length(size_type __n)
+ _M_set_length_no_wipe(size_type __n)
{
_M_length(__n);
traits_type::assign(_M_data()[__n], _CharT());
}
+ void
+ _M_set_length(size_type __n)
+ {
+#if __google_stl_debug_string_dangling
+ if (__n + 1 < _M_length())
+ {
+ // Wipe the storage with 0xCD.
+ // Also wipes the old NUL terminator.
+ __builtin_memset(_M_data() + __n + 1, 0xcd, _M_length() - __n);
+ }
+#endif
+ _M_set_length_no_wipe(__n);
+ }
+
__sso_string_base()
: _M_dataplus(_M_local_data)
- { _M_set_length(0); }
+ { _M_set_length_no_wipe(0); }
__sso_string_base(const _Alloc& __a);
@@ -336,7 +357,7 @@
__sso_string_base<_CharT, _Traits, _Alloc>::
__sso_string_base(const _Alloc& __a)
: _M_dataplus(__a, _M_local_data)
-{ _M_set_length(0); }
+{ _M_set_length_no_wipe(0); }
template
__sso_string_base<_CharT, _Traits, _Alloc>::
@@ -426,7 +447,7 @@
__throw_exception_again;
}
- _M_set_length(__len);
+ _M_set_length_no_wipe(__len);
}
template
@@ -458,7 +479,7 @@
__throw_exception_again;
}
- _M_set_length(__dnew);
+ _M_set_length_no_wipe(__dnew);
}
template
@@ -475,7 +496,7 @@
if (__n)
_S_assign(_M_data(), __n, __c);
- _M_set_length(__n);
+ _M_set_length_no_wipe(__n);
}
template
Index: libstdc++-v3/include/bits/stl_vector.h
===
--- libstdc++-v3/include/bits/stl_vector.h (revision 17234
[google/integration] Enable lightweight debug checks (issue4408041)
This patch adds lightweight debug checks (if enabled by macros).
To be applied only to google/integration branch.
Tested by bootstrapping and running "make check".
2011-04-12 Paul Pluzhnikov
* libstdc++-v3/include/bits/stl_algo.h: Add comparator debug checks
when __google_stl_debug_compare is 1.
* libstdc++-v3/include/bits/stl_tree.h: Add comparator debug checks
when __google_stl_debug_rbtree is 1.
Index: libstdc++-v3/include/bits/stl_algo.h
===
--- libstdc++-v3/include/bits/stl_algo.h(revision 172360)
+++ libstdc++-v3/include/bits/stl_algo.h(working copy)
@@ -318,6 +318,39 @@
// count_if
// search
+// Local modification: if __google_stl_debug_compare is defined to
+// non-zero value, check sort predicate for strict weak ordering.
+// Google ref b/1731200.
+#if __google_stl_debug_compare
+ template
+ struct _CheckedCompare {
+_Compare _M_compare;
+
+_CheckedCompare(const _Compare & __comp): _M_compare(__comp) { }
+
+template
+bool operator()(const _Tp& __x, const _Tp& __y) {
+ if (_M_compare(__x, __x))
+__throw_runtime_error("strict weak ordering: (__x LT __x) != false");
+ if (_M_compare(__y, __y))
+__throw_runtime_error("strict weak ordering: (__y LT __y) != false");
+ bool lt = _M_compare(__x, __y);
+ if (lt && _M_compare(__y, __x))
+__throw_runtime_error("strict weak ordering: ((__x LT __y) && (__y LT
__x)) != false");
+ return lt;
+}
+
+// Different types; can't perform any checks.
+template
+bool operator()(const _Tp1& __x, const _Tp2& __y) {
+ return _M_compare(__x, __y);
+}
+ };
+# define __CheckedCompare(__comp) _CheckedCompare<__typeof__(__comp)>(__comp)
+#else
+# define __CheckedCompare(__comp) __comp
+#endif
+
/**
* This is an uglified
* search_n(_ForwardIterator, _ForwardIterator, _Integer, const _Tp&)
@@ -2041,18 +2074,20 @@
++__result_real_last;
++__first;
}
- std::make_heap(__result_first, __result_real_last, __comp);
+ std::make_heap(__result_first, __result_real_last,
+ __CheckedCompare(__comp));
while (__first != __last)
{
- if (__comp(*__first, *__result_first))
+ if (__CheckedCompare(__comp)(*__first, *__result_first))
std::__adjust_heap(__result_first, _DistanceType(0),
_DistanceType(__result_real_last
- __result_first),
_InputValueType(*__first),
- __comp);
+ __CheckedCompare(__comp));
++__first;
}
- std::sort_heap(__result_first, __result_real_last, __comp);
+ std::sort_heap(__result_first, __result_real_last,
+ __CheckedCompare(__comp));
return __result_real_last;
}
@@ -2413,7 +2448,7 @@
_DistanceType __half = __len >> 1;
_ForwardIterator __middle = __first;
std::advance(__middle, __half);
- if (__comp(*__middle, __val))
+ if (__CheckedCompare(__comp)(*__middle, __val))
{
__first = __middle;
++__first;
@@ -2509,7 +2544,7 @@
_DistanceType __half = __len >> 1;
_ForwardIterator __middle = __first;
std::advance(__middle, __half);
- if (__comp(__val, *__middle))
+ if (__CheckedCompare(__comp)(__val, *__middle))
__len = __half;
else
{
@@ -2628,13 +2663,13 @@
_DistanceType __half = __len >> 1;
_ForwardIterator __middle = __first;
std::advance(__middle, __half);
- if (__comp(*__middle, __val))
+ if (__CheckedCompare(__comp)(*__middle, __val))
{
__first = __middle;
++__first;
__len = __len - __half - 1;
}
- else if (__comp(__val, *__middle))
+ else if (__CheckedCompare(__comp)(__val, *__middle))
__len = __half;
else
{
@@ -2711,7 +2746,7 @@
__val, __comp);
_ForwardIterator __i = std::lower_bound(__first, __last, __val, __comp);
- return __i != __last && !bool(__comp(__val, *__i));
+ return __i != __last && !bool(__CheckedCompare(__comp)(__val, *__i));
}
// merge
@@ -3180,11 +3215,11 @@
__last);
if (__buf.begin() == 0)
std::__merge_without_buffer(__first, __middle, __last, __len1,
- __len2, __comp);
+ __len2, __CheckedCompare(__comp));
else
std::__me
Re: [patch] make default linker --hash-style configurable option
Ping? Ping? On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov wrote: > Ping? -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
Ping? Ping? Ping? On Mon, Apr 18, 2011 at 9:45 AM, Paul Pluzhnikov wrote: > Ping? Ping? > > On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov > wrote: >> Ping? -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
Ping? Ping? Ping? Ping? This is getting ridiculous. Would someone please accept the patch, tell me what to fix in it to make it acceptable, or explain why it is a bad idea? Thanks! On Mon, Apr 25, 2011 at 9:08 AM, Paul Pluzhnikov wrote: > Ping? Ping? Ping? > > On Mon, Apr 18, 2011 at 9:45 AM, Paul Pluzhnikov > wrote: >> Ping? Ping? >> >> On Mon, Apr 11, 2011 at 11:00 AM, Paul Pluzhnikov >> wrote: >>> Ping? > > -- > Paul Pluzhnikov > -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
On Mon, May 2, 2011 at 7:59 AM, Joseph S. Myers wrote: Thanks for your comments. > When pinging, please include the URL of the patch being pinged and CC http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html > relevant maintainers (in this case, build system maintainers). All of them? [I've started with two ...] Thanks! -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
Ping? Ping? Ping? Ping? Ping? http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html CC'ing the rest of build system maintainers. On Mon, May 2, 2011 at 8:56 AM, Paul Pluzhnikov wrote: > On Mon, May 2, 2011 at 7:59 AM, Joseph S. Myers > wrote: > > Thanks for your comments. > >> When pinging, please include the URL of the patch being pinged and CC > > http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00246.html > >> relevant maintainers (in this case, build system maintainers). > > All of them? [I've started with two ...] Thanks! -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
On Mon, May 9, 2011 at 9:51 AM, Joseph S. Myers wrote: > On Mon, 9 May 2011, Paolo Bonzini wrote: > >> Uhm, so we deadlocked, I thought the other way. I cannot really >> express any opinion about the desirability of the feature, but the >> configure syntax is certainly okay with me, and I gather from the >> thread that you are fine with that as well. > > Given the build system changes, the gcc.c changes are OK. Ok for trunk then? I'll wait till tomorrow in case someone has additional comments on the desirability part. Thanks! -- Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
On Tue, May 10, 2011 at 7:02 AM, Ian Lance Taylor wrote:
> Richard Guenther writes:
>> I wonder why this is a GCC specific patch and not a linker patch. Why
>> not change the linker(s) to accept such configure option that changes its
>> default behavior?
>
> It is traditionally gcc which tells the linker what to do. E.g., Fedora
> has patched gcc to pass --hash-style=gnu to the linker.
I think 'traditionally' is the key here.
The same argument applies to e.g. --build-id. We already have GCC
supply that, but we could have changed the default in
binutils/{ld,gold}.
Also, the argument is discoverable (by observing 'gcc -v' output).
Changing the default in {ld,gold} makes it hard(er) to understand why
e.g. binaries linked from the same object files using two different
versions of GCC are different.
>> Otherwise if people link with ld they suddenly get different hash-style.
>> That looks wrong to me.
>
> That turns out not to be the case.
I think Richard meant "if people link directly with either ld or gold
(i.e. bypassing 'gcc' driver) get different hash-style". That's kind
of expected though -- if you want to get the same output you get from
'gcc', you need to examine 'gcc -v' very carefully. And, as Jakub
noted, linking directly with 'ld' is discouraged.
Thanks,
--
Paul Pluzhnikov
Re: [patch] make default linker --hash-style configurable option
On Wed, May 11, 2011 at 4:01 PM, Eric Botcazou wrote: > No gcc/ prefix in the ChangeLog file of the gcc/ directory. See other > entries. Fixed. Sorry about that. -- Paul Pluzhnikov
Build more of libstdc++ exception throwing code with frame pointers (issue4539068)
2011-05-19 Paul Pluzhnikov * libstdc++-v3/libsupc++/Makefile.am: Add -fno-omit-frame-pointer to vterminate. * libstdc++-v3/libsupc++/Makefile.in: Regenerate. Index: libstdc++-v3/libsupc++/Makefile.in === --- libstdc++-v3/libsupc++/Makefile.in (revision 173915) +++ libstdc++-v3/libsupc++/Makefile.in (working copy) @@ -481,6 +481,7 @@ # Google-specific pessimization eh_terminate.lo_no_omit_frame_pointer = -fno-omit-frame-pointer eh_throw.lo_no_omit_frame_pointer = -fno-omit-frame-pointer +vterminate.lo_no_omit_frame_pointer = -fno-omit-frame-pointer all: all-am .SUFFIXES: Index: libstdc++-v3/libsupc++/Makefile.am === --- libstdc++-v3/libsupc++/Makefile.am (revision 173915) +++ libstdc++-v3/libsupc++/Makefile.am (working copy) @@ -216,3 +216,4 @@ # Google-specific pessimization eh_terminate.lo_no_omit_frame_pointer = -fno-omit-frame-pointer eh_throw.lo_no_omit_frame_pointer = -fno-omit-frame-pointer +vterminate.lo_no_omit_frame_pointer = -fno-omit-frame-pointer -- This patch is available for review at http://codereview.appspot.com/4539068
[google] Re: Build more of libstdc++ exception throwing code with frame pointers (issue4539068)
This patch is for google/integration branch. Sorry about not setting the markers correctly. Tested by doing a bootstrap build and verifying that __gnu_cxx::__verbose_terminate_handler is built with frame pointers. On Thu, May 19, 2011 at 10:46 AM, Paul Pluzhnikov wrote: > 2011-05-19 Paul Pluzhnikov > > * libstdc++-v3/libsupc++/Makefile.am: Add -fno-omit-frame-pointer > to vterminate. > * libstdc++-v3/libsupc++/Makefile.in: Regenerate. -- Paul Pluzhnikov
Re: [Patch] Make libstdc++'s abi_check more robust against readelf output format
On Fri, May 20, 2011 at 8:10 AM, Ollie Wild wrote: > Ok, for google/integration. Please integrate to google/main and > google/gcc-4_6 as well. Done: r173959, r173960, r173961. Thanks, -- Paul Pluzhnikov
