I agree that the rules are sometimes tedious to follow (and rebasing
patches just to fix some formatting issues isn't exactly fun). I don't
use git to commit either. My "secret" is to enable highlighting of trailing
whitespaces in vim ("let c_space_errors=1"), but that's of no use to you
I guess.
Thanks.
I just downloaded and installed an Emacs equivalent called redspace.
It does the same thing. Unfortunately, it also has the effect of
highlighting in red the one blank inserted by diff at the beginning
of empty lines. I guess the way to deal with that is to avoid
inserting blank spaces into changes to begin with rather than trying
to remove them from the final patch.
But there's this contrib/check_GNU_style.sh script that you can use,
which points out e.g. 8 spaces -> tabs. Before posting a patch, I always do
'check_GNU_style z.patch'.
That might help.
I use the script on bigger patches. I just haven't trained myself
to use it consistently, for all of them, and each time I post a new
revision of a patch for review. (The script also complains about
tests where following the convention isn't always possible -- e.g.,
lines over 80 characters are unavoidable due to dg-error directives,
and periods can't be preceded by a space when they are part of
regular expressions in dg- directives..)
But what I'm hoping to find is a fully automated way of dealing
with this so I don't have to think about and waste other people's
time pointing it out. (Ideally, it would be automated for everyone
so that none of us have to worry about it. Perhaps a post-commit
hook could be put in place for this.)
Index: gcc/c-family/c-common.c
...
This change is unrelated.
Yes, sorry about that. It's yet another weakness in my workflow
(using the same local copy to test multiple unrelated changes).
A new patch is attached.
Martin
gcc/ChangeLog:
2016-01-04 Martin Sebor <mse...@redhat.com>
PR c/68966
* doc/extend.texi (__atomic Builtins, __sync Builtins): Document
constraint on the type of arguments.
gcc/c-family/ChangeLog:
2016-01-04 Martin Sebor <mse...@redhat.com>
PR c/68966
* c-common.c (sync_resolve_size): Reject first argument when it's
a pointer to _Bool.
gcc/testsuite/ChangeLog:
2016-01-04 Martin Sebor <mse...@redhat.com>
PR c/68966
* gcc.dg/atomic-fetch-bool.c: New test.
* gcc.dg/sync-fetch-bool.c: Same.
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi (revision 232069)
+++ gcc/doc/extend.texi (working copy)
@@ -9238,6 +9238,9 @@
@{ tmp = *ptr; *ptr = ~(tmp & value); return tmp; @} // nand
@end smallexample
+The object pointed to by the first argument must be of integer or pointer
+type. It must not be a Boolean type.
+
@emph{Note:} GCC 4.4 and later implement @code{__sync_fetch_and_nand}
as @code{*ptr = ~(tmp & value)} instead of @code{*ptr = ~tmp & value}.
@@ -9261,6 +9264,9 @@
@{ *ptr = ~(*ptr & value); return *ptr; @} // nand
@end smallexample
+The same constraints on arguments apply as for the corresponding
+@code{__sync_op_and_fetch} built-in functions.
+
@emph{Note:} GCC 4.4 and later implement @code{__sync_nand_and_fetch}
as @code{*ptr = ~(*ptr & value)} instead of
@code{*ptr = ~*ptr & value}.
@@ -9507,13 +9513,14 @@
@deftypefnx {Built-in Function} @var{type} __atomic_or_fetch (@var{type} *ptr, @var{type} val, int memorder)
@deftypefnx {Built-in Function} @var{type} __atomic_nand_fetch (@var{type} *ptr, @var{type} val, int memorder)
These built-in functions perform the operation suggested by the name, and
-return the result of the operation. That is,
+return the result of the operation. That is,
@smallexample
@{ *ptr @var{op}= val; return *ptr; @}
@end smallexample
-All memory orders are valid.
+The object pointed to by the first argument must be of integer or pointer
+type. It must not be a Boolean type. All memory orders are valid.
@end deftypefn
@@ -9530,7 +9537,8 @@
@{ tmp = *ptr; *ptr @var{op}= val; return tmp; @}
@end smallexample
-All memory orders are valid.
+The same constraints on arguments apply as for the corresponding
+@code{__atomic_op_fetch} built-in functions. All memory orders are valid.
@end deftypefn
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c (revision 232069)
+++ gcc/c-family/c-common.c (working copy)
@@ -10657,11 +10657,16 @@
/* A helper function for resolve_overloaded_builtin in resolving the
overloaded __sync_ builtins. Returns a positive power of 2 if the
first operand of PARAMS is a pointer to a supported data type.
- Returns 0 if an error is encountered. */
+ Returns 0 if an error is encountered.
+ FETCH is true when FUNCTION is one of the _FETCH_OP_ or _OP_FETCH_
+ built-ins. */
static int
-sync_resolve_size (tree function, vec<tree, va_gc> *params)
+sync_resolve_size (tree function, vec<tree, va_gc> *params, bool fetch)
{
+ /* Type of the argument. */
+ tree argtype;
+ /* Type the argument points to. */
tree type;
int size;
@@ -10671,7 +10676,7 @@
return 0;
}
- type = TREE_TYPE ((*params)[0]);
+ argtype = type = TREE_TYPE ((*params)[0]);
if (TREE_CODE (type) == ARRAY_TYPE)
{
/* Force array-to-pointer decay for C++. */
@@ -10686,12 +10691,16 @@
if (!INTEGRAL_TYPE_P (type) && !POINTER_TYPE_P (type))
goto incompatible;
+ if (fetch && TREE_CODE (type) == BOOLEAN_TYPE)
+ goto incompatible;
+
size = tree_to_uhwi (TYPE_SIZE_UNIT (type));
if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16)
return size;
incompatible:
- error ("incompatible type for argument %d of %qE", 1, function);
+ error ("operand type %qT is incompatible with argument %d of %qE",
+ argtype, 1, function);
return 0;
}
@@ -11250,6 +11259,11 @@
vec<tree, va_gc> *params)
{
enum built_in_function orig_code = DECL_FUNCTION_CODE (function);
+
+ /* Is function one of the _FETCH_OP_ or _OP_FETCH_ built-ins?
+ Those are not valid to call with a pointer to _Bool (or C++ bool)
+ and so must be rejected. */
+ bool fetch_op = true;
bool orig_format = true;
tree new_return = NULL_TREE;
@@ -11325,6 +11339,10 @@
case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
case BUILT_IN_ATOMIC_LOAD_N:
case BUILT_IN_ATOMIC_STORE_N:
+ {
+ fetch_op = false;
+ /* Fallthrough to further processing. */
+ }
case BUILT_IN_ATOMIC_ADD_FETCH_N:
case BUILT_IN_ATOMIC_SUB_FETCH_N:
case BUILT_IN_ATOMIC_AND_FETCH_N:
@@ -11358,7 +11376,16 @@
case BUILT_IN_SYNC_LOCK_TEST_AND_SET_N:
case BUILT_IN_SYNC_LOCK_RELEASE_N:
{
- int n = sync_resolve_size (function, params);
+ /* The following are not _FETCH_OPs and must be accepted with
+ pointers to _Bool (or C++ bool). */
+ if (fetch_op)
+ fetch_op =
+ (orig_code != BUILT_IN_SYNC_BOOL_COMPARE_AND_SWAP_N
+ && orig_code != BUILT_IN_SYNC_VAL_COMPARE_AND_SWAP_N
+ && orig_code != BUILT_IN_SYNC_LOCK_TEST_AND_SET_N
+ && orig_code != BUILT_IN_SYNC_LOCK_RELEASE_N);
+
+ int n = sync_resolve_size (function, params, fetch_op);
tree new_function, first_param, result;
enum built_in_function fncode;
Index: gcc/testsuite/gcc.dg/atomic-fetch-bool.c
===================================================================
--- gcc/testsuite/gcc.dg/atomic-fetch-bool.c (revision 0)
+++ gcc/testsuite/gcc.dg/atomic-fetch-bool.c (working copy)
@@ -0,0 +1,64 @@
+/* PR c/68966 - atomic_fetch_* on atomic_bool not diagnosed
+ Test to verify that calls to __atomic_fetch_op funcions with a _Bool
+ argument are rejected. This is necessary because GCC expects that
+ all initialized _Bool objects have a specific representation and
+ allowing atomic operations to change it would break the invariant. */
+/* { dg-do compile } */
+/* { dg-options "-pedantic-errors -std=c11" } */
+
+
+void test_atomic_bool (_Atomic _Bool *a)
+{
+ enum { SEQ_CST = __ATOMIC_SEQ_CST };
+
+ __atomic_fetch_add (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_add." } */
+ __atomic_fetch_sub (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_sub." } */
+ __atomic_fetch_and (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_and." } */
+ __atomic_fetch_xor (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_xor." } */
+ __atomic_fetch_or (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_or." } */
+ __atomic_fetch_nand (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_nand." } */
+
+ __atomic_add_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_add_fetch." } */
+ __atomic_sub_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_sub_fetch." } */
+ __atomic_and_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_and_fetch." } */
+ __atomic_xor_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_xor_fetch." } */
+ __atomic_or_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_or_fetch." } */
+ __atomic_nand_fetch (a, 1, SEQ_CST); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_nand_fetch." } */
+
+ /* The following are valid and must be accepted. */
+ _Bool val = 0, ret = 0;
+ __atomic_exchange (a, &val, &ret, SEQ_CST);
+ __atomic_exchange_n (a, val, SEQ_CST);
+ __atomic_compare_exchange (a, &val, &ret, !1, SEQ_CST, SEQ_CST);
+ __atomic_compare_exchange_n (a, &val, ret, !1, SEQ_CST, SEQ_CST);
+ __atomic_test_and_set (a, SEQ_CST);
+ __atomic_clear (a, SEQ_CST);
+}
+
+void test_bool (_Bool *b)
+{
+ enum { SEQ_CST = __ATOMIC_SEQ_CST };
+
+ __atomic_fetch_add (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_add." } */
+ __atomic_fetch_sub (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_sub." } */
+ __atomic_fetch_and (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_and." } */
+ __atomic_fetch_xor (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_xor." } */
+ __atomic_fetch_or (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_or." } */
+ __atomic_fetch_nand (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_fetch_nand." } */
+
+ __atomic_add_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_add_fetch." } */
+ __atomic_sub_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_sub_fetch." } */
+ __atomic_and_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_and_fetch." } */
+ __atomic_xor_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_xor_fetch." } */
+ __atomic_or_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_or_fetch." } */
+ __atomic_nand_fetch (b, 1, SEQ_CST); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__atomic_nand_fetch." } */
+
+ /* The following are valid and must be accepted. */
+ _Bool val = 0, ret = 0;
+ __atomic_exchange (b, &val, &ret, SEQ_CST);
+ __atomic_exchange_n (b, val, SEQ_CST);
+ __atomic_compare_exchange (b, &val, &ret, !1, SEQ_CST, SEQ_CST);
+ __atomic_compare_exchange_n (b, &val, ret, !1, SEQ_CST, SEQ_CST);
+ __atomic_test_and_set (b, SEQ_CST);
+ __atomic_clear (b, SEQ_CST);
+}
Index: gcc/testsuite/gcc.dg/sync-fetch-bool.c
===================================================================
--- gcc/testsuite/gcc.dg/sync-fetch-bool.c (revision 0)
+++ gcc/testsuite/gcc.dg/sync-fetch-bool.c (working copy)
@@ -0,0 +1,54 @@
+/* PR c/68966 - atomic_fetch_* on atomic_bool not diagnosed
+ Test to verify that calls to __sync_fetch_op funcions with a _Bool
+ argument are rejected. This is necessary because GCC expects that
+ all initialized _Bool objects have a specific representation and
+ allowing atomic operations to change it would break the invariant. */
+/* { dg-do compile } */
+/* { dg-options "-pedantic-errors -std=c11" } */
+
+
+void test_sync_atomic_bool (_Atomic _Bool *a)
+{
+ __sync_fetch_and_add (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_add." } */
+ __sync_fetch_and_sub (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_sub." } */
+ __sync_fetch_and_and (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_and." } */
+ __sync_fetch_and_xor (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_xor." } */
+ __sync_fetch_and_or (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_or." } */
+ __sync_fetch_and_nand (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_nand." } */
+
+ __sync_add_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_add_and_fetch." } */
+ __sync_sub_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_sub_and_fetch." } */
+ __sync_and_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_and_and_fetch." } */
+ __sync_xor_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_xor_and_fetch." } */
+ __sync_or_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_or_and_fetch." } */
+ __sync_nand_and_fetch (a, 1); /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_nand_and_fetch." } */
+
+ /* The following are valid and must be accepted. */
+ __sync_bool_compare_and_swap (a, 0, 1);
+ __sync_val_compare_and_swap (a, 0, 1);
+ __sync_lock_test_and_set (a, 1);
+ __sync_lock_release (a);
+}
+
+void test_sync_bool (_Bool *b)
+{
+ __sync_fetch_and_add (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_add." } */
+ __sync_fetch_and_sub (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_sub." } */
+ __sync_fetch_and_and (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_and." } */
+ __sync_fetch_and_xor (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_xor." } */
+ __sync_fetch_and_or (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_or." } */
+ __sync_fetch_and_nand (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_nand." } */
+
+ __sync_add_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_add_and_fetch." } */
+ __sync_sub_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_sub_and_fetch." } */
+ __sync_and_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_and_and_fetch." } */
+ __sync_xor_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_xor_and_fetch." } */
+ __sync_or_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_or_and_fetch." } */
+ __sync_nand_and_fetch (b, 1); /* { dg-error "operand type ._Bool \\*. is incompatible with argument 1 of .__sync_nand_and_fetch." } */
+
+ /* The following are valid and must be accepted. */
+ __sync_bool_compare_and_swap (b, 0, 1);
+ __sync_val_compare_and_swap (b, 0, 1);
+ __sync_lock_test_and_set (b, 1);
+ __sync_lock_release (b);
+}
