On 11/22/2011 04:20 PM, Joseph S. Myers wrote:
* If TYPE_SIZE_UNIT if the pointer target type is zero. This could be the
case for empty structures or zero-size arrays (both GNU extensions).
Logically there's nothing wrong with atomic operations on such a zero-size
object (they should evaluate all operands for their side effects and then
need do nothing more). But if you don't want to make them work now, there
should be a sorry () call and a PR for making them work.
I talked to Lawrence about this. To be atomic, every object must have a
distinct address, which is violated with zero-size objects. Within the
language, structs without fields are expanded to a byte. Zero size
objects are a gcc only extension, and since the atomic version of an
object doesn't need to be the same size as the object itself, it is
reasonable to presume that we could make a zero size object a byte in
size and not support zero size. I have some preliminary classes for C++
that pads objects up to the next nearest lock-free size, but I doubt it
is going to get into this release.
His inclination, as is mine, is to make it illegal. I added an error
for that as well.
void f (int n) { int a[n], b[n]; __atomic_load (&a,&b, __ATOMIC_SEQ_CST); }
ICEs in tree_low_cst. C1X doesn't require atomic operations on
variable-length types - arrays can't be _Atomic-qualified and atomic
operations are only required on atomic types - but there should be a
proper error not an ICE.
Yes, I've been occupied with other issues and haven't gotten to working
on testcases to flush out the various illegal conditions. I'll take
care of this one now.
It seems a bad idea to put error_mark_node in the IR for code that has not
generated an error. So maybe you need to change get_atomic_generic_size's
interface so that errors can be unambiguously distinguished from zero-size
objects - and make the comment above that function explicitly say that on
error return it has output an error message.
Now the routine always issues an error if it returns 0.
Finally, the patch needs a testcase added to the testsuite for the case
reported in the PR as ICEing - and for any other problems found and fixed
in the process.
done.
bootstraps on x86_64-unknown-linux-gnu with no new regressions.
Andrew
PR c/51256
c-family
* c-common.c (get_atomic_generic_size): Check for various error
conditions
(resolve_overloaded_atomic_exchange,
resolve_overloaded_atomic_compare_exchange,
resolve_overloaded_atomic_load, resolve_overloaded_atomic_store): Return
error_mark_node for error conditions.
testsuite
* gcc.dg/atomic-pr51256.c: New. Test error conditions.
Index: c-family/c-common.c
===================================================================
*** c-family/c-common.c (revision 181614)
--- c-family/c-common.c (working copy)
*************** get_atomic_generic_size (location_t loc,
*** 9392,9398 ****
n_model = 2;
break;
default:
! return 0;
}
if (VEC_length (tree, params) != n_param)
--- 9392,9398 ----
n_model = 2;
break;
default:
! gcc_unreachable ();
}
if (VEC_length (tree, params) != n_param)
*************** get_atomic_generic_size (location_t loc,
*** 9403,9415 ****
/* Get type of first parameter, and determine its size. */
type_0 = TREE_TYPE (VEC_index (tree, params, 0));
! if (TREE_CODE (type_0) != POINTER_TYPE)
{
! error_at (loc, "argument 1 of %qE must be a pointer type", function);
return 0;
}
size_0 = tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type_0)), 1);
/* Check each other parameter is a pointer and the same size. */
for (x = 0; x < n_param - n_model; x++)
{
--- 9403,9435 ----
/* Get type of first parameter, and determine its size. */
type_0 = TREE_TYPE (VEC_index (tree, params, 0));
! if (TREE_CODE (type_0) != POINTER_TYPE || VOID_TYPE_P (TREE_TYPE (type_0)))
{
! error_at (loc, "argument 1 of %qE must be a non-void pointer type",
! function);
return 0;
}
+
+ /* Types must be compile time constant sizes. */
+ if (TREE_CODE ((TYPE_SIZE_UNIT (TREE_TYPE (type_0)))) != INTEGER_CST)
+ {
+ error_at (loc,
+ "argument 1 of %qE must be a pointer to a constant size type",
+ function);
+ return 0;
+ }
+
size_0 = tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (type_0)), 1);
+ /* Zero size objects are not allowed. */
+ if (size_0 == 0)
+ {
+ error_at (loc,
+ "argument 1 of %qE must be a pointer to a non-zero size object",
+ function);
+ return 0;
+ }
+
/* Check each other parameter is a pointer and the same size. */
for (x = 0; x < n_param - n_model; x++)
{
*************** get_atomic_generic_size (location_t loc,
*** 9445,9451 ****
warning_at (loc, OPT_Winvalid_memory_model,
"invalid memory model argument %d of %qE", x + 1,
function);
- return MEMMODEL_SEQ_CST;
}
}
else
--- 9465,9470 ----
*************** resolve_overloaded_atomic_exchange (loca
*** 9515,9520 ****
--- 9534,9546 ----
tree I_type, I_type_ptr;
int n = get_atomic_generic_size (loc, function, params);
+ /* Size of 0 is an error condition. */
+ if (n == 0)
+ {
+ *new_return = error_mark_node;
+ return true;
+ }
+
/* If not a lock-free size, change to the library generic format. */
if (n != 1 && n != 2 && n != 4 && n != 8 && n != 16)
{
*************** resolve_overloaded_atomic_exchange (loca
*** 9538,9545 ****
/* Convert object pointer to required type. */
p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0);
! VEC_replace (tree, 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);
--- 9564,9570 ----
/* Convert object pointer to required type. */
p0 = build1 (VIEW_CONVERT_EXPR, I_type_ptr, p0);
! VEC_replace (tree, 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);
*************** resolve_overloaded_atomic_compare_exchan
*** 9574,9579 ****
--- 9599,9611 ----
tree I_type, I_type_ptr;
int n = get_atomic_generic_size (loc, function, params);
+ /* Size of 0 is an error condition. */
+ if (n == 0)
+ {
+ *new_return = error_mark_node;
+ return true;
+ }
+
/* If not a lock-free size, change to the library generic format. */
if (n != 1 && n != 2 && n != 4 && n != 8 && n != 16)
{
*************** resolve_overloaded_atomic_load (location
*** 9643,9648 ****
--- 9675,9687 ----
tree I_type, I_type_ptr;
int n = get_atomic_generic_size (loc, function, params);
+ /* Size of 0 is an error condition. */
+ if (n == 0)
+ {
+ *new_return = error_mark_node;
+ return true;
+ }
+
/* If not a lock-free size, change to the library generic format. */
if (n != 1 && n != 2 && n != 4 && n != 8 && n != 16)
{
*************** resolve_overloaded_atomic_store (locatio
*** 9696,9701 ****
--- 9735,9747 ----
tree I_type, I_type_ptr;
int n = get_atomic_generic_size (loc, function, params);
+ /* Size of 0 is an error condition. */
+ if (n == 0)
+ {
+ *new_return = error_mark_node;
+ return true;
+ }
+
/* If not a lock-free size, change to the library generic format. */
if (n != 1 && n != 2 && n != 4 && n != 8 && n != 16)
{
Index: testsuite/gcc.dg/atomic-pr51256.c
===================================================================
*** testsuite/gcc.dg/atomic-pr51256.c (revision 0)
--- testsuite/gcc.dg/atomic-pr51256.c (revision 0)
***************
*** 0 ****
--- 1,47 ----
+ /* Test generic __atomic routines for various errors. */
+ /* { dg-do compile } */
+ /* { dg-require-effective-target sync_int_long } */
+ /* { dg-options "-ansi" } */
+
+ void f1 (void* p)
+ {
+ __atomic_compare_exchange(p, p, p, 0, 0, 0); /* { dg-error "must be a
non-void pointer type" } */
+ }
+
+ void f2 (int n)
+ {
+ int a[n], b[n];
+ __atomic_load (&a, &b, __ATOMIC_SEQ_CST); /* { dg-error "must be a pointer
to a constant size" } */
+ }
+
+ struct s { };
+ void f3 (void)
+ {
+ struct s a,b;
+ __atomic_load (&a, &b, __ATOMIC_SEQ_CST); /* { dg-error "must be a pointer
to a non-zero size" } */
+ }
+
+ void f4 (int a, int b, int c)
+ {
+ __atomic_load (&a, &b, &c, __ATOMIC_SEQ_CST); /* { dg-error "incorrect
number of arguments" } */
+ }
+
+ void f5 (int a, int b)
+ {
+ __atomic_load (&a, b, __ATOMIC_SEQ_CST); /* { dg-error "must be a pointer
type" } */
+ }
+
+ void f6 (int a, char b)
+ {
+ __atomic_load (&a, &b, __ATOMIC_SEQ_CST); /* { dg-error "size mismatch in
argument" } */
+ }
+
+ void f7 (int a, int b)
+ {
+ __atomic_load (&a, &b, 45); /* { dg-warning "invalid memory model argument"
} */
+ }
+
+ void f8 (int a, int b, float c)
+ {
+ __atomic_load (&a, &b, c); /* { dg-error "non-integer memory model
argument" } */
+ }
