[patch, fortran] Always return malloc(1) for empty arrays in the library

[patch, fortran] Always return malloc(1) for empty arrays in the library

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

Hello world,

looking at PR 49479 and other functions in the library made me realize
there are lots of places where we don't malloc one byte for empty
arrays.

This patch is an attempt at fixing the ton of regressions likely
caused by this (like in the PR) which haven't been found yet.
No test cases, as they haven't been found yet :-)

I also noticed two places where we had a memory leak (in eoshift1 and
eoshift3), which I also fixed.

Regression-tested.  OK for trunk and, after a few days, for 4.6?

Thomas



[-- Attachment #2: changelog --]
[-- Type: text/plain, Size: 7765 bytes --]

2011-06-28  Thomas Koenig  <tkoenig@gcc.gnu.org>

	* m4/in_pack.m4 (internal_pack_'rtype_ccode`):  If size is
	less than one, allocate a single byte.
	* m4/transpose.m4 (transpose_'rtype_code`):  Likewise.
	* m4/cshift1.m4 (cshift1):  Likewise.
	* m4/matmull.m4 (matmul_'rtype_code`):  Likewise.
	* m4/unpack.m4 (unpack0_'rtype_code`):  Likewise.
	* m4/ifunction_logical.m4 (name`'rtype_qual`_'atype_code):  Likewise.
	* m4/matmul.m4 (name`'rtype_qual`_'atype_code):  Likewise.
	* intrinics/transpose_generic.c (transpose_internal):  Likewise.
	* intrinsics/unpack_generic.c (unpack_internal):  Likewise.
	* m4/eoshift1.m4 (eoshift1):  Remove double allocation.
	* m4/eoshift3.m4 (eoshift3):  Likewise.
	* generated/all_l16.c: Regenerated.
	* generated/all_l1.c: Regenerated.
	* generated/all_l2.c: Regenerated.
	* generated/all_l4.c: Regenerated.
	* generated/all_l8.c: Regenerated.
	* generated/any_l16.c: Regenerated.
	* generated/any_l1.c: Regenerated.
	* generated/any_l2.c: Regenerated.
	* generated/any_l4.c: Regenerated.
	* generated/any_l8.c: Regenerated.
	* generated/count_16_l.c: Regenerated.
	* generated/count_1_l.c: Regenerated.
	* generated/count_2_l.c: Regenerated.
	* generated/count_4_l.c: Regenerated.
	* generated/count_8_l.c: Regenerated.
	* generated/cshift1_16.c: Regenerated.
	* generated/cshift1_4.c: Regenerated.
	* generated/cshift1_8.c: Regenerated.
	* generated/eoshift1_16.c: Regenerated.
	* generated/eoshift1_4.c: Regenerated.
	* generated/eoshift1_8.c: Regenerated.
	* generated/eoshift3_16.c: Regenerated.
	* generated/eoshift3_4.c: Regenerated.
	* generated/eoshift3_8.c: Regenerated.
	* generated/in_pack_c10.c: Regenerated.
	* generated/in_pack_c16.c: Regenerated.
	* generated/in_pack_c4.c: Regenerated.
	* generated/in_pack_c8.c: Regenerated.
	* generated/in_pack_i16.c: Regenerated.
	* generated/in_pack_i1.c: Regenerated.
	* generated/in_pack_i2.c: Regenerated.
	* generated/in_pack_i4.c: Regenerated.
	* generated/in_pack_i8.c: Regenerated.
	* generated/in_pack_r10.c: Regenerated.
	* generated/in_pack_r16.c: Regenerated.
	* generated/in_pack_r4.c: Regenerated.
	* generated/in_pack_r8.c: Regenerated.
	* generated/matmul_c10.c: Regenerated.
	* generated/matmul_c16.c: Regenerated.
	* generated/matmul_c4.c: Regenerated.
	* generated/matmul_c8.c: Regenerated.
	* generated/matmul_i16.c: Regenerated.
	* generated/matmul_i1.c: Regenerated.
	* generated/matmul_i2.c: Regenerated.
	* generated/matmul_i4.c: Regenerated.
	* generated/matmul_i8.c: Regenerated.
	* generated/matmul_l16.c: Regenerated.
	* generated/matmul_l4.c: Regenerated.
	* generated/matmul_l8.c: Regenerated.
	* generated/matmul_r10.c: Regenerated.
	* generated/matmul_r16.c: Regenerated.
	* generated/matmul_r4.c: Regenerated.
	* generated/matmul_r8.c: Regenerated.
	* generated/maxloc1_16_i16.c: Regenerated.
	* generated/maxloc1_16_i1.c: Regenerated.
	* generated/maxloc1_16_i2.c: Regenerated.
	* generated/maxloc1_16_i4.c: Regenerated.
	* generated/maxloc1_16_i8.c: Regenerated.
	* generated/maxloc1_16_r10.c: Regenerated.
	* generated/maxloc1_16_r16.c: Regenerated.
	* generated/maxloc1_16_r4.c: Regenerated.
	* generated/maxloc1_16_r8.c: Regenerated.
	* generated/maxloc1_4_i16.c: Regenerated.
	* generated/maxloc1_4_i1.c: Regenerated.
	* generated/maxloc1_4_i2.c: Regenerated.
	* generated/maxloc1_4_i4.c: Regenerated.
	* generated/maxloc1_4_i8.c: Regenerated.
	* generated/maxloc1_4_r10.c: Regenerated.
	* generated/maxloc1_4_r16.c: Regenerated.
	* generated/maxloc1_4_r4.c: Regenerated.
	* generated/maxloc1_4_r8.c: Regenerated.
	* generated/maxloc1_8_i16.c: Regenerated.
	* generated/maxloc1_8_i1.c: Regenerated.
	* generated/maxloc1_8_i2.c: Regenerated.
	* generated/maxloc1_8_i4.c: Regenerated.
	* generated/maxloc1_8_i8.c: Regenerated.
	* generated/maxloc1_8_r10.c: Regenerated.
	* generated/maxloc1_8_r16.c: Regenerated.
	* generated/maxloc1_8_r4.c: Regenerated.
	* generated/maxloc1_8_r8.c: Regenerated.
	* generated/maxval_i16.c: Regenerated.
	* generated/maxval_i1.c: Regenerated.
	* generated/maxval_i2.c: Regenerated.
	* generated/maxval_i4.c: Regenerated.
	* generated/maxval_i8.c: Regenerated.
	* generated/maxval_r10.c: Regenerated.
	* generated/maxval_r16.c: Regenerated.
	* generated/maxval_r4.c: Regenerated.
	* generated/maxval_r8.c: Regenerated.
	* generated/minloc1_16_i16.c: Regenerated.
	* generated/minloc1_16_i1.c: Regenerated.
	* generated/minloc1_16_i2.c: Regenerated.
	* generated/minloc1_16_i4.c: Regenerated.
	* generated/minloc1_16_i8.c: Regenerated.
	* generated/minloc1_16_r10.c: Regenerated.
	* generated/minloc1_16_r16.c: Regenerated.
	* generated/minloc1_16_r4.c: Regenerated.
	* generated/minloc1_16_r8.c: Regenerated.
	* generated/minloc1_4_i16.c: Regenerated.
	* generated/minloc1_4_i1.c: Regenerated.
	* generated/minloc1_4_i2.c: Regenerated.
	* generated/minloc1_4_i4.c: Regenerated.
	* generated/minloc1_4_i8.c: Regenerated.
	* generated/minloc1_4_r10.c: Regenerated.
	* generated/minloc1_4_r16.c: Regenerated.
	* generated/minloc1_4_r4.c: Regenerated.
	* generated/minloc1_4_r8.c: Regenerated.
	* generated/minloc1_8_i16.c: Regenerated.
	* generated/minloc1_8_i1.c: Regenerated.
	* generated/minloc1_8_i2.c: Regenerated.
	* generated/minloc1_8_i4.c: Regenerated.
	* generated/minloc1_8_i8.c: Regenerated.
	* generated/minloc1_8_r10.c: Regenerated.
	* generated/minloc1_8_r16.c: Regenerated.
	* generated/minloc1_8_r4.c: Regenerated.
	* generated/minloc1_8_r8.c: Regenerated.
	* generated/minval_i16.c: Regenerated.
	* generated/minval_i1.c: Regenerated.
	* generated/minval_i2.c: Regenerated.
	* generated/minval_i4.c: Regenerated.
	* generated/minval_i8.c: Regenerated.
	* generated/minval_r10.c: Regenerated.
	* generated/minval_r16.c: Regenerated.
	* generated/minval_r4.c: Regenerated.
	* generated/minval_r8.c: Regenerated.
	* generated/product_c10.c: Regenerated.
	* generated/product_c16.c: Regenerated.
	* generated/product_c4.c: Regenerated.
	* generated/product_c8.c: Regenerated.
	* generated/product_i16.c: Regenerated.
	* generated/product_i1.c: Regenerated.
	* generated/product_i2.c: Regenerated.
	* generated/product_i4.c: Regenerated.
	* generated/product_i8.c: Regenerated.
	* generated/product_r10.c: Regenerated.
	* generated/product_r16.c: Regenerated.
	* generated/product_r4.c: Regenerated.
	* generated/product_r8.c: Regenerated.
	* generated/sum_c10.c: Regenerated.
	* generated/sum_c16.c: Regenerated.
	* generated/sum_c4.c: Regenerated.
	* generated/sum_c8.c: Regenerated.
	* generated/sum_i16.c: Regenerated.
	* generated/sum_i1.c: Regenerated.
	* generated/sum_i2.c: Regenerated.
	* generated/sum_i4.c: Regenerated.
	* generated/sum_i8.c: Regenerated.
	* generated/sum_r10.c: Regenerated.
	* generated/sum_r16.c: Regenerated.
	* generated/sum_r4.c: Regenerated.
	* generated/sum_r8.c: Regenerated.
	* generated/transpose_c10.c: Regenerated.
	* generated/transpose_c16.c: Regenerated.
	* generated/transpose_c4.c: Regenerated.
	* generated/transpose_c8.c: Regenerated.
	* generated/transpose_i16.c: Regenerated.
	* generated/transpose_i4.c: Regenerated.
	* generated/transpose_i8.c: Regenerated.
	* generated/transpose_r10.c: Regenerated.
	* generated/transpose_r16.c: Regenerated.
	* generated/transpose_r4.c: Regenerated.
	* generated/transpose_r8.c: Regenerated.
	* generated/unpack_c10.c: Regenerated.
	* generated/unpack_c16.c: Regenerated.
	* generated/unpack_c4.c: Regenerated.
	* generated/unpack_c8.c: Regenerated.
	* generated/unpack_i16.c: Regenerated.
	* generated/unpack_i1.c: Regenerated.
	* generated/unpack_i2.c: Regenerated.
	* generated/unpack_i4.c: Regenerated.
	* generated/unpack_i8.c: Regenerated.
	* generated/unpack_r10.c: Regenerated.
	* generated/unpack_r16.c: Regenerated.
	* generated/unpack_r4.c: Regenerated.
	* generated/unpack_r8.c: Regenerated.

[-- Attachment #3: p3.diff --]
[-- Type: text/x-patch, Size: 9581 bytes --]

Index: m4/in_pack.m4
===================================================================
--- m4/in_pack.m4	(Revision 175598)
+++ m4/in_pack.m4	(Arbeitskopie)
@@ -45,6 +45,7 @@ internal_pack_'rtype_ccode` ('rtype` * source)
   index_type stride0;
   index_type dim;
   index_type ssize;
+  index_type alloc_size;
   const 'rtype_name` *src;
   'rtype_name` * restrict dest;
   'rtype_name` *destptr;
@@ -79,7 +80,12 @@ internal_pack_'rtype_ccode` ('rtype` * source)
     return source->data;
 
   /* Allocate storage for the destination.  */
-  destptr = ('rtype_name` *)internal_malloc_size (ssize * sizeof ('rtype_name`));
+  if (unlikely (ssize < 1))
+    alloc_size = 1;
+  else
+    alloc_size = ssize * sizeof ('rtype_name`);
+
+  destptr = ('rtype_name` *)internal_malloc_size (alloc_size);
   dest = destptr;
   src = source->data;
   stride0 = stride[0];
Index: m4/transpose.m4
===================================================================
--- m4/transpose.m4	(Revision 175598)
+++ m4/transpose.m4	(Arbeitskopie)
@@ -52,6 +52,8 @@ transpose_'rtype_code` ('rtype` * const restrict r
 
   if (ret->data == NULL)
     {
+      index_type alloc_size, array_size;
+
       assert (GFC_DESCRIPTOR_RANK (ret) == 2);
       assert (ret->dtype == source->dtype);
 
@@ -61,7 +63,13 @@ transpose_'rtype_code` ('rtype` * const restrict r
       GFC_DIMENSION_SET(ret->dim[1], 0, GFC_DESCRIPTOR_EXTENT(source,0) - 1,
 			GFC_DESCRIPTOR_EXTENT(source, 1));
 
-      ret->data = internal_malloc_size (sizeof ('rtype_name`) * size0 ((array_t *) ret));
+      array_size = size0 ((array_t *) ret);
+      if (unlikely (array_size < 1))
+        alloc_size = 1;
+      else
+        alloc_size = sizeof ('rtype_name`) * array_size;
+
+      ret->data = internal_malloc_size (alloc_size);
       ret->offset = 0;
     } else if (unlikely (compile_options.bounds_check))
     {
Index: m4/eoshift1.m4
===================================================================
--- m4/eoshift1.m4	(Revision 175598)
+++ m4/eoshift1.m4	(Arbeitskopie)
@@ -89,7 +89,6 @@ eoshift1 (gfc_array_char * const restrict ret,
     {
       int i;
 
-      ret->data = internal_malloc_size (size * arraysize);
       ret->offset = 0;
       ret->dtype = array->dtype;
       for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
@@ -107,7 +106,7 @@ eoshift1 (gfc_array_char * const restrict ret,
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
 
         }
-      if (arraysize > 0)
+      if (likely (arraysize > 0))
 	ret->data = internal_malloc_size (size * arraysize);
       else
 	ret->data = internal_malloc_size (1);
Index: m4/eoshift3.m4
===================================================================
--- m4/eoshift3.m4	(Revision 175598)
+++ m4/eoshift3.m4	(Arbeitskopie)
@@ -90,7 +90,6 @@ eoshift3 (gfc_array_char * const restrict ret,
     {
       int i;
 
-      ret->data = internal_malloc_size (size * arraysize);
       ret->offset = 0;
       ret->dtype = array->dtype;
       for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
Index: m4/cshift1.m4
===================================================================
--- m4/cshift1.m4	(Revision 175598)
+++ m4/cshift1.m4	(Arbeitskopie)
@@ -81,7 +81,6 @@ cshift1 (gfc_array_char * const restrict ret,
     {
       int i;
 
-      ret->data = internal_malloc_size (size * arraysize);
       ret->offset = 0;
       ret->dtype = array->dtype;
       for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
@@ -98,6 +97,11 @@ cshift1 (gfc_array_char * const restrict ret,
 
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
         }
+      if (likely (arraysize > 0))
+	ret->data = internal_malloc_size (size * arraysize);
+      else
+	ret->data = internal_malloc_size (1);
+
     }
   else if (unlikely (compile_options.bounds_check))
     {
Index: m4/matmull.m4
===================================================================
--- m4/matmull.m4	(Revision 175598)
+++ m4/matmull.m4	(Arbeitskopie)
@@ -68,6 +68,8 @@ matmul_'rtype_code` ('rtype` * const restrict reta
 
   if (retarray->data == NULL)
     {
+      index_type array_size, alloc_size;
+
       if (GFC_DESCRIPTOR_RANK (a) == 1)
         {
 	  GFC_DIMENSION_SET(retarray->dim[0], 0,
@@ -87,9 +89,15 @@ matmul_'rtype_code` ('rtype` * const restrict reta
 	                    GFC_DESCRIPTOR_EXTENT(b,1) - 1,
 			    GFC_DESCRIPTOR_EXTENT(retarray,0));
         }
-          
-      retarray->data
-	= internal_malloc_size (sizeof ('rtype_name`) * size0 ((array_t *) retarray));
+
+      array_size = size0 ((array_t *) retarray);
+
+      if (unlikely (array_size < 1))
+        alloc_size = 1;
+      else
+        alloc_size = sizeof ('rtype_name`) * alloc_size;
+
+      retarray->data = internal_malloc_size ( alloc_size );
       retarray->offset = 0;
     }
     else if (unlikely (compile_options.bounds_check))
Index: m4/unpack.m4
===================================================================
--- m4/unpack.m4	(Revision 175598)
+++ m4/unpack.m4	(Arbeitskopie)
@@ -86,6 +86,9 @@ unpack0_'rtype_code` ('rtype` *ret, const 'rtype`
     {
       /* The front end has signalled that we need to populate the
 	 return array descriptor.  */
+
+      index_type alloc_size;
+
       dim = GFC_DESCRIPTOR_RANK (mask);
       rs = 1;
       for (n = 0; n < dim; n++)
@@ -100,7 +103,13 @@ unpack0_'rtype_code` ('rtype` *ret, const 'rtype`
 	  rs *= extent[n];
 	}
       ret->offset = 0;
-      ret->data = internal_malloc_size (rs * sizeof ('rtype_name`));
+
+      if (unlikely (rs < 1))
+        alloc_size = 1;
+      else
+        alloc_size = rs * sizeof ('rtype_name`);
+
+      ret->data = internal_malloc_size (alloc_size);
     }
   else
     {
Index: m4/ifunction_logical.m4
===================================================================
--- m4/ifunction_logical.m4	(Revision 175598)
+++ m4/ifunction_logical.m4	(Arbeitskopie)
@@ -94,6 +94,7 @@ name`'rtype_qual`_'atype_code (rtype * const restr
 
       if (alloc_size == 0)
 	{
+	  retarray->data = internal_malloc_size (1);
 	  /* Make sure we have a zero-sized array.  */
 	  GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
 	  return;
Index: m4/ifunction.m4
===================================================================
--- m4/ifunction.m4	(Revision 175598)
+++ m4/ifunction.m4	(Arbeitskopie)
@@ -90,6 +90,7 @@ name`'rtype_qual`_'atype_code (rtype * const restr
 
       if (alloc_size == 0)
 	{
+	  retarray->data = internal_malloc_size (1);
 	  /* Make sure we have a zero-sized array.  */
 	  GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
 	  return;
@@ -269,6 +270,7 @@ void
 
       if (alloc_size == 0)
 	{
+	  retarray->data = internal_malloc_size (1);
 	  /* Make sure we have a zero-sized array.  */
 	  GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1);
 	  return;
Index: m4/matmul.m4
===================================================================
--- m4/matmul.m4	(Revision 175598)
+++ m4/matmul.m4	(Arbeitskopie)
@@ -104,6 +104,8 @@ matmul_'rtype_code` ('rtype` * const restrict reta
 
   if (retarray->data == NULL)
     {
+      index_type array_size, alloc_size;
+
       if (GFC_DESCRIPTOR_RANK (a) == 1)
         {
 	  GFC_DIMENSION_SET(retarray->dim[0], 0,
@@ -124,8 +126,14 @@ matmul_'rtype_code` ('rtype` * const restrict reta
 			    GFC_DESCRIPTOR_EXTENT(retarray,0));
         }
 
+      array_size = size0 ((array_t *) retarray);
+
+      if (unlikely (array_size < 1))
+        alloc_size = 1;
+      else
+        alloc_size = sizeof ('rtype_name`) * array_size;
       retarray->data
-	= internal_malloc_size (sizeof ('rtype_name`) * size0 ((array_t *) retarray));
+	= internal_malloc_size (alloc_size);
       retarray->offset = 0;
     }
     else if (unlikely (compile_options.bounds_check))
Index: intrinsics/transpose_generic.c
===================================================================
--- intrinsics/transpose_generic.c	(Revision 175598)
+++ intrinsics/transpose_generic.c	(Arbeitskopie)
@@ -52,6 +52,8 @@ transpose_internal (gfc_array_char *ret, gfc_array
 
   if (ret->data == NULL)
     {
+      index_type alloc_size, array_size;
+
       assert (ret->dtype == source->dtype);
 
       GFC_DIMENSION_SET(ret->dim[0], 0, GFC_DESCRIPTOR_EXTENT(source,1) - 1,
@@ -60,7 +62,14 @@ transpose_internal (gfc_array_char *ret, gfc_array
       GFC_DIMENSION_SET(ret->dim[1], 0, GFC_DESCRIPTOR_EXTENT(source,0) - 1,
 			GFC_DESCRIPTOR_EXTENT(source, 1));
 
-      ret->data = internal_malloc_size (size * size0 ((array_t*)ret));
+
+      array_size = size0 ((array_t*)ret);
+      if (unlikely (array_size < 1))
+	alloc_size = 1;
+      else
+	alloc_size = size * array_size;
+
+      ret->data = internal_malloc_size (alloc_size);
       ret->offset = 0;
     }
   else if (unlikely (compile_options.bounds_check))
Index: intrinsics/unpack_generic.c
===================================================================
--- intrinsics/unpack_generic.c	(Revision 175598)
+++ intrinsics/unpack_generic.c	(Arbeitskopie)
@@ -111,6 +111,8 @@ unpack_internal (gfc_array_char *ret, const gfc_ar
     {
       /* The front end has signalled that we need to populate the
 	 return array descriptor.  */
+      index_type alloc_size;
+
       dim = GFC_DESCRIPTOR_RANK (mask);
       rs = 1;
       for (n = 0; n < dim; n++)
@@ -126,7 +128,13 @@ unpack_internal (gfc_array_char *ret, const gfc_ar
 	  rs *= extent[n];
 	}
       ret->offset = 0;
-      ret->data = internal_malloc_size (rs * size);
+
+      if (unlikely (rs < 1))
+        alloc_size = 1;
+      else
+        alloc_size = rs * size;
+
+      ret->data = internal_malloc_size (alloc_size);
     }
   else
     {

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Janne Blomqvist]

On Wed, Jun 29, 2011 at 00:50, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Hello world,
>
> looking at PR 49479 and other functions in the library made me realize
> there are lots of places where we don't malloc one byte for empty
> arrays.

I'd prefer to add the zero check to runtime/memory.c
(internal_malloc_size), i.e. change

if (size == 0)
  return NULL;

to

if (size == 0)
  size = 1;

Or perhaps even better would be to add that check to get_mem() instead
(as a side effect making internal_malloc_size redundant), thus
matching how memory allocation works in the frontend? Looking at
get_mem() we actually have a small bug here; if size == 0 and malloc
happens to return NULL we treat it as an error.

-- 
Janne Blomqvist

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Thomas Koenig]

Hi Janne,

> I'd prefer to add the zero check to runtime/memory.c
> (internal_malloc_size), i.e. change
>
> if (size == 0)
>    return NULL;
>
> to
>
> if (size == 0)
>    size = 1;
>

Good point.  I have done so in the attached patch, plus removed
all special cases for checking for zero size.

Regression-tested.  OK for trunk?

For 4.6, I would just commit the change to internal_malloc_size
(which would also fix PR 49479), plus the test case for that
PR.

OK?

Regards

	Thomas

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Janne Blomqvist]

On Thu, Jun 30, 2011 at 21:09, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Good point.  I have done so in the attached patch

Seems you forgot to attach it.. ;)

-- 
Janne Blomqvist

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Thomas Koenig]

[-- Attachment #1: Type: text/plain, Size: 2721 bytes --]

Am 01.07.2011 14:34, schrieb Janne Blomqvist:
> On Thu, Jun 30, 2011 at 21:09, Thomas Koenig<tkoenig@netcologne.de>  wrote:
>> Good point.  I have done so in the attached patch
>
> Seems you forgot to attach it.. ;)
>

Oops, I hadn't realized your crystal ball was broken :-)

Is this better?

	Thomas

2011-06-30  Thomas Koenig  <tkoenig@gcc.gnu.org>

         * runtime/memory.c (internal_malloc_size):  If size is zero or
         less, allocate a single byte.
         * m4/pack.m4 (pack_'rtype_code`):  Don't check for zero size
         for the argument of internal_malloc_size.
         * m4/spread.m4 (spread_'rtype_code`):  Likewise.
         * m4/eoshift1.m4 (eoshift1):  Don't allocate twice.  Don't check
         for zero size for the argument of internal_malloc_size.
         * m4/eoshift3.m4: Don't check for zero size for the argument of
         internal_malloc_size.
         * intrinsics/pack_generic.c (pack_internal):  Likewise.
         (pack_s_internal):  Likewise.
         * intrinsics/cshift0.c (cshift0):  Likewise.
         * intrinsics/spread_generic.c (spread_internal):  Likewise.
         * intrinsics/eoshift0.c (eoshift0):  Likewise.
         * intrinsics/eoshift2.c (eoshift2):  Likewise.
         * generated/eoshift1_16.c: Regenerated.
         * generated/eoshift1_4.c: Regenerated.
         * generated/eoshift1_8.c: Regenerated.
         * generated/eoshift3_16.c: Regenerated.
         * generated/eoshift3_4.c: Regenerated.
         * generated/eoshift3_8.c: Regenerated.
         * generated/pack_c10.c: Regenerated.
         * generated/pack_c16.c: Regenerated.
         * generated/pack_c4.c: Regenerated.
         * generated/pack_c8.c: Regenerated.
         * generated/pack_i16.c: Regenerated.
         * generated/pack_i1.c: Regenerated.
         * generated/pack_i2.c: Regenerated.
         * generated/pack_i4.c: Regenerated.
         * generated/pack_i8.c: Regenerated.
         * generated/pack_r10.c: Regenerated.
         * generated/pack_r16.c: Regenerated.
         * generated/pack_r4.c: Regenerated.
         * generated/pack_r8.c: Regenerated.
         * generated/spread_c10.c: Regenerated.
         * generated/spread_c16.c: Regenerated.
         * generated/spread_c4.c: Regenerated.
         * generated/spread_c8.c: Regenerated.
         * generated/spread_i16.c: Regenerated.
         * generated/spread_i1.c: Regenerated.
         * generated/spread_i2.c: Regenerated.
         * generated/spread_i4.c: Regenerated.
         * generated/spread_i8.c: Regenerated.
         * generated/spread_r10.c: Regenerated.
         * generated/spread_r16.c: Regenerated.
         * generated/spread_r4.c: Regenerated.
         * generated/spread_r8.c: Regenerated.

[-- Attachment #2: p4.diff --]
[-- Type: text/x-patch, Size: 6849 bytes --]

Index: runtime/memory.c
===================================================================
--- runtime/memory.c	(Revision 175598)
+++ runtime/memory.c	(Arbeitskopie)
@@ -54,8 +54,8 @@ get_mem (size_t n)
 void *
 internal_malloc_size (size_t size)
 {
-  if (size == 0)
-    return NULL;
+  if (unlikely (size <= 0))
+    size = 1;
 
   return get_mem (size);
 }
Index: m4/pack.m4
===================================================================
--- m4/pack.m4	(Revision 175598)
+++ m4/pack.m4	(Arbeitskopie)
@@ -167,14 +167,12 @@ pack_'rtype_code` ('rtype` *ret, const 'rtype` *ar
 	  GFC_DIMENSION_SET(ret->dim[0], 0, total-1, 1);
 
 	  ret->offset = 0;
+
+	  /* internal_malloc_size allocates a single byte for zero size.  */
+	  ret->data = internal_malloc_size (sizeof ('rtype_name`) * total);
+
 	  if (total == 0)
-	    {
-	      /* In this case, nothing remains to be done.  */
-	      ret->data = internal_malloc_size (1);
-	      return;
-	    }
-	  else
-	    ret->data = internal_malloc_size (sizeof ('rtype_name`) * total);
+	    return;
 	}
       else 
 	{
Index: m4/spread.m4
===================================================================
--- m4/spread.m4	(Revision 175598)
+++ m4/spread.m4	(Arbeitskopie)
@@ -101,13 +101,11 @@ spread_'rtype_code` ('rtype` *ret, const 'rtype` *
 	  GFC_DIMENSION_SET(ret->dim[n], 0, ub, stride);
 	}
       ret->offset = 0;
-      if (rs > 0)
-        ret->data = internal_malloc_size (rs * sizeof('rtype_name`));
-      else
-	{
-	  ret->data = internal_malloc_size (1);
-	  return;
-	}
+
+      /* internal_malloc_size allocates a single byte for zero size.  */
+      ret->data = internal_malloc_size (rs * sizeof('rtype_name`));
+      if (rs <= 0)
+        return;
     }
   else
     {
Index: m4/eoshift1.m4
===================================================================
--- m4/eoshift1.m4	(Revision 175598)
+++ m4/eoshift1.m4	(Arbeitskopie)
@@ -89,7 +89,6 @@ eoshift1 (gfc_array_char * const restrict ret,
     {
       int i;
 
-      ret->data = internal_malloc_size (size * arraysize);
       ret->offset = 0;
       ret->dtype = array->dtype;
       for (i = 0; i < GFC_DESCRIPTOR_RANK (array); i++)
@@ -107,10 +106,8 @@ eoshift1 (gfc_array_char * const restrict ret,
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
 
         }
-      if (arraysize > 0)
-	ret->data = internal_malloc_size (size * arraysize);
-      else
-	ret->data = internal_malloc_size (1);
+      /* internal_malloc_size allocates a single byte for zero size.  */
+      ret->data = internal_malloc_size (size * arraysize);
 
     }
   else if (unlikely (compile_options.bounds_check))
Index: m4/eoshift3.m4
===================================================================
--- m4/eoshift3.m4	(Revision 175598)
+++ m4/eoshift3.m4	(Arbeitskopie)
@@ -108,10 +108,8 @@ eoshift3 (gfc_array_char * const restrict ret,
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
 
         }
-      if (arraysize > 0)
-	ret->data = internal_malloc_size (size * arraysize);
-      else
-	ret->data = internal_malloc_size (1);
+      /* internal_malloc_size allocates a single byte for zero size.  */
+      ret->data = internal_malloc_size (size * arraysize);
 
     }
   else if (unlikely (compile_options.bounds_check))
Index: intrinsics/pack_generic.c
===================================================================
--- intrinsics/pack_generic.c	(Revision 175598)
+++ intrinsics/pack_generic.c	(Arbeitskopie)
@@ -153,14 +153,11 @@ pack_internal (gfc_array_char *ret, const gfc_arra
 	  GFC_DIMENSION_SET(ret->dim[0], 0, total-1, 1);
 
 	  ret->offset = 0;
+	  /* internal_malloc_size allocates a single byte for zero size.  */
+	  ret->data = internal_malloc_size (size * total);
+
 	  if (total == 0)
-	    {
-	      /* In this case, nothing remains to be done.  */
-	      ret->data = internal_malloc_size (1);
-	      return;
-	    }
-	  else
-	    ret->data = internal_malloc_size (size * total);
+	    return;      /* In this case, nothing remains to be done.  */
 	}
       else 
 	{
@@ -523,13 +520,10 @@ pack_s_internal (gfc_array_char *ret, const gfc_ar
 
       ret->offset = 0;
 
+      ret->data = internal_malloc_size (size * total);
+
       if (total == 0)
-	{
-	  ret->data = internal_malloc_size (1);
-	  return;
-	}
-      else
-	ret->data = internal_malloc_size (size * total);
+	return;
     }
 
   rstride0 = GFC_DESCRIPTOR_STRIDE_BYTES(ret,0);
Index: intrinsics/cshift0.c
===================================================================
--- intrinsics/cshift0.c	(Revision 175598)
+++ intrinsics/cshift0.c	(Arbeitskopie)
@@ -79,10 +79,8 @@ cshift0 (gfc_array_char * ret, const gfc_array_cha
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
         }
 
-      if (arraysize > 0)
-	ret->data = internal_malloc_size (size * arraysize);
-      else
-	ret->data = internal_malloc_size (1);
+      /* internal_malloc_size allocates a single byte for zero size.  */
+      ret->data = internal_malloc_size (size * arraysize);
     }
   else if (unlikely (compile_options.bounds_check))
     {
Index: intrinsics/spread_generic.c
===================================================================
--- intrinsics/spread_generic.c	(Revision 175598)
+++ intrinsics/spread_generic.c	(Arbeitskopie)
@@ -100,13 +100,10 @@ spread_internal (gfc_array_char *ret, const gfc_ar
 	  GFC_DIMENSION_SET(ret->dim[n], 0, ub, stride);
 	}
       ret->offset = 0;
-      if (rs > 0)
-        ret->data = internal_malloc_size (rs * size);
-      else
-	{
-	  ret->data = internal_malloc_size (1);
-	  return;
-	}
+      ret->data = internal_malloc_size (rs * size);
+
+      if (rs <= 0)
+	return;
     }
   else
     {
Index: intrinsics/eoshift0.c
===================================================================
--- intrinsics/eoshift0.c	(Revision 175598)
+++ intrinsics/eoshift0.c	(Arbeitskopie)
@@ -86,11 +86,8 @@ eoshift0 (gfc_array_char * ret, const gfc_array_ch
 
         }
 
-      if (arraysize > 0)
-	ret->data = internal_malloc_size (size * arraysize);
-      else
-	ret->data = internal_malloc_size (1);
-
+      /* internal_malloc_size allocates a single byte for zero size.  */
+      ret->data = internal_malloc_size (size * arraysize);
     }
   else if (unlikely (compile_options.bounds_check))
     {
Index: intrinsics/eoshift2.c
===================================================================
--- intrinsics/eoshift2.c	(Revision 175598)
+++ intrinsics/eoshift2.c	(Arbeitskopie)
@@ -91,10 +91,8 @@ eoshift2 (gfc_array_char *ret, const gfc_array_cha
 
 	  GFC_DIMENSION_SET(ret->dim[i], 0, ub, str);
 
-	  if (arraysize > 0)
-	    ret->data = internal_malloc_size (size * arraysize);
-	  else
-	    ret->data = internal_malloc_size (1);
+          /* internal_malloc_size allocates a single byte for zero size.  */
+	  ret->data = internal_malloc_size (size * arraysize);
 
         }
     }

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Janne Blomqvist]

On Fri, Jul 1, 2011 at 21:31, Thomas Koenig <tkoenig@netcologne.de> wrote:
> Is this better?

Index: runtime/memory.c
===================================================================
--- runtime/memory.c	(Revision 175598)
+++ runtime/memory.c	(Arbeitskopie)
@@ -54,8 +54,8 @@ get_mem (size_t n)
 void *
 internal_malloc_size (size_t size)
 {
-  if (size == 0)
-    return NULL;
+  if (unlikely (size <= 0))
+    size = 1;

   return get_mem (size);
 }

Since size_t is unsigned, just test (size == 0).  Otherwise Ok. Thanks
for the patch.




-- 
Janne Blomqvist

Re: [patch, fortran] Always return malloc(1) for empty arrays in the library

[Thomas Koenig]

Hi Janne,


> Since size_t is unsigned, just test (size == 0).  Otherwise Ok. Thanks
> for the patch.

OK.

Ãœbertrage Daten ...........................................
Revision 175880 übertragen.

Thanks for the review!

	Thomas