>> The following fixes PR63152 zeroing the data field only for allocatables, >> not pointers. The benefit of the patch is a >small speedup, and it avoids >> that code starts to rely on behavior that is undefined in the standard. With >> this patch, >something like >> >> INTEGER, DIMENSION(:), POINTER :: foo >> IF (ASSOCIATED(foo)) ... >> >> will be detected by valgrind as undefined behavior. > >The code you touch is exercised in four different cases, as far as I can see >from the assert earlier in the function: > > gcc_assert (sym->attr.pointer || sym->attr.allocatable || sym_has_alloc_comp > || has_finalizer); > >So do we want to test (sym->attr.allocatable), or (!sym->attr.pointer)? Or, >asked another way: should we NULLIFY in the case of sym_has_alloc_comp || >has_finalizer?
Hi FX, thanks for your good question. I think it is equivalent, as it seems that GFC_DESCRIPTOR_TYPE_P (type) implies either sym->attr.allocatable or sym->attr.pointer. To check, I rank a check-fortran with the explicit patch below, and this made no difference. Code gen for a number of additional testcases involving alloc_comp and finalizers looked good as well. So, I think the original patch is still fine. Joost Index: trans-array.c =================================================================== --- trans-array.c (revision 215373) +++ trans-array.c (working copy) @@ -8647,9 +8647,18 @@ gfc_trans_deferred_array (gfc_symbol * s type = TREE_TYPE (descriptor); } - /* NULLIFY the data pointer. */ + /* NULLIFY the data pointer, for non-saved allocatables. */ if (GFC_DESCRIPTOR_TYPE_P (type) && !sym->attr.save) - gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node); + { + if (sym->attr.allocatable) + { + gfc_conv_descriptor_data_set (&init, descriptor, null_pointer_node); + } + else + { + if (!sym->attr.pointer) gcc_unreachable (); + } + } gfc_restore_backend_locus (&loc); gfc_init_block (&cleanup);