diff mbox

correct atomic_compare_exchange_n return type (c++/71675)

Message ID 57729E11.9030405@gmail.com
State New
Headers show

Commit Message

Martin Sebor June 28, 2016, 3:56 p.m. UTC
c++/71675 - __atomic_compare_exchange_n returns wrong type for
typed enum

__atomic_compare_exchange_n is documented to return bool but when
its operands are of one of the character types or derived from
it (such as a class enum in C++) it converts the bool result to
that type.  The attached patch corrects that to prevent this
conversion, analogously to what's already being done for
__sync_bool_compare_and_swap.

Martin

Comments

Jeff Law July 13, 2016, 10:28 p.m. UTC | #1
On 06/28/2016 09:56 AM, Martin Sebor wrote:
> c++/71675 - __atomic_compare_exchange_n returns wrong type for
> typed enum
>
> __atomic_compare_exchange_n is documented to return bool but when
> its operands are of one of the character types or derived from
> it (such as a class enum in C++) it converts the bool result to
> that type.  The attached patch corrects that to prevent this
> conversion, analogously to what's already being done for
> __sync_bool_compare_and_swap.
>
> Martin
>
> gcc-71675.diff
>
>
> PR c++/71675 - __atomic_compare_exchange_n returns wrong type for typed enum
>
> gcc/c-family/ChangeLog:
> 2016-06-28  Martin Sebor  <msebor@redhat.com>
>
> 	PR c++/71675
> 	* c-common.c (resolve_overloaded_builtin): Avoid converting
> 	__atomic_compare_exchange_n return type to that of what its
> 	first argument points to.
>
> gcc/testsuite/ChangeLog:
> 2016-06-28  Martin Sebor  <msebor@redhat.com>
>
> 	PR c++/71675
> 	* g++.dg/ext/atomic-3.C: New test.
> 	* gcc.dg/atomic/pr71675.c: New test.
OK.  Thanks.
jeff
Renlin Li July 25, 2016, 11:03 a.m. UTC | #2
Hi Martin,

I observed the following error:

ERROR: gcc.dg/atomic/pr71675.c   -O2 -flto -fuse-linker-plugin 
-fno-fat-lto-objects : syntax error in target selector "target c11" for 
" dg-do 3 compile { target c11 } "

It seems we don't have a c11 effective target check available
in dejagnu target-supports.exp.

Thanks,
Renlin

> diff --git a/gcc/testsuite/gcc.dg/atomic/pr71675.c b/gcc/testsuite/gcc.dg/atomic/pr71675.c
> new file mode 100644
> index 0000000..0e344ac
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/atomic/pr71675.c
> @@ -0,0 +1,32 @@
> +/* PR c++/71675 - __atomic_compare_exchange_n returns wrong type for typed enum
> + */
> +/* { dg-do compile { target c11 } } */
Jeff Law July 25, 2016, 5:56 p.m. UTC | #3
On 07/25/2016 05:03 AM, Renlin Li wrote:
> Hi Martin,
>
> I observed the following error:
>
> ERROR: gcc.dg/atomic/pr71675.c   -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects : syntax error in target selector "target c11" for
> " dg-do 3 compile { target c11 } "
>
> It seems we don't have a c11 effective target check available
> in dejagnu target-supports.exp.
ISTM like we should either add a c11 effective target check, or force 
the compiler into c11 mode via -std=c11.

Jeff
Jason Merrill July 25, 2016, 6:06 p.m. UTC | #4
On Mon, Jul 25, 2016 at 1:56 PM, Jeff Law <law@redhat.com> wrote:
> On 07/25/2016 05:03 AM, Renlin Li wrote:
>>
>> Hi Martin,
>>
>> I observed the following error:
>>
>> ERROR: gcc.dg/atomic/pr71675.c   -O2 -flto -fuse-linker-plugin
>> -fno-fat-lto-objects : syntax error in target selector "target c11" for
>> " dg-do 3 compile { target c11 } "
>>
>> It seems we don't have a c11 effective target check available
>> in dejagnu target-supports.exp.
>
> ISTM like we should either add a c11 effective target check, or force the
> compiler into c11 mode via -std=c11.

I think the C testsuite also doesn't run in multiple modes like C++,
so adding -std=c11 is probably the better choice.

Jason
Martin Sebor July 26, 2016, 8:50 p.m. UTC | #5
On 07/25/2016 05:03 AM, Renlin Li wrote:
> Hi Martin,
>
> I observed the following error:
>
> ERROR: gcc.dg/atomic/pr71675.c   -O2 -flto -fuse-linker-plugin
> -fno-fat-lto-objects : syntax error in target selector "target c11" for
> " dg-do 3 compile { target c11 } "
>
> It seems we don't have a c11 effective target check available
> in dejagnu target-supports.exp.

Thanks for pointing this out.  I had trouble getting this or any
of the other atomic tests to run on their own with make check-c
RUNTESTFLAGS='atomic.exp'  They simply don't.  I didn't notice
a failure in the full test suite run and since the test compiled
successfully outside DejaGnu I didn't take the time to understand
why it wouldn't run on its own or with the rest of the atomic
subset, or to confirm that the c11 target selector was understood.
Looks like I managed to run into not one but two gotchas with
this trivial test.

I've committed r238766 and replaced the unsupported target selector
with a dg-options directive as Jeff and Jason suggested.

Martin

>
> Thanks,
> Renlin
>
>> diff --git a/gcc/testsuite/gcc.dg/atomic/pr71675.c
>> b/gcc/testsuite/gcc.dg/atomic/pr71675.c
>> new file mode 100644
>> index 0000000..0e344ac
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/atomic/pr71675.c
>> @@ -0,0 +1,32 @@
>> +/* PR c++/71675 - __atomic_compare_exchange_n returns wrong type for
>> typed enum
>> + */
>> +/* { dg-do compile { target c11 } } */
diff mbox

Patch

PR c++/71675 - __atomic_compare_exchange_n returns wrong type for typed enum

gcc/c-family/ChangeLog:
2016-06-28  Martin Sebor  <msebor@redhat.com>

	PR c++/71675
	* c-common.c (resolve_overloaded_builtin): Avoid converting
	__atomic_compare_exchange_n return type to that of what its
	first argument points to.

gcc/testsuite/ChangeLog:
2016-06-28  Martin Sebor  <msebor@redhat.com>

	PR c++/71675
	* g++.dg/ext/atomic-3.C: New test.
	* gcc.dg/atomic/pr71675.c: New test.

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 524fbc5..e4e6d5f 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -11503,7 +11503,8 @@  resolve_overloaded_builtin (location_t loc, tree function,
 	  return result;
 	if (orig_code != BUILT_IN_SYNC_BOOL_COMPARE_AND_SWAP_N
 	    && orig_code != BUILT_IN_SYNC_LOCK_RELEASE_N
-	    && orig_code != BUILT_IN_ATOMIC_STORE_N)
+	    && orig_code != BUILT_IN_ATOMIC_STORE_N
+	    && orig_code != BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N)
 	  result = sync_resolve_return (first_param, result, orig_format);
 
 	if (fetch_op)
diff --git a/gcc/testsuite/g++.dg/ext/atomic-3.C b/gcc/testsuite/g++.dg/ext/atomic-3.C
new file mode 100644
index 0000000..f9e102e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/atomic-3.C
@@ -0,0 +1,37 @@ 
+// PR c++/71675 - __atomic_compare_exchange_n returns wrong type for typed enum
+// { dg-do compile { target c++11 } }
+
+template <class T>
+void sink (T);
+
+bool sink (bool);
+
+template <class T>
+bool test ()
+{
+  enum class E: T { };
+  static E e;
+
+  return sink (__atomic_compare_exchange_n (&e, &e, e, false, 0, 0));
+}
+
+void tests ()
+{
+  // __atomic_compare_exchange_n would fail to return bool when
+  //   its arguments were one of the three character types.
+  test<char>();
+  test<signed char>();
+  test<unsigned char>();
+
+  test<short>();
+  test<unsigned short>();
+
+  test<int>();
+  test<unsigned int>();
+
+  test<long>();
+  test<unsigned long>();
+
+  test<long long>();
+  test<unsigned long long>();
+}
diff --git a/gcc/testsuite/gcc.dg/atomic/pr71675.c b/gcc/testsuite/gcc.dg/atomic/pr71675.c
new file mode 100644
index 0000000..0e344ac
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/atomic/pr71675.c
@@ -0,0 +1,32 @@ 
+/* PR c++/71675 - __atomic_compare_exchange_n returns wrong type for typed enum
+ */
+/* { dg-do compile { target c11 } } */
+
+#define Test(T)								\
+  do {									\
+    static T x;								\
+    int r [_Generic (__atomic_compare_exchange_n (&x, &x, x, 0, 0, 0),	\
+		     _Bool: 1, default: -1)];				\
+    (void)&r;								\
+  } while (0)
+
+void f (void)
+{
+  /* __atomic_compare_exchange_n would fail to return _Bool when
+     its arguments were one of the three character types. */
+  Test (char);
+  Test (signed char);
+  Test (unsigned char);
+
+  Test (int);
+  Test (unsigned int);
+
+  Test (long);
+  Test (unsigned long);
+
+  Test (long long);
+  Test (unsigned long long);
+
+  typedef enum E { e } E;
+  Test (E);
+}