Re: Long-standing error in exception handling by pool_allocator (containsuntested patch)

[Paolo Carlini <pcarlini at suse dot de>]

Paolo Carlini wrote:

> Therefore, my plan would be preparing a simple testcase from your 
> complete
> analysis, regression test the slighlty tweaked version, and repost it 
> to the list
> for final review.

The below is what I have just finished regtesting on x86/x86_64-linux. 
As you
can see, I simply test that the fall back machinery originally designed 
by SGI/HP
is actually working, instead of testing for memory corruption: the 
latter is more
tricky, depends on too many parameters and risk to convey the misleading
impression that the patch is "only" about memory corruption: in that 
case simply
zeroing _S_start_free/_S_end_free before calling ::operator new would 
suffice.

I'll wait until tomorrow in case of comments...

Paolo.

////////////////
2004-10-11  Joachim Kuebart  <kuebart@mathematik.uni-ulm.de>
	    Paolo Carlini  <pcarlini@suse.de>

	* src/pool_allocator.cc (__pool_alloc_base::_M_allocate_chunk):
	Deal properly with exceptions thrown by ::operator new(size_t).
	* testsuite/ext/pool_allocator/allocate_chunk.cc: New.

	* include/ext/pool_allocator.h: Include <cstdlib>.

diff -urN libstdc++-v3-orig/include/ext/pool_allocator.h libstdc++-v3/include/ext/pool_allocator.h
--- libstdc++-v3-orig/include/ext/pool_allocator.h	2004-08-30 20:22:01.000000000 +0200
+++ libstdc++-v3/include/ext/pool_allocator.h	2004-10-10 15:38:38.000000000 +0200
@@ -48,6 +48,7 @@
 #define _POOL_ALLOCATOR_H 1
 
 #include <bits/c++config.h>
+#include <cstdlib>
 #include <new>
 #include <bits/functexcept.h>
 #include <bits/atomicity.h>
diff -urN libstdc++-v3-orig/src/pool_allocator.cc libstdc++-v3/src/pool_allocator.cc
--- libstdc++-v3-orig/src/pool_allocator.cc	2004-09-02 00:16:58.000000000 +0200
+++ libstdc++-v3/src/pool_allocator.cc	2004-10-10 11:16:23.000000000 +0200
@@ -90,8 +90,11 @@
 	
 	size_t __bytes_to_get = (2 * __total_bytes
 				 + _M_round_up(_S_heap_size >> 4));
-	_S_start_free = static_cast<char*>(::operator new(__bytes_to_get));
-	if (_S_start_free == 0)
+	try
+	  {
+	    _S_start_free = static_cast<char*>(::operator new(__bytes_to_get));
+	  }
+	catch (...)
 	  {
 	    // Try to make do with what we have.  That can't hurt.  We
 	    // do not try smaller requests, since that tends to result
@@ -111,11 +114,9 @@
 		    // right free list.
 		  }
 	      }
-	    _S_end_free = 0;        // In case of exception.
-
-	    // This should either throw an exception or remedy the situation.
-	    // Thus we assume it succeeded.
-	    _S_start_free = static_cast<char*>(::operator new(__bytes_to_get));
+	    // what we have wasn't enough. Rethrow.
+	    _S_start_free = _S_end_free = 0;        // we have no chunk
+	    __throw_exception_again;
 	  }
 	_S_heap_size += __bytes_to_get;
 	_S_end_free = _S_start_free + __bytes_to_get;
diff -urN libstdc++-v3-orig/testsuite/ext/pool_allocator/allocate_chunk.cc libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc
--- libstdc++-v3-orig/testsuite/ext/pool_allocator/allocate_chunk.cc	1970-01-01 01:00:00.000000000 +0100
+++ libstdc++-v3/testsuite/ext/pool_allocator/allocate_chunk.cc	2004-10-10 17:12:24.000000000 +0200
@@ -0,0 +1,63 @@
+// 2004-10-10  Paolo Carlini  <pcarlini@suse.de>
+
+// Copyright (C) 2004 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 2, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING.  If not, write to the Free
+// Software Foundation, 59 Temple Place - Suite 330, Boston, MA 02111-1307,
+// USA.
+
+// 20.4.1.1 allocator members
+
+#include <ext/pool_allocator.h>
+
+struct small
+{
+  char c[16];
+};
+
+struct big
+{
+  char c[64];
+};
+
+void*
+operator new(size_t n) throw(std::bad_alloc)
+{
+  static bool first = true;
+  if (!first)
+    throw std::bad_alloc();
+  first = false;
+  return std::malloc(n);
+}
+
+// http://gcc.gnu.org/ml/libstdc++/2004-10/msg00098.html
+void test01()
+{
+  using __gnu_cxx::__pool_alloc;
+
+  __pool_alloc<big> alloc_big;
+  alloc_big.allocate(1);
+
+  // The constant 20 comes from __pool_alloc_base::_M_refill. See
+  // also __pool_alloc_base::_M_allocate_chunk.
+  __pool_alloc<small> alloc_small;
+  for (int i = 0; i < 20 * sizeof(big) / sizeof(small) + 1; ++i)
+    alloc_small.allocate(1);
+}
+
+int main()
+{
+  test01();
+}