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 <bur...@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 <bur...@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;
}
