Issues when building libcxx libcxxabi with MUSL

Issues when building libcxx libcxxabi with MUSL

[Zhao, Weiming]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

Hi,

I tried to build libcxx/libcxxabi using MUSL. Compiler is clang.

The build issues I got:

1) In libcxx, it complains that some pthread structure is not completely 
initialized.

2) Some types like max_align_t are already defined in clang's include.

3) libcxx can't find it's own math.h

A patch is attached. Please review if the changes are correct.


Thank,

Weiming




-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


[-- Attachment #2: build_libcxx.patch --]
[-- Type: text/plain, Size: 1230 bytes --]

diff --git a/include/math.h b/include/math.h
index 6ac91da..7c7e4aa 100644
--- a/include/math.h
+++ b/include/math.h
@@ -427,4 +427,8 @@ long double pow10l(long double);
 }
 #endif
 
+#ifdef __cplusplus
+#include_next <math.h>
+#endif
+
 #endif
diff --git a/include/pthread.h b/include/pthread.h
index af70b73..2995846 100644
--- a/include/pthread.h
+++ b/include/pthread.h
@@ -55,7 +55,7 @@ extern "C" {
 #define PTHREAD_PROCESS_SHARED 1
 
 
-#define PTHREAD_MUTEX_INITIALIZER {{{0}}}
+#define PTHREAD_MUTEX_INITIALIZER {{{0, 0, 0, 0, 0, 0 }}}
 #define PTHREAD_RWLOCK_INITIALIZER {{{0}}}
 #define PTHREAD_COND_INITIALIZER {{{0}}}
 #define PTHREAD_ONCE_INIT 0
diff --git a/tools/mkalltypes.sed b/tools/mkalltypes.sed
index fa15efc..f03f6a4 100644
--- a/tools/mkalltypes.sed
+++ b/tools/mkalltypes.sed
@@ -1,6 +1,7 @@
-/^TYPEDEF/s/TYPEDEF \(.*\) \([^ ]*\);$/#if defined(__NEED_\2) \&\& !defined(__DEFINED_\2)\
+/^TYPEDEF/s/TYPEDEF \(.*\) \([^ ]*\);$/#if defined(__NEED_\2) \&\& !defined(__DEFINED_\2) \&\& !defined(__CLANG_\U\2\E_DEFINED)\
 typedef \1 \2;\
 #define __DEFINED_\2\
+#define __CLANG_\U\2\E_DEFINED\
 #endif\
 /
 /^STRUCT/s/STRUCT * \([^ ]*\) \(.*\);$/#if defined(__NEED_struct_\1) \&\& !defined(__DEFINED_struct_\1)\

Re: Issues when building libcxx libcxxabi with MUSL

[Rich Felker]

On Wed, Jun 22, 2016 at 12:24:17PM -0700, Zhao, Weiming wrote:
> Hi,
> 
> I tried to build libcxx/libcxxabi using MUSL. Compiler is clang.
> 
> The build issues I got:
> 
> 1) In libcxx, it complains that some pthread structure is not
> completely initialized.
> 
> 2) Some types like max_align_t are already defined in clang's include.
> 
> 3) libcxx can't find it's own math.h
> 
> A patch is attached. Please review if the changes are correct.

These are all problems in libcxx, not musl, but since other people are
(I think) using it successfully they probably already have patches
which are just not upstream. Let's see if someone who has it working
can tell you how they did it.

> diff --git a/include/math.h b/include/math.h
> index 6ac91da..7c7e4aa 100644
> --- a/include/math.h
> +++ b/include/math.h
> @@ -427,4 +427,8 @@ long double pow10l(long double);
>  }
>  #endif
>  
> +#ifdef __cplusplus
> +#include_next <math.h>
> +#endif
> +

This is compiler-specific and certainly does not belong there.

>  #endif
> diff --git a/include/pthread.h b/include/pthread.h
> index af70b73..2995846 100644
> --- a/include/pthread.h
> +++ b/include/pthread.h
> @@ -55,7 +55,7 @@ extern "C" {
>  #define PTHREAD_PROCESS_SHARED 1
>  
>  
> -#define PTHREAD_MUTEX_INITIALIZER {{{0}}}
> +#define PTHREAD_MUTEX_INITIALIZER {{{0, 0, 0, 0, 0, 0 }}}

Presumably this is just a warning and not an error, and the compiler
should be suppressing the warning in macro expansion from a system
header.

Anyway I'd probably rather change these to something like:

#ifdef __cplusplus
#define PTHREAD_MUTEX_INITIALIZER {}
...
#else
#define PTHREAD_MUTEX_INITIALIZER {0}
...
#endif

if we make any changes.

>  #define PTHREAD_RWLOCK_INITIALIZER {{{0}}}
>  #define PTHREAD_COND_INITIALIZER {{{0}}}
>  #define PTHREAD_ONCE_INIT 0
> diff --git a/tools/mkalltypes.sed b/tools/mkalltypes.sed
> index fa15efc..f03f6a4 100644
> --- a/tools/mkalltypes.sed
> +++ b/tools/mkalltypes.sed
> @@ -1,6 +1,7 @@
> -/^TYPEDEF/s/TYPEDEF \(.*\) \([^ ]*\);$/#if defined(__NEED_\2) \&\& !defined(__DEFINED_\2)\
> +/^TYPEDEF/s/TYPEDEF \(.*\) \([^ ]*\);$/#if defined(__NEED_\2) \&\& !defined(__DEFINED_\2) \&\& !defined(__CLANG_\U\2\E_DEFINED)\
>  typedef \1 \2;\
>  #define __DEFINED_\2\
> +#define __CLANG_\U\2\E_DEFINED\
>  #endif\

This is a hack to workaround a problem somewhere else. You should not
be including any compiler-provided headers that define types covered
by the libc headers.

Rich

Re: Issues when building libcxx libcxxabi with MUSL

[Julien Ramseier]

Hi Zhao,

Not sure how you did build llvm-libcxx, but the only issue I encountered myself is
the use of PTHREAD_MUTEX_INITIALIZER in libcxx include/__mutex_base.
musl uses volatile types in its pthread_mutex_t struct and C++11 does not allow them
with constexpr member initializers. This rule has been relaxed in C++14.

Julien

> Le 22 juin 2016 à 21:24, Zhao, Weiming <weimingz@codeaurora.org> a écrit :
> 
> Hi,
> 
> I tried to build libcxx/libcxxabi using MUSL. Compiler is clang.
> 
> The build issues I got:
> 
> 1) In libcxx, it complains that some pthread structure is not completely initialized.
> 
> 2) Some types like max_align_t are already defined in clang's include.
> 
> 3) libcxx can't find it's own math.h
> 
> A patch is attached. Please review if the changes are correct.
> 
> 
> Thank,
> 
> Weiming
> 
> 
> 
> 
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> 
> <build_libcxx.patch>

Re: Issues when building libcxx libcxxabi with MUSL

[Rich Felker]

On Wed, Jun 22, 2016 at 09:38:17PM +0200, Julien Ramseier wrote:
> Hi Zhao,
> 
> Not sure how you did build llvm-libcxx, but the only issue I encountered myself is
> the use of PTHREAD_MUTEX_INITIALIZER in libcxx include/__mutex_base.
> musl uses volatile types in its pthread_mutex_t struct and C++11 does not allow them
> with constexpr member initializers. This rule has been relaxed in C++14.

Note that the code using them is still formally/conceptually buggy.
POSIX does not permit using these initializers as values. They can
only be used as initializers for objects with static storage duration.
A "mutex value" makes no sense; mutexes are objects not values.

Rich


> > Le 22 juin 2016 à 21:24, Zhao, Weiming <weimingz@codeaurora.org> a écrit :
> > 
> > Hi,
> > 
> > I tried to build libcxx/libcxxabi using MUSL. Compiler is clang.
> > 
> > The build issues I got:
> > 
> > 1) In libcxx, it complains that some pthread structure is not completely initialized.
> > 
> > 2) Some types like max_align_t are already defined in clang's include.
> > 
> > 3) libcxx can't find it's own math.h
> > 
> > A patch is attached. Please review if the changes are correct.
> > 
> > 
> > Thank,
> > 
> > Weiming
> > 
> > 
> > 
> > 
> > -- 
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
> > 
> > <build_libcxx.patch>

Re: Issues when building libcxx libcxxabi with MUSL

[Julien Ramseier]

[-- Attachment #1: Type: text/plain, Size: 1143 bytes --]


> Le 22 juin 2016 à 21:41, Rich Felker <dalias@libc.org> a écrit :
> 
> On Wed, Jun 22, 2016 at 09:38:17PM +0200, Julien Ramseier wrote:
>> Hi Zhao,
>> 
>> Not sure how you did build llvm-libcxx, but the only issue I encountered myself is
>> the use of PTHREAD_MUTEX_INITIALIZER in libcxx include/__mutex_base.
>> musl uses volatile types in its pthread_mutex_t struct and C++11 does not allow them
>> with constexpr member initializers. This rule has been relaxed in C++14.
> 
> Note that the code using them is still formally/conceptually buggy.
> POSIX does not permit using these initializers as values. They can
> only be used as initializers for objects with static storage duration.
> A "mutex value" makes no sense; mutexes are objects not values.

Actually the latest POSIX spec allows it.
See http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html <http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html>

A workaround for the C++11 constexpr issue should land soon in libcxx trunk.
http://reviews.llvm.org/D21637 <http://reviews.llvm.org/D21637>

Julien

[-- Attachment #2: Type: text/html, Size: 1923 bytes --]

Re: Issues when building libcxx libcxxabi with MUSL

[Szabolcs Nagy]

* Julien Ramseier <j.ramseier@gmail.com> [2016-06-30 14:50:14 +0200]:
> > Le 22 juin 2016 à 21:41, Rich Felker <dalias@libc.org> a écrit :
> > 
> > On Wed, Jun 22, 2016 at 09:38:17PM +0200, Julien Ramseier wrote:
> >> Hi Zhao,
> >> 
> >> Not sure how you did build llvm-libcxx, but the only issue I encountered myself is
> >> the use of PTHREAD_MUTEX_INITIALIZER in libcxx include/__mutex_base.
> >> musl uses volatile types in its pthread_mutex_t struct and C++11 does not allow them
> >> with constexpr member initializers. This rule has been relaxed in C++14.
> > 
> > Note that the code using them is still formally/conceptually buggy.
> > POSIX does not permit using these initializers as values. They can
> > only be used as initializers for objects with static storage duration.
> > A "mutex value" makes no sense; mutexes are objects not values.
> 
> Actually the latest POSIX spec allows it.
> See http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html <http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html>
> 

yes, the changelog says

POSIX.1-2008, Technical Corrigendum 1, XSH/TC1-2008/0460 [70,428] is applied.

so the semantics changed in 2013
http://austingroupbugs.net/view.php?id=70

> A workaround for the C++11 constexpr issue should land soon in libcxx trunk.
> http://reviews.llvm.org/D21637 <http://reviews.llvm.org/D21637>
> 

note that without pthread_mutex_destroy it may leak resources.

(in practice this e.g. matters on freebsd see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71684
on musl this is not an issue)

Re: Issues when building libcxx libcxxabi with MUSL

[Rich Felker]

On Thu, Jun 30, 2016 at 02:50:14PM +0200, Julien Ramseier wrote:
> 
> > Le 22 juin 2016 à 21:41, Rich Felker <dalias@libc.org> a écrit :
> > 
> > On Wed, Jun 22, 2016 at 09:38:17PM +0200, Julien Ramseier wrote:
> >> Hi Zhao,
> >> 
> >> Not sure how you did build llvm-libcxx, but the only issue I encountered myself is
> >> the use of PTHREAD_MUTEX_INITIALIZER in libcxx include/__mutex_base.
> >> musl uses volatile types in its pthread_mutex_t struct and C++11 does not allow them
> >> with constexpr member initializers. This rule has been relaxed in C++14.
> > 
> > Note that the code using them is still formally/conceptually buggy.
> > POSIX does not permit using these initializers as values. They can
> > only be used as initializers for objects with static storage duration.
> > A "mutex value" makes no sense; mutexes are objects not values.
> 
> Actually the latest POSIX spec allows it.
> See http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html <http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_init.html>

Could you cite the specific text you're looking at? The requirement of
static storage was removed by
http://austingroupbugs.net/view.php?id=70 but I see nothing allowing
the use of mutex values or assigning one to another, which as I
understand it is what the offending libc++ code is doing.

> A workaround for the C++11 constexpr issue should land soon in libcxx trunk.
> http://reviews.llvm.org/D21637 <http://reviews.llvm.org/D21637>

I'm not very good at reading modern C++, but that looks like an actual
fix, not a workaround.

Rich

Re: Issues when building libcxx libcxxabi with MUSL

[Szabolcs Nagy]

* Szabolcs Nagy <nsz@port70.net> [2016-06-30 15:15:47 +0200]:
> * Julien Ramseier <j.ramseier@gmail.com> [2016-06-30 14:50:14 +0200]:
> > A workaround for the C++11 constexpr issue should land soon in libcxx trunk.
> > http://reviews.llvm.org/D21637 <http://reviews.llvm.org/D21637>
> > 
> 
> note that without pthread_mutex_destroy it may leak resources.
> 

sorry libc++ uses pthread_mutex_destroy correctly
(the ~mutex() definition is just in another file).

there is still a nasty conformance issue that affects
all c++ library implementations: the mutex and thread
headers include <pthread.h>

it is not valid to include a posix header in c++ code,
this pollutes the global namespace, might not have valid
c++ syntax (posix headers are c99) and more importantly
pthread types are visible through native_handle() which
makes this impossible to fix.

Re: Issues when building libcxx libcxxabi with MUSL

[Rich Felker]

On Thu, Jun 30, 2016 at 03:38:40PM +0200, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@port70.net> [2016-06-30 15:15:47 +0200]:
> > * Julien Ramseier <j.ramseier@gmail.com> [2016-06-30 14:50:14 +0200]:
> > > A workaround for the C++11 constexpr issue should land soon in libcxx trunk.
> > > http://reviews.llvm.org/D21637 <http://reviews.llvm.org/D21637>
> > > 
> > 
> > note that without pthread_mutex_destroy it may leak resources.
> > 
> 
> sorry libc++ uses pthread_mutex_destroy correctly
> (the ~mutex() definition is just in another file).
> 
> there is still a nasty conformance issue that affects
> all c++ library implementations: the mutex and thread
> headers include <pthread.h>

They could at least reduce the dependency to sys/types.h with almost
no changes, I think. Better would be including just a proper-sized
buffer and initializing the mutex into it via code in its own TU, not
a public header file.

> it is not valid to include a posix header in c++ code,
> this pollutes the global namespace, might not have valid
> c++ syntax (posix headers are c99) and more importantly
> pthread types are visible through native_handle() which
> makes this impossible to fix.

That can be fixed just by removing them (existence of native handles
is optional).

Rich

Re: Issues when building libcxx libcxxabi with MUSL

[Julien Ramseier]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]


> Le 30 juin 2016 à 15:38, Rich Felker <dalias@libc.org> a écrit :
> 
> Could you cite the specific text you're looking at? The requirement of
> static storage was removed by
> http://austingroupbugs.net/view.php?id=70 <http://austingroupbugs.net/view.php?id=70> but I see nothing allowing
> the use of mutex values or assigning one to another, which as I
> understand it is what the offending libc++ code is doing.

I'm not sure what you mean by "mutex value".
The libcxx mutex class only uses the PTHREAD_MUTEX_INITIALIZER
macro as an initializer for its underlying pthread_mutex member.

The part of the spec I was referring to is the use of
PTHREAD_MUTEX_INITIALIZER to initialize non-static mutexes,
which is what libcxx did and still do with my patch, but in a different
way to avoid the constexpr issue due to the nested volatile types.


[-- Attachment #2: Type: text/html, Size: 5758 bytes --]

Re: Issues when building libcxx libcxxabi with MUSL

[Rich Felker]

On Thu, Jun 30, 2016 at 04:36:09PM +0200, Julien Ramseier wrote:
> 
> > Le 30 juin 2016 à 15:38, Rich Felker <dalias@libc.org> a écrit :
> > 
> > Could you cite the specific text you're looking at? The requirement of
> > static storage was removed by
> > http://austingroupbugs.net/view.php?id=70 <http://austingroupbugs.net/view.php?id=70> but I see nothing allowing
> > the use of mutex values or assigning one to another, which as I
> > understand it is what the offending libc++ code is doing.
> 
> I'm not sure what you mean by "mutex value".

What I mean is any use of an expression of type pthread_mutex_t except
an lvalue whose address is taken to pass to one of the pthread_mutex_*
functions -- and especially, assignment expressions assigning a value
of type pthread_mutex_t to an object of type pthread_mutex_t.

> The libcxx mutex class only uses the PTHREAD_MUTEX_INITIALIZER
> macro as an initializer for its underlying pthread_mutex member.

I don't know modern C++ well enough to evaluate whether that's
accurate. But in the old code this looks like use of a mutex value:

	constexpr mutex() _NOEXCEPT : __m_(_LIBCPP_MUTEX_INITIALIZER) {} 

Maybe I'm misreading it.

> The part of the spec I was referring to is the use of
> PTHREAD_MUTEX_INITIALIZER to initialize non-static mutexes,
> which is what libcxx did and still do with my patch, but in a different
> way to avoid the constexpr issue due to the nested volatile types.

If the old code was actually just an initialization but just failing
to work for some technical C++ reasons, I'm sorry for the noise. In
any case it seems like there's a working fix now.

Rich