[google/integration] Enable lightweight debug checks

Committed to google/integration as r172354.

[ppluzhnikov, 2011-04-12 22:56:55]

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  <ppluzhnikov@google.com>

	* 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 <ext/rc_string_base.h>
 #include <ext/sso_string_base.h>
 
+#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<typename _CharT, typename _Traits, typename _Alloc>
     __sso_string_base<_CharT, _Traits, _Alloc>::
@@ -426,7 +447,7 @@
 	    __throw_exception_again;
 	  }
 
-	_M_set_length(__len);
+	_M_set_length_no_wipe(__len);
       }
 
   template<typename _CharT, typename _Traits, typename _Alloc>
@@ -458,7 +479,7 @@
 	    __throw_exception_again;
 	  }
 
-	_M_set_length(__dnew);
+	_M_set_length_no_wipe(__dnew);
       }
 
   template<typename _CharT, typename _Traits, typename _Alloc>
@@ -475,7 +496,7 @@
       if (__n)
 	_S_assign(_M_data(), __n, __c);
 
-      _M_set_length(__n);
+      _M_set_length_no_wipe(__n);
     }
 
   template<typename _CharT, typename _Traits, typename _Alloc>
Index: libstdc++-v3/include/bits/stl_vector.h
===================================================================
--- libstdc++-v3/include/bits/stl_vector.h	(revision 172341)
+++ libstdc++-v3/include/bits/stl_vector.h	(working copy)
@@ -690,10 +690,18 @@
        *  Note that data access with this operator is unchecked and
        *  out_of_range lookups are not defined. (For checked lookups
        *  see at().)
+       *
+       *  Local modification: range checks are performed if
+       *  __google_stl_debug_vector is defined to non-zero.
        */
       reference
       operator[](size_type __n)
-      { return *(this->_M_impl._M_start + __n); }
+      {
+#if __google_stl_debug_vector
+	_M_range_check(__n);
+#endif
+	return *(this->_M_impl._M_start + __n);
+      }
 
       /**
        *  @brief  Subscript access to the data contained in the %vector.
@@ -705,10 +713,18 @@
        *  Note that data access with this operator is unchecked and
        *  out_of_range lookups are not defined. (For checked lookups
        *  see at().)
+       *
+       *  Local modification: range checks are performed if
+       *  __google_stl_debug_vector is defined to non-zero.
        */
       const_reference
       operator[](size_type __n) const
-      { return *(this->_M_impl._M_start + __n); }
+      {
+#if __google_stl_debug_vector
+	_M_range_check(__n);
+#endif
+	return *(this->_M_impl._M_start + __n);
+      }
 
     protected:
       /// Safety check used only from at().
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
===================================================================
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc	(revision
172341)
+++
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 *-*-* } 1150 }
+// { dg-error "no matching" "" { target *-*-* } 1166 }
 // { dg-excess-errors "" }
 
 #include <vector>
Index:
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
===================================================================
---
libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc	(revision
172341)
+++
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 *-*-* } 1191 }
+// { dg-error "no matching" "" { target *-*-* } 1207 }
 // { dg-excess-errors "" }
 
 #include <vector>
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
172341)
+++
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 *-*-* } 1080 }
+// { dg-error "no matching" "" { target *-*-* } 1096 }
 // { dg-excess-errors "" }
 
 #include <vector>
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
172341)
+++
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 *-*-* } 1080 }
+// { dg-error "no matching" "" { target *-*-* } 1096 }
 // { dg-excess-errors "" }
 
 #include <vector>

--
This patch is available for review at http://codereview.appspot.com/4402041

[Diego Novillo, 2011-04-13 00:39:29]

On Tue, Apr 12, 2011 at 18:56, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> 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  <ppluzhnikov@google.com>
>
>        * 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.

s/Likewize/Likewise/

OK with those changes.  Could you please cherrypick into google/main
and google/gcc-4_6 as well?


Diego.