[libc++] Provide SEM_VALUE_MAX fallback on Solaris

[ldionne]

Summary

This is the first in a series of patches to port libc++ to Solaris. There does exist a
slightly bitrotten port, however it was done on Illumos (an OpenSolaris derivative)
and the two have diverged in some areas over the last 10 years.

Building libc++ on Solaris fails like this:

In file included from /vol/llvm/src/llvm-project/local/libcxx/include/atomic:550,
                 from /vol/llvm/src/llvm-project/local/libcxx/include/memory:681,
                 from /vol/llvm/src/llvm-project/local/libcxx/include/algorithm:643,
                 from /vol/llvm/src/llvm-project/local/libcxx/test/libcxx/double_include.sh.cpp:24:
/vol/llvm/src/llvm-project/local/libcxx/include/semaphore:165:73: error: ‘SEM_VALUE_MAX’ was not declared in this scope
  165 |     typename conditional<(__least_max_value > 1 && __least_max_value <= _LIBCPP_SEMAPHORE_MAX),
      |                                                                         ^~~~~~~~~~~~~~~~~~~~~

This happens because <limits.h> doesn't define SEM_VALUE_MAX. While this is
allowed by XPG7 if the value is indeterminate ([[ https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html#tag_13_24 | XPG7 <limits.h> ]] and the
application is expected to fall back to sysconf(_SC_SEM_VALUE_MAX), this isn't the case on Solaris:
sysconf(_SC_SEM_VALUE_MAX) always returns _SEM_VALUE_MAX from
<sys/param.h>.

I've chose a solution similar to the one in Python Issue 3110 which uses that value as a fallback.

Tested on amd64-pc-solaris2.11 (Solaris 11.4, 11.3 and OpenIndiana 2020.04).

Ok for master?

[jfb]

I'd much rather remove unsupported platforms, instead of seemingly supporting them. Is there interest in officially supporting Solaris? Are there going to be actively maintained bots, as well as a specific set of people who'll fix issues? Would these folks do more libc++ contributions than just maintain Solaris?

[ro]

Where are such requirements documented? Nothing like this has ever come up with my Solaris patches to llvm, clang, or compiler-rt.

That said, I do maintain both Solaris/SPARC and Solaris/x86 buildbots. However, I certainly cannot commit to full-time maintenance of LLVM on
Solaris in addition to my Solaris GCC maintainership in my copious spare time: such a claim would deceive both myself and others. If that's
a requirement for accepting any Solaris patches, please say so up front so I can avoid wasting both your and my time. However, that should be a
matter of LLVM project policy, not the personal preference of an individual maintainer.

[ldionne]

In D81602#2087403, @ro wrote:

Where are such requirements documented? Nothing like this has ever come up with my Solaris patches to llvm, clang, or compiler-rt.

That said, I do maintain both Solaris/SPARC and Solaris/x86 buildbots. However, I certainly cannot commit to full-time maintenance of LLVM on
Solaris in addition to my Solaris GCC maintainership in my copious spare time: such a claim would deceive both myself and others. If that's
a requirement for accepting any Solaris patches, please say so up front so I can avoid wasting both your and my time. However, that should be a
matter of LLVM project policy, not the personal preference of an individual maintainer.

Let's put it that way: as far as I can tell, this code is dead, so this patch isn't adding anything beyond technical debt. And the reason why it's dead is that there are no *libc++* build bots set up for Solaris, and no maintainer actively working on supporting that platform AFAICT.

JF made the comment he made because I recently complained to him about how the maintenance burden for libc++ is both huge and also rests on the shoulder of too few. The main reason for the burden being so large is that we try to support a high number of system configurations and sub-configurations of libc++ (like no-threads, no-stdio, etc.), and we also tend to bend backwards instead of requesting that systems conform to e.g. POSIX in a more stringent way. That's unfortunately part of a discussion that needs to happen and that is much larger than this patch, but my point is that libc++ needs to start dropping workarounds for systems that are not actively supported, not add new ones.

Going back to the patch itself -- is there any reason why <limits.h> can't define SEM_VALUE_MAX on Solaris? That's the right way of solving this problem.

[ro]

In D81602#2087683, @ldionne wrote:

Let's put it that way: as far as I can tell, this code is dead, so this patch isn't adding anything beyond technical debt. And the reason why it's dead is that there are no *libc++* build bots set up for Solaris, and no maintainer actively working on supporting that platform AFAICT.

True, because right now libc++ doesn't build even on Illumos. As I mentioned, my patch is only the first in a series to change that.
I'd have posted more of them already but for the pushback I got on the first trivial one. While working on my patch series, I already encountered more of those (like AIX and partially Windows) that caused me to spend quite some time trying to verify that my changes wouldn't negatively affect them, only to discover in the end that the code doesn't even compile.

JF made the comment he made because I recently complained to him about how the maintenance burden for libc++ is both huge and also rests on the shoulder of too few. The main reason for the burden being so large is that we try to support a high number of system configurations and sub-configurations of libc++ (like no-threads, no-stdio, etc.), and we also tend to bend backwards instead of requesting that systems conform to e.g. POSIX in a more stringent way. That's unfortunately part of a discussion that needs to happen and that is much larger than this patch, but my point is that libc++ needs to start dropping workarounds for systems that are not actively supported, not add new ones.

Fully understood: keeping code around for dead systems is only creating work for everyone else and making the code harder to maintain
for no gain. However, demanding systems that conform to the very latest C and POSIX.1 standards quickly restricts you to a very small set: many people tend to forget that.

Going back to the patch itself -- is there any reason why <limits.h> can't define SEM_VALUE_MAX on Solaris? That's the right way of solving this problem.

Yes and no: I'm discussing this very question with the Solaris engineers as we speak. However, even if this were changed tomorrow, there's
a large installed base without that fix, so unless you restrict libc++ to an (at least initially) neglegible number of systems, you have to allow
for those. Besides, even if the choice of not defining SEM_VALUE_MAX is strange especially if the return value of sysconf(_SC_SEM_VALUE_MAX) is a constant, it's fully within at least the letter of POSIX.1/XPG7 which allows for that situation.

[jfb]

In D81602#2087793, @ro wrote:

In D81602#2087683, @ldionne wrote:

Let's put it that way: as far as I can tell, this code is dead, so this patch isn't adding anything beyond technical debt. And the reason why it's dead is that there are no *libc++* build bots set up for Solaris, and no maintainer actively working on supporting that platform AFAICT.

True, because right now libc++ doesn't build even on Illumos. As I mentioned, my patch is only the first in a series to change that.
I'd have posted more of them already but for the pushback I got on the first trivial one. While working on my patch series, I already encountered more of those (like AIX and partially Windows) that caused me to spend quite some time trying to verify that my changes wouldn't negatively affect them, only to discover in the end that the code doesn't even compile.

JF made the comment he made because I recently complained to him about how the maintenance burden for libc++ is both huge and also rests on the shoulder of too few. The main reason for the burden being so large is that we try to support a high number of system configurations and sub-configurations of libc++ (like no-threads, no-stdio, etc.), and we also tend to bend backwards instead of requesting that systems conform to e.g. POSIX in a more stringent way. That's unfortunately part of a discussion that needs to happen and that is much larger than this patch, but my point is that libc++ needs to start dropping workarounds for systems that are not actively supported, not add new ones.

Fully understood: keeping code around for dead systems is only creating work for everyone else and making the code harder to maintain
for no gain. However, demanding systems that conform to the very latest C and POSIX.1 standards quickly restricts you to a very small set: many people tend to forget that.

Right, I didn't see bots for this platform and that's why I asked. I'm completely happy if libc++ supports a variety of platforms, IMO more is better because it prevents local-minimum solutions, but this has to be balanced with maintenance cost. Right now so many platforms have a semblance of support without actually being supported that it's a big hindrance to folks working on libc++. Committing a patch might break bots that are unmaintained, or might break configurations for which there are no bots at all (and the we get reports a few weeks after the facts). That's not an ideal situation at all.

If you're actually supporting Solaris, providing bots and fixing incoming breakages then I agree that the platform should be supported. However, it would be good to do more than just support a platform, and actually support libc++ as a whole. There's a significant number of bugs and features which aren't getting the attention they need, in large part because maintainers are busy with the large number of semi-supported platforms.

In other words, I'd much rather see ongoing commitments to the codebase, not just for a single platforms. The reality is that right now many important things are lagging because most folks support their platforms and nothing else. That's far from ideal, but I understand that it's sometimes the only option.

[ldionne]

Is there interest for pushing this forward? We have instructions for setting up workers for your platform here: https://libcxx.llvm.org/AddingNewCIJobs.html#addingnewcijobs

Once this is done, I'd be happy to help you with reviewing patches that expand support to Solaris and make sure it doesn't bit rot like the previous port.

[ro]

In D81602#2982911, @ldionne wrote:

Is there interest for pushing this forward? We have instructions for setting up workers for your platform here: https://libcxx.llvm.org/AddingNewCIJobs.html#addingnewcijobs

Once this is done, I'd be happy to help you with reviewing patches that expand support to Solaris and make sure it doesn't bit rot like the previous port.

I'm pretty certain I won't move this (and my subsequent patches I hadn't submitted) forward any time soon for several reasons:

• I've spent way too much time in LLVM in recent months, with limited gain, meanwhile massively neglecting my Solaris GCC maintainer duties.
• buildkite hasn't even been ported to Solaris or Illumos AFAICT and I have very little inclination of doing this and adding yet another buildbot system to my mix.
• I can't tell for certain how libcxx CI works, but if LLVM's buildbot is any indication, the agent is useless unless you manage to not only make libcxx buildable on your target (which I did for Solaris), but also have no failures in the testsuite. When I stopped working on my port more than a year ago, I was at 20 failures for libcxx, plus 6 for libunwind and 1 for libc++abi.
• Last but not least, I would only be able to handle the Solaris side of things. Given that Solaris and Illumos have diverged considerably especially in areas that affect libcxx (like locale support), any patch I would add is likely to break Illumos (even further). When I reached out to handle the same problem for clang and compiler-rt, there has been almost no feedback from the Illumos community.

[ldionne]

[Github PR Transition cleanup]

As explained in D138680, we didn't move forward with official support for Solaris so I think this should be abandoned.

libcxx/include/__config

 
 #ifdef __sun__
 #  include <sys/isa_defs.h>
 #  ifdef _LITTLE_ENDIAN
 #    define _LIBCPP_LITTLE_ENDIAN
 #  else
 #    define _LIBCPP_BIG_ENDIAN
 #  endif
+#  ifndef SEM_VALUE_MAX
+     // <limits.h> doesn't define SEM_VALUE_MAX as allowed by XPG7.  However,
+     // sysconf(_SC_SEM_VALUE_MAX) has returned _SEM_VALUE_MAX since Solaris 2.6.
+#    define _KMEMUSER
+#    include <sys/param.h>
+#    define SEM_VALUE_MAX _SEM_VALUE_MAX
+#  endif
 #endif // __sun__
 
 #if defined(__CloudABI__)
    // Certain architectures provide arc4random(). Prefer using
    // arc4random() over /dev/{u,}random to make it possible to obtain
    // random data even when using sandboxing mechanisms such as chroots,
    // Capsicum, etc.
 #  define _LIBCPP_USING_ARC4_RANDOM