Re: [PATCH 2/3] Add support for musl-libc

[Bryan Hundven <bryanhundven at gmail dot com>]

Yann, List,

On Sun, Jul 27, 2014 at 4:15 PM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> Bryan, All,
>
> Here are a few comments...
>
> On 2014-07-26 18:04 -0700, Bryan Hundven spake thusly:
>> "Musl-libc is a new standard library to power a new generation of Linux-based
>> devices. musl is lightweight, fast, simple, free, and strives to be correct in
>> the sense of standards-conformance and safety."
>>
>> This patch adds support for musl to be selected and built by crosstool-ng.
>> NOTE: Experimental patches are needed for gcc and possibly other components.
>> Later patches include this support.
>
> I know we discussed this on IRC, but maybe we were too quick when
> setting up the plan. Maybe the patches could be added in this
> changeset, no?
>
> I can see that splitting is easier for review, but in the end, it does
> not really make sense to add musl support without the gcc patches,
> right? So, we may just squash your patches 2 and 3 into a single one
> when applying for upstreaming?

Sure, I'll do that in my next revision. (*cough* plus some other fixes
*cough*) :)

>> Signed-off-by: Bryan Hundven <bryanhundven@gmail.com>
> [--SNIP diffstat--]
>> diff --git a/config/libc.in b/config/libc.in
>> index 03f5db2..ff9ce9e 100644
>> --- a/config/libc.in
>> +++ b/config/libc.in
>> @@ -28,6 +28,10 @@ config LIBC_SUPPORT_NPTL
>>      bool
>>      select LIBC_SUPPORT_THREADS_ANY
>>
>> +config LIBC_SUPPORT_MUSL
>> +    bool
>> +    select LIBC_SUPPORT_THREADS_ANY
>> +
>
> All changes in this file is to add a new type of threads implementation,
> specific to musl.
>
> I've pushed to a temporary WIP clone a reworked infra for threads;
>     http://ymorin.is-a-geek.org/git/crosstool-ng/ yem/threads-rework
>
> So, all those changes you've done here should no longer be needed,
> hopefully.

Excellent!

>>  config LIBC_SUPPORT_LINUXTHREADS
>>      bool
>>      select LIBC_SUPPORT_THREADS_ANY
>> @@ -42,6 +46,7 @@ config LIBC_SUPPORT_THREADS_NONE
>>  config THREADS
>>      string
>>      default "nptl"          if THREADS_NPTL
>> +    default "musl"          if THREADS_MUSL
>
> This default should now go into the musl.in file, as:
>
>     config THREADS
>         default "XXXX"
>
> We should see if XXX can be set to just "nptl" and abuse that so we have
> no special handling to do in the gcc build script.
>
> Otherwise, "musl" is a good compromise, but the gcc build script should
> be adapted accordingly, which I don't see in this patch.

Perfect!

> [--SNIP--]
>> diff --git a/config/libc/musl.in b/config/libc/musl.in
>> new file mode 100644
>> index 0000000..69f465c
>> --- /dev/null
>> +++ b/config/libc/musl.in
>> @@ -0,0 +1,47 @@
>> +# musl options
>> +
>> +## depends on ! WINDOWS && ! BARE_METAL
>> +##
>> +## select LIBC_SUPPORT_MUSL
>> +## select CC_CORE_PASSES_NEEDED
>> +##
>> +## help Musl is a new standard library to power a new generation of Linux-based
>> +## help devices. musl is lightweight, fast, simple, free, and strives to be
>> +## help correct in the sense of standards-conformance and safety.
>
> Here, add the "config THREADS" discussed just above.
>
> it needs not be conditional, since the parsing of this file is already
> conditional to musl being selected.

Right. I like it!

>> +choice
>> +    bool
>> +    prompt "musl version"
>> +# Don't remove next line
>> +# CT_INSERT_VERSION_BELOW
>> +
>> +config LIBC_MUSL_V_1_1_3
>> +    bool
>> +    prompt "1.1.3 (Mainline)"
>> +    select LIBC_MUSL_DEFAULTS
>> +    depends on EXPERIMENTAL
>> +
>> +config LIBC_MUSL_V_1_0_3
>> +    bool
>> +    prompt "1.0.3 (Stable)"
>> +    select LIBC_MUSL_DEFAULTS
>> +
>> +config LIBC_MUSL_V_CUSTOM
>> +    bool
>> +    prompt "Custom musl"
>> +    select LIBC_MUSL_DEFAULTS
>> +    depends on EXPERIMENTAL
>> +
>> +endchoice
>> +
>> +config LIBC_MUSL_DEFAULTS
>> +    bool
>> +    select LIBC_SUPPORT_MUSL
>
> No longer needed.

Check!

> [--SNIP--]
>> diff --git a/config/libc/musl.in.2 b/config/libc/musl.in.2
>> new file mode 100644
>> index 0000000..bf4fb71
>> --- /dev/null
>> +++ b/config/libc/musl.in.2
>> @@ -0,0 +1,2 @@
>> +# musl second-part options
>
> If there is nothing to set here, do not provide that file.

Ack.

> [--SNIP--]
>> diff --git a/scripts/build/libc/musl.sh b/scripts/build/libc/musl.sh
>> new file mode 100644
>> index 0000000..f72517c
>> --- /dev/null
>> +++ b/scripts/build/libc/musl.sh
>> @@ -0,0 +1,111 @@
>
> I have not completely reviewd that file, but have at least a few
> comments:
>
> [--SNIP--]
>> +do_libc_configure() {
>> +    CT_DoLog EXTRA "Configuring C library"
>> +    local -a extra_cflags
>> +
>> +    # From buildroot:
>> +    # gcc constant folding bug with weak aliases workaround
>> +    # See http://www.openwall.com/lists/musl/2014/05/15/1
>> +    if [ "${CT_CC_GCC_4_9_or_later}" = "y" ]; then
>
> Completely off-topic: gcc-4.9 is utterly broken. See this thread where
> Linus Torvalds, in his usual coloured language, bashes gcc-4.9:
>     http://lkml.iu.edu//hypermail/linux/kernel/1407.3/00650.html

Technically:

https://lkml.org/lkml/2014/7/24/584

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61904
"The bad compiler versions are 4.5.0 (when debug_insn came in) to
4.8.3 and 4.9.0 and 4.9.1."

They are all broken, on host/build/target = x86_64, in some configurations.

>> +        extra_cflags+=("-fno-toplevel-reorder")
>
> Is that still needed with the current version of musl?

Yes, I got this from buildroot.

>> +    fi
>> +
>> +    # NOTE: musl handles the build/host/target a little bit differently
>> +    # then one would expect:
>> +    #   build   : not used
>> +    #   host    : the machine building musl
>> +    #   target  : the machine musl runs on
>> +    CT_DoExecLog CFG                \
>> +    CFLAGS="${extra_cflags[@]}"     \
>> +    CROSS_COMPILE="${CT_TARGET}-"   \
>> +    ./configure                     \
>> +        --host="${CT_BUILD}"        \
>> +        --target="${CT_TARGET}"     \
>> +        --prefix="/usr"             \
>> +        --disable-gcc-wrapper
>> +}
>> +
>> +do_libc_start_files() {
>> +    local ARCH
>> +
>> +    CT_DoStep INFO "Installing C library headers"
>> +
>> +    # Simply copy files until musl has the ability to build out-of-tree
>> +    CT_DoLog EXTRA "Copying sources to build directory"
>> +    CT_DoExecLog ALL cp -av "${CT_SRC_DIR}/musl-${CT_LIBC_VERSION}" \
>> +                            "${CT_BUILD_DIR}/build-libc-headers"
>> +    cd "${CT_BUILD_DIR}/build-libc-headers"
>> +
>> +    # musl can be installed without gcc if arch is known
>> +    case "${CT_TARGET}" in
>> +        arm*) ARCH=arm ;;
>> +        i?86*) ARCH=i386 ;;
>> +        x86_64*) ARCH=x86_64 ;;
>> +        mips-*|mipsel-*) ARCH=mips ;;
>> +        microblaze-*) ARCH=microblaze ;;
>> +        powerpc-*) ARCH=powerpc ;;
>> +        sh-*|sh[34]-*) ARCH=sh ;;
>> +        x32-*) ARCH=x32 ;;
>> +        *) return 1 ;;
>> +    esac
>
> You forgot to call do_libc_configure here. That should be the reason for
> the incorrect install daggs1-work reported incorrect install path (on
> IRC.)

Yup, noted, as we discussed on irc. fixed in the next revision (should
fix the sysroot/usr/local/musl/include issue).

> [--SNIP--]
>> diff --git a/scripts/functions b/scripts/functions
>> index 2e4d4fa..eae3b95 100644
>> --- a/scripts/functions
>> +++ b/scripts/functions
>> @@ -1182,6 +1182,7 @@ CT_DoBuildTargetTuple() {
>>      case "${CT_LIBC}" in
>>          *glibc) CT_TARGET_SYS=gnu;;
>>          uClibc) CT_TARGET_SYS=uclibc;;
>> +        musl)   CT_TARGET_SYS=musl;;
>>          *)      CT_TARGET_SYS=elf;;
>>      esac
>
> Regards,
> Yann E. MORIN.
>
> --
> .-----------------.--------------------.------------------.--------------------.
> |  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
> | +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
> | +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
> | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
> '------------------------------^-------^------------------^--------------------'

Thanks for the review!

-Bryan

--
For unsubscribe information see http://sourceware.org/lists.html#faq