Hi,

in some old, large, originally C-written application (using gcc-4.2.4 still)
I did have to find a bug that boils down to something like this:

   std::string x;
   strcpy( (char*) x.c_str(), "abc");

Any subsequent empty std::string instance did contain "abc" instead of "",
which was the issue showing up to the user.

My idea what could have helped out here was to make the empty string _Rep
object readonly (ie. const), to get a segmentation fault along the strcpy.

As the string impl doesn't write to the empty string _Rep object any more[1],
are there still reasons not to make it readonly, like in attached patch?

[1] http://gcc.gnu.org/PR40518

Thank you!
/haubi/
>From d16c5d0cabbe9b308f852534be0a209b5b2c5cfd Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner <michael.haubenwall...@salomon.at>
Date: Tue, 28 Aug 2012 17:05:48 +0200
Subject: [PATCH] Make default string storage readonly.

To help finding bugs that write to the empty string storage using C
string manipulation functions like strcpy, define the empty string
storage as const, resulting in a segmentation fault with such strcpy,
which is ways better than having subsequent empty strings non-empty.
---
 libstdc++-v3/ChangeLog                     |    6 ++++++
 libstdc++-v3/include/bits/basic_string.h   |    4 ++--
 libstdc++-v3/include/bits/basic_string.tcc |    4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index c2fcddc..86a5781 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,9 @@
+2012-08-28  Michael Haubenwallner <michael.haubenwall...@salomon.at>
+
+	(_S_empty_rep_storage): Make default string storage readonly.
+	* include/bits/basic_string.h: Declare as const.
+	* include/bits/basic_string.tcc: Define the const value.
+
 2012-08-27  Ulrich Drepper  <drep...@gmail.com>
 
 	Add interfaces to retrieve random numbers in bulk.
diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 24562c4..2d9b742 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -177,7 +177,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 	// The following storage is init'd to 0 by the linker, resulting
         // (carefully) in an empty string with one reference.
-        static size_type _S_empty_rep_storage[];
+        static size_type const _S_empty_rep_storage[];
 
         static _Rep&
         _S_empty_rep()
@@ -185,7 +185,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  // NB: Mild hack to avoid strict-aliasing warnings.  Note that
 	  // _S_empty_rep_storage is never modified and the punning should
 	  // be reasonably safe in this case.
-	  void* __p = reinterpret_cast<void*>(&_S_empty_rep_storage);
+	  void* __p = const_cast<void*>(reinterpret_cast<void const*>(&_S_empty_rep_storage));
 	  return *reinterpret_cast<_Rep*>(__p);
 	}
 
diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.tcc
index 7eff818..945ce1c 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -64,10 +64,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   // Linker sets _S_empty_rep_storage to all 0s (one reference, empty string)
   // at static init time (before static ctors are run).
   template<typename _CharT, typename _Traits, typename _Alloc>
-    typename basic_string<_CharT, _Traits, _Alloc>::size_type
+    typename basic_string<_CharT, _Traits, _Alloc>::size_type const
     basic_string<_CharT, _Traits, _Alloc>::_Rep::_S_empty_rep_storage[
     (sizeof(_Rep_base) + sizeof(_CharT) + sizeof(size_type) - 1) /
-      sizeof(size_type)];
+      sizeof(size_type)] = {};
 
   // NB: This is the special case for Input Iterators, used in
   // istreambuf_iterators, etc.
-- 
1.7.3.4

Reply via email to