This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: __atomic_futex_unsigned::_M_load_when_not_equal has a 'return' statement with no value, in function returning unsigned int
- From: Jonathan Wakely <jwakely at redhat dot com>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: Patrick Palka <patrick at parcs dot ath dot cx>, libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Fri, 24 Jul 2015 17:30:25 +0100
- Subject: Re: __atomic_futex_unsigned::_M_load_when_not_equal has a 'return' statement with no value, in function returning unsigned int
- Authentication-results: sourceware.org; auth=none
- References: <CA+C-WL-r5b-yq8xPSL8KxgRFy+2Od0wKMD7CNj7T1BGUWVODzw at mail dot gmail dot com> <20150723082103 dot GC21787 at redhat dot com> <1437688488 dot 18846 dot 116 dot camel at localhost dot localdomain>
On 23/07/15 23:54 +0200, Torvald Riegel wrote:
On Thu, 2015-07-23 at 09:21 +0100, Jonathan Wakely wrote:
On 22/07/15 22:30 -0400, Patrick Palka wrote:
>Specifically, line 149 of atomix_futex.h has a bare "return;"
>statement, but the function is marked as returning non-void. This was
>caught while working on PR c++/18969. Vanilla G++ does not catch this
>error because return statements inside templates are currently only
>analyzed during instantiation time.
Ouch. Luckily that function is never called, so it's not doing any
harm.
Torvald, assuming we want to keep that unused function, is this the
right fix?
I think so. Thanks!
I'm committing this which also makes some other small tweaks.
Tested powerpc64le-linux, committed to trunk. Will commit to
gcc-5-branch too.
Torvald, I wonder if instead of doing (unsigned*)(void*)&_M_data, to
get to the member of the atomic<unsigned> we should make
__atomic_futex_unsigned a friend of __atomic_base so it can do that
more cleanly. I was OK with the casts earlier, but am changing my
mind.
commit 5e07807a055a7e9636b575445411bd1f79afb278
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Fri Jul 24 15:13:51 2015 +0100
* include/bits/atomic_futex.h [_GLIBCXX_HAVE_LINUX_FUTEX]
(_M_load_and_test_until): Whitespace.
(_M_load_and_test): Value-initialize the unused durations.
(_M_load_when_equal): Add missing return value.
diff --git a/libstdc++-v3/include/bits/atomic_futex.h b/libstdc++-v3/include/bits/atomic_futex.h
index ca3260d..90317f2 100644
--- a/libstdc++-v3/include/bits/atomic_futex.h
+++ b/libstdc++-v3/include/bits/atomic_futex.h
@@ -93,15 +93,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{
for (;;)
{
- // Don't bother checking the value again because we expect the caller to
- // have done it recently.
+ // Don't bother checking the value again because we expect the caller
+ // to have done it recently.
// memory_order_relaxed is sufficient because we can rely on just the
// modification order (store_notify uses an atomic RMW operation too),
// and the futex syscalls synchronize between themselves.
_M_data.fetch_or(_Waiter_bit, memory_order_relaxed);
- bool __ret;
- __ret = _M_futex_wait_until((unsigned*)(void*)&_M_data,
- __assumed | _Waiter_bit, __has_timeout, __s, __ns);
+ bool __ret = _M_futex_wait_until((unsigned*)(void*)&_M_data,
+ __assumed | _Waiter_bit,
+ __has_timeout, __s, __ns);
// Fetch the current value after waiting (clears _Waiter_bit).
__assumed = _M_load(__mo);
if (!__ret || ((__operand == __assumed) == __equal))
@@ -119,7 +119,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
bool __equal, memory_order __mo)
{
return _M_load_and_test_until(__assumed, __operand, __equal, __mo,
- false, chrono::seconds(0), chrono::nanoseconds(0));
+ false, {}, {});
}
// If a timeout occurs, returns a current value after the timeout;
@@ -146,7 +146,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_load_when_not_equal(unsigned __val, memory_order __mo)
{
unsigned __i = _M_load(__mo);
- if ((__i & ~_Waiter_bit) != __val) return;
+ if ((__i & ~_Waiter_bit) != __val)
+ return (__i & ~_Waiter_bit);
// TODO Spin-wait first.
return _M_load_and_test(__i, __val, false, __mo);
}