diff mbox

Fix PR63152

Message ID 908103EDB4893A42920B21D3568BFD9315105D12@MBX13.d.ethz.ch
State New
Headers show

Commit Message

VandeVondele Joost Sept. 19, 2014, 7:12 a.m. UTC 
>> 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

Comments

FX Coudert Sept. 19, 2014, 8:01 a.m. UTC | #1 
> 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.

OK to commit, then. Thanks for the thorough answer to my question.

FX
diff mbox

Patch

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);