On 2021-04-20 18:08, David Edelsohn wrote:
On Tue, Apr 20, 2021 at 8:43 PM David Edelsohn <dje....@gmail.com>
wrote:
On Tue, Apr 20, 2021 at 8:23 PM Thomas Rodgers
<rodg...@appliantology.com> wrote:
On 2021-04-20 17:09, David Edelsohn wrote:
On Tue, Apr 20, 2021 at 7:52 PM Thomas Rodgers
<rodg...@appliantology.com> wrote:
On 2021-04-20 15:25, David Edelsohn via Gcc wrote:
On Tue, Apr 20, 2021 at 12:43 PM Jakub Jelinek via Gcc
<gcc@gcc.gnu.org> wrote:
The first release candidate for GCC 11.1 is available from
https://gcc.gnu.org/pub/gcc/snapshots/11.1.0-RC-20210420/
ftp://gcc.gnu.org/pub/gcc/snapshots/11.1.0-RC-20210420
and shortly its mirrors. It has been generated from git revision
r11-8265-g246abba01f302eb453475b650ba839ec905be76d.
I have so far bootstrapped and tested the release candidate on
x86_64-linux and i686-linux. Please test it and report any issues to
bugzilla.
If all goes well, I'd like to release 11.1 on Tuesday, April 27th.
As I have reported in Bugzilla, the last minute
libstdc++: Refactor/cleanup of C++20 atomic wait implementation
has severely regressed libstdc++ on AIX due to changes to
bits/semaphore_base.h header.
- David
I posted a patch to BZ that should disable <semaphore> entirely for AIX
(and other targets where there's not a supported implementation
strategy).
This patch isn't the best way of addressing this for a variety of
reasons, but this support is intended as experimental for GCC11 anyway.
Unfortunately I can't test it on AIX because it would seem that my ssh
keys never landed on the AIX cfarm machines.
I am testing the patch on an AIX system inside IBM.
But it seems that you are disabling semaphore entirely on AIX, which
is an unnecessary regression. AIX has POSIX semaphores. libstdc++
configure defines
_GLIBCXX_HAVE_POSIX_SEMAPHORE
I don't understand your comments about disabling semaphore on AIX
while the comment about experimental for GCC11 implies that this is
some new, experimental feature. I could understand disabling the
experimental feature, but not disabling all semaphore support.
Thanks, David
The #error would not be hit if _GLIBCXX_HAVE_POSIX_SEMAPHORE were
defined, but it shows up in your error report.
You now have pinpointed the problem.
It's not that AIX doesn't have semaphore, but that the code previously
had a fallback that hid a bug in the macros:
#if defined _GLIBCXX_HAVE_LINUX_FUTEX &&
!_GLIBCXX_REQUIRE_POSIX_SEMAPHORE
// Use futex if available and didn't force use of POSIX
using __fast_semaphore =
__atomic_semaphore<__detail::__platform_wait_t>;
#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE
using __fast_semaphore = __platform_semaphore;
#else
using __fast_semaphore = __atomic_semaphore<ptrdiff_t>;
#endif
The problem is that libstdc++ configure defines
_GLIBCXX_HAVE_POSIX_SEMAPHORE in config.h. libstdc++ uses sed to
rewrite config.h to c++config.h and prepends _GLIBCXX_, so c++config.h
contains
#define _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE 1
And bits/semaphore_base.h is not testing that corrupted macro. Either
semaphore_base.h needs to test for the corrupted macro, or libtsdc++
configure needs to define HAVE_POSIX_SEMAPHORE without itself
prepending _GLIBCXX_ so that the c++config.h rewriting works
correctly and defines the correct macro for semaphore_base.h.
Thanks, David
By the way, you can see the bug in the macro name on any Linux system
and reproduce the failure on any Linux system if you force it to
fallback to POSIX semaphores instead of Linux Futex or atomic wait.
Thanks, David
I think the attached patch (also in BZ) addresses the issue in
bits/semaphore_base.h, but I'm going to defer to Jonathan on why the
macro name is being transformed incorrectly in the first place.From b1892fe84fb702ff3085eee8a062bc8606e5566a Mon Sep 17 00:00:00 2001
From: Thomas Rodgers <rodg...@twrodgers.com>
Date: Tue, 20 Apr 2021 21:56:21 -0700
Subject: [PATCH] [libstdc++] Work around for PR100164
As dje....@gmail.com pointed out, the _GLIBCXX_HAVE_POSIX_SEMAPHORE
macro is being munged into _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE. This
caused the detection logic in bits/semaphore_base.h to not define
__platform_semaphore. Fixing this uncovered the issue that
__platform_semaphore::_M_try_wait() was missing. This patch works around
the former issue and addresses the latter issue.
libstdc++-v3/ChangeLog:
* include/bits/semaphore_base.h: Define
__platform_semaphore::_M_try_wait(), temporarily adjust
detection macro to reflect the actual name being generated
during configuration.
* testsuite/30_threads/semaphore/try_acquire_posix.cc: Force
use of Posix semaphores if available and always run the test.
---
libstdc++-v3/include/bits/semaphore_base.h | 27 ++++++++++++++++---
.../30_threads/semaphore/try_acquire_posix.cc | 15 ++++++++---
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/libstdc++-v3/include/bits/semaphore_base.h b/libstdc++-v3/include/bits/semaphore_base.h
index 7e3235d182e..5c687bfae6f 100644
--- a/libstdc++-v3/include/bits/semaphore_base.h
+++ b/libstdc++-v3/include/bits/semaphore_base.h
@@ -38,7 +38,8 @@
#include <ext/numeric_traits.h>
#endif // __cpp_lib_atomic_wait
-#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+#ifdef _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
# include <limits.h>
# include <semaphore.h>
#endif
@@ -50,7 +51,8 @@ namespace std _GLIBCXX_VISIBILITY(default)
{
_GLIBCXX_BEGIN_NAMESPACE_VERSION
-#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+#ifdef _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
struct __platform_semaphore
{
using __clock_t = chrono::system_clock;
@@ -86,6 +88,24 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
}
}
+ _GLIBCXX_ALWAYS_INLINE bool
+ _M_try_acquire() noexcept
+ {
+ for (;;)
+ {
+ auto __err = sem_trywait(&_M_semaphore);
+ if (__err && (errno == EINTR))
+ continue;
+ else if (__err && (errno == EAGAIN))
+ return false;
+ else if (__err)
+ std::terminate();
+ else
+ break;
+ }
+ return true;
+ }
+
_GLIBCXX_ALWAYS_INLINE void
_M_release(std::ptrdiff_t __update) noexcept
{
@@ -253,7 +273,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
// use of Posix semaphores (sem_t). Doing so however, alters the ABI.
#if defined __cpp_lib_atomic_wait && !_GLIBCXX_REQUIRE_POSIX_SEMAPHORE
using __semaphore_impl = __atomic_semaphore;
-#elif _GLIBCXX_HAVE_POSIX_SEMAPHORE
+#elif _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #elif _GLIBCXX_HAVE_POSIX_SEMAPHORE
using __semaphore_impl = __platform_semaphore;
#else
# error "No suitable semaphore implementation available"
diff --git a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc
index 97e386f7b76..54d67652950 100644
--- a/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc
+++ b/libstdc++-v3/testsuite/30_threads/semaphore/try_acquire_posix.cc
@@ -20,8 +20,16 @@
// { dg-require-effective-target pthread }
// { dg-require-gthreads "" }
+#include <atomic>
+#ifdef _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+#define _GLIBCXX_REQUIRE_POSIX_SEMAPHORE 1
+#endif
+
#include <semaphore>
-#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+
+#ifdef _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
#include <chrono>
#include <thread>
#include <atomic>
@@ -139,11 +147,12 @@ void test04()
b.wait(1);
}
-#endif
+#endif // _GLIBCXX_HAVE_POSIX_SEMAPHORE
int main()
{
-#ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
+#ifdef _GLIBCXX__GLIBCXX_HAVE_POSIX_SEMAPHORE
+// #ifdef _GLIBCXX_HAVE_POSIX_SEMAPHORE
test01();
test02();
test03();
--
2.30.2
