[PATCH][C] Avoid reading from FUNCTION_DECL with atomics

[PATCH][C] Avoid reading from FUNCTION_DECL with atomics

[Richard Biener]

The following avoids creating IL that accesses a FUNCTION_DECLs memory
directly rather than indirectly through an address based on it.

Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

Thanks,
Richard.

2016-06-13  Richard Biener  <rguenther@suse.de>

	PR c/71514
	c-family/
	* c-common.c (resolve_overloaded_atomic_exchange): Convert
	the pointer and dereference instead of dereferencing and
	view-converting.
	(resolve_overloaded_atomic_compare_exchange): Likewise.
	(resolve_overloaded_atomic_store): Likewise.

	* gcc.dg/torture/pr71514.c: New testcase.

Index: gcc/c-family/c-common.c
===================================================================
*** gcc/c-family/c-common.c	(revision 237372)
--- gcc/c-family/c-common.c	(working copy)
*************** resolve_overloaded_atomic_exchange (loca
*** 11129,11136 ****
    p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0);
    (*params)[0] = p0; 
    /* Convert new value to required type, and dereference it.  */
    p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR);
-   p1 = build1 (VIEW_CONVERT_EXPR, I_type, p1);
    (*params)[1] = p1;
  
    /* Move memory model to the 3rd position, and end param list.  */
--- 11129,11136 ----
    p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0);
    (*params)[0] = p0; 
    /* Convert new value to required type, and dereference it.  */
+   p1 = fold_convert_loc (loc, I_type_ptr, p1);
    p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR);
    (*params)[1] = p1;
  
    /* Move memory model to the 3rd position, and end param list.  */
*************** resolve_overloaded_atomic_compare_exchan
*** 11209,11216 ****
    (*params)[1] = p1;
  
    /* Convert desired value to required type, and dereference it.  */
    p2 = build_indirect_ref (loc, p2, RO_UNARY_STAR);
-   p2 = build1 (VIEW_CONVERT_EXPR, I_type, p2);
    (*params)[2] = p2;
  
    /* The rest of the parameters are fine. NULL means no special return value
--- 11209,11216 ----
    (*params)[1] = p1;
  
    /* Convert desired value to required type, and dereference it.  */
+   p2 = fold_convert_loc (loc, I_type_ptr, p2);
    p2 = build_indirect_ref (loc, p2, RO_UNARY_STAR);
    (*params)[2] = p2;
  
    /* The rest of the parameters are fine. NULL means no special return value
*************** resolve_overloaded_atomic_store (locatio
*** 11329,11336 ****
    (*params)[0] = p0;
  
    /* Convert new value to required type, and dereference it.  */
    p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR);
-   p1 = build1 (VIEW_CONVERT_EXPR, I_type, p1);
    (*params)[1] = p1;
    
    /* The memory model is in the right spot already. Return is void.  */
--- 11329,11336 ----
    (*params)[0] = p0;
  
    /* Convert new value to required type, and dereference it.  */
+   p1 = fold_convert_loc (loc, I_type_ptr, p1);
    p1 = build_indirect_ref (loc, p1, RO_UNARY_STAR);
    (*params)[1] = p1;
    
    /* The memory model is in the right spot already. Return is void.  */
Index: gcc/testsuite/gcc.dg/torture/pr71514.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/pr71514.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/pr71514.c	(working copy)
***************
*** 0 ****
--- 1,11 ----
+ /* { dg-do compile } */
+ 
+ void foo () {}
+ 
+ char a, b; 
+ 
+ int main ()
+ {
+   __atomic_exchange (&a, &foo, &b, __ATOMIC_RELAXED);
+   return 0; 
+ }

Re: [PATCH][C] Avoid reading from FUNCTION_DECL with atomics

[Jakub Jelinek]

On Mon, Jun 13, 2016 at 01:25:35PM +0200, Richard Biener wrote:
> The following avoids creating IL that accesses a FUNCTION_DECLs memory
> directly rather than indirectly through an address based on it.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu, ok for trunk?

I think the problem is that for these generic builtins we perform no sanity
checking, except for checking TYPE_SIZE_UNIT equality.  But as you show
in the PR, even that doesn't work, as while we check for VLAs on the first
argument, we don't check for that on the second and following arguments.

The question is what all should we reject.

We accept:

void foo (void);
void bar (void);
void baz (void);
void
test (void)
{
  __atomic_exchange (&foo, &bar, &baz, __ATOMIC_RELAXED);
}

which IMHO we definitely should not, what does it mean to exchange
functions?
So, at least diagnose if any of the arguments is pointer to
FUNCTION_TYPE/METHOD_TYPE and handle gracefully VLAs in 2nd+ argument.

Should we perform some further type checking though, like e.g.
complain if one pointer is pointer to integral type and another to
floating, or one to struct, another to union, or do we just keep the
builtins very forgiving and assume that on the C++ side the templates
make sure the arguments are type compatible and for C _Atomic handling is
done differently anyway?

	Jakub