[Patch, Fortran] PR53521 - Fix size == 0 handling with reallocate

[Tobias Burnus <burnus at net-b dot de>]

Dear all,

gfortran was producing the following code:

  res = realloc (mem, size)
  if (size == 0)
    res = NULL;
  ...
  if (res != 0)
    free (res)

The problem is that "realloc (..., 0)" does not have to produce a NULL 
pointer as result.


While in "valgrind" one only gets the warning that "0 bytes in <n> 
blocks" where not freed, looking at "top", one sees an excessive use of 
memory.

That seems to cause crashes in CP2K. At least the valgrind issue is a 
GCC 4.3 to 4.8 regression.


Without the patch, gfortran produces:
--------------------------------------------------------------------
      D.1888 = (integer(kind=4)[0] * restrict) __builtin_realloc ((void 
*) atmp.0.data, D.1887);
      if (D.1888 == 0B && D.1887 != 0)
        {
          _gfortran_os_error (&"Allocation would exceed memory 
limit"[1]{lb: 1 sz: 1});
        }
      if (D.1887 == 0)
        {
          D.1888 = 0B;
        }
      atmp.0.data = (void * restrict) D.1888;
--------------------------------------------------------------------


I see two possibilities:


1) FIRST approach: manual freeing/NULL setting; that's the closed what 
is currently done. (Note: It calls free unconditionally; "free(0)" is 
optimized away by the middle end.
(Note: That requires an update of the "free" count in 
gfortran.dg/allocatable_function_4.f90)
--------------------------------------------------------------------
      if (D.1887 == 0)
        {
          __builtin_free ((void *) atmp.0.data);
          D.1888 = 0B;
        }
      else
        {
          D.1888 = (integer(kind=4)[0] * restrict) __builtin_realloc 
((void *) atmp.0.data, D.1887);
          if (D.1888 == 0B && D.1887 != 0)
            {
              _gfortran_os_error (&"Allocation would exceed memory 
limit"[1]{lb: 1 sz: 1});
            }
        }
      atmp.0.data = (void * restrict) D.1888;
--------------------------------------------------------------------


2) SECOND approach: Simply removing the NULL setting and just using the 
result which realloc returns; if not NULL, the free() will properly 
handle it.
--------------------------------------------------------------------
      D.1888 = (integer(kind=4)[0] * restrict) __builtin_realloc ((void 
*) atmp.0.data, D.1887);
      if (D.1888 == 0B && D.1887 != 0)
        {
          _gfortran_os_error (&"Allocation would exceed memory 
limit"[1]{lb: 1 sz: 1});
        }
      atmp.0.data = (void * restrict) D.1888;
--------------------------------------------------------------------


Both patches have been tested with Joost's example and the test suite.
Which version do you prefer? OK for the trunk?

I like the second version more. (And I would even consider to get rid of 
the os_error.)

Tobias
2012-05-31  Tobias Burnus  <burnus@net-b.de>

	PR fortran/53521
	* trans.c (gfc_deallocate_scalar_with_status): Properly
	handle the case size == 0.

diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 5d6e6ef..d53112f 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -1126,20 +1126,24 @@ gfc_deallocate_scalar_with_status (tree pointer, tree status, bool can_fail,
 void *
 internal_realloc (void *mem, size_t size)
 {
+  if (size == 0)
+    {
+      free (mem);
+      return NULL;
+    }
+
   res = realloc (mem, size);
   if (!res && size != 0)
     _gfortran_os_error ("Allocation would exceed memory limit");
 
-  if (size == 0)
-    return NULL;
-
   return res;
 }  */
 tree
 gfc_call_realloc (stmtblock_t * block, tree mem, tree size)
 {
-  tree msg, res, nonzero, zero, null_result, tmp;
+  tree msg, res, cond, null_result, tmp;
   tree type = TREE_TYPE (mem);
+  stmtblock_t nonzero, zero;
 
   size = gfc_evaluate_now (size, block);
 
@@ -1149,17 +1153,20 @@ gfc_call_realloc (stmtblock_t * block, tree mem, tree size)
   /* Create a variable to hold the result.  */
   res = gfc_create_var (type, NULL);
 
+  gfc_init_block (&nonzero);
+  gfc_init_block (&zero);
+
   /* Call realloc and check the result.  */
   tmp = build_call_expr_loc (input_location,
 			 builtin_decl_explicit (BUILT_IN_REALLOC), 2,
 			 fold_convert (pvoid_type_node, mem), size);
-  gfc_add_modify (block, res, fold_convert (type, tmp));
+  gfc_add_modify (&nonzero, res, fold_convert (type, tmp));
   null_result = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node,
 				 res, build_int_cst (pvoid_type_node, 0));
-  nonzero = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, size,
+  cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, size,
 			     build_int_cst (size_type_node, 0));
   null_result = fold_build2_loc (input_location, TRUTH_AND_EXPR, boolean_type_node,
-				 null_result, nonzero);
+				 null_result, cond);
   msg = gfc_build_addr_expr (pchar_type_node, gfc_build_localized_cstring_const
 			     ("Allocation would exceed memory limit"));
   tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node,
@@ -1167,15 +1174,22 @@ gfc_call_realloc (stmtblock_t * block, tree mem, tree size)
 			 build_call_expr_loc (input_location,
 					      gfor_fndecl_os_error, 1, msg),
 			 build_empty_stmt (input_location));
-  gfc_add_expr_to_block (block, tmp);
+  gfc_add_expr_to_block (&nonzero, tmp);
 
   /* if (size == 0) then the result is NULL.  */
+  tmp = build_call_expr_loc (input_location,
+			      builtin_decl_explicit (BUILT_IN_FREE),
+			      1, fold_convert (pvoid_type_node, mem));
+  gfc_add_expr_to_block (&zero, tmp);
   tmp = fold_build2_loc (input_location, MODIFY_EXPR, type, res,
 			 build_int_cst (type, 0));
-  zero = fold_build1_loc (input_location, TRUTH_NOT_EXPR, boolean_type_node,
-			  nonzero);
-  tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, zero, tmp,
-			 build_empty_stmt (input_location));
+
+  gfc_add_expr_to_block (&zero, tmp);
+  cond = fold_build1_loc (input_location, TRUTH_NOT_EXPR, boolean_type_node,
+			  cond);
+  tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond,
+			 gfc_finish_block (&zero),
+			 gfc_finish_block (&nonzero));
   gfc_add_expr_to_block (block, tmp);
 
   return res;

2012-05-31  Tobias Burnus  <burnus@net-b.de>

	PR fortran/53521
	* trans.c (gfc_deallocate_scalar_with_status): Properly
	handle the case size == 0.

diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c
index 5d6e6ef..3313be9 100644
--- a/gcc/fortran/trans.c
+++ b/gcc/fortran/trans.c
@@ -1130,15 +1130,12 @@ internal_realloc (void *mem, size_t size)
   if (!res && size != 0)
     _gfortran_os_error ("Allocation would exceed memory limit");
 
-  if (size == 0)
-    return NULL;
-
   return res;
 }  */
 tree
 gfc_call_realloc (stmtblock_t * block, tree mem, tree size)
 {
-  tree msg, res, nonzero, zero, null_result, tmp;
+  tree msg, res, nonzero, null_result, tmp;
   tree type = TREE_TYPE (mem);
 
   size = gfc_evaluate_now (size, block);
@@ -1169,15 +1166,6 @@ gfc_call_realloc (stmtblock_t * block, tree mem, tree size)
 			 build_empty_stmt (input_location));
   gfc_add_expr_to_block (block, tmp);
 
-  /* if (size == 0) then the result is NULL.  */
-  tmp = fold_build2_loc (input_location, MODIFY_EXPR, type, res,
-			 build_int_cst (type, 0));
-  zero = fold_build1_loc (input_location, TRUTH_NOT_EXPR, boolean_type_node,
-			  nonzero);
-  tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, zero, tmp,
-			 build_empty_stmt (input_location));
-  gfc_add_expr_to_block (block, tmp);
-
   return res;
 }