[0/3] picolibc: Add picolibc linking help

  Picolibc is a C library for embedded systems based on code from newlib
and avr libc. To connect some system-dependent picolibc functions
(like stdio) to an underlying platform, the platform may provide an OS
library.

This OS library must follow the C library in the link command line. In
current picolibc, that is done by providing an alternate .specs file
which can rewrite the *lib spec to insert the OS library in the right
spot.

This patch series adds the ability to specify the OS library on the
gcc command line when GCC is configured to us picolibc as the default
C library, and then hooks that up for arm, nds32, riscv and sh targets.

Keith Packard (3):
  Allow default libc to be specified to configure
  Add newlib and picolibc as default C library choices
  Add '--oslib=' option when default C library is picolibc

 gcc/config.gcc            | 56 ++++++++++++++++++++++++++++++++-------
 gcc/config/arm/elf.h      |  5 ++++
 gcc/config/nds32/elf.h    |  4 +++
 gcc/config/picolibc.opt   | 26 ++++++++++++++++++
 gcc/config/riscv/elf.h    |  4 +++
 gcc/config/sh/embed-elf.h |  5 ++++
 gcc/configure.ac          |  4 +++
 7 files changed, 95 insertions(+), 9 deletions(-)
 create mode 100644 gcc/config/picolibc.opt
  

On Wed, Aug 24, 2022 at 11:12 AM Keith Packard via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Picolibc is a C library for embedded systems based on code from newlib
> and avr libc. To connect some system-dependent picolibc functions
> (like stdio) to an underlying platform, the platform may provide an OS
> library.
>
> This OS library must follow the C library in the link command line. In
> current picolibc, that is done by providing an alternate .specs file
> which can rewrite the *lib spec to insert the OS library in the right
> spot.

Why do you need to change the specs to support picolibc? Why not have
the library supply the specs file instead, like what is done for
newlib and libgloss?

>
> This patch series adds the ability to specify the OS library on the
> gcc command line when GCC is configured to us picolibc as the default
> C library, and then hooks that up for arm, nds32, riscv and sh targets.


What OS libraries are not included in libc? I trying to figure out why
this needs to be special cased here.

Thanks,
Andrew Pinski

>
> Keith Packard (3):
>   Allow default libc to be specified to configure
>   Add newlib and picolibc as default C library choices
>   Add '--oslib=' option when default C library is picolibc
>
>  gcc/config.gcc            | 56 ++++++++++++++++++++++++++++++++-------
>  gcc/config/arm/elf.h      |  5 ++++
>  gcc/config/nds32/elf.h    |  4 +++
>  gcc/config/picolibc.opt   | 26 ++++++++++++++++++
>  gcc/config/riscv/elf.h    |  4 +++
>  gcc/config/sh/embed-elf.h |  5 ++++
>  gcc/configure.ac          |  4 +++
>  7 files changed, 95 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/config/picolibc.opt
>
> --
> 2.36.1
>
  

Andrew Pinski <pinskia@gmail.com> writes:

(removing gcc@ as not appropriate for patch discussions)

Thanks for reviewing my patches; I appreciate the time you have taken to
think about this.

> Why do you need to change the specs to support picolibc? Why not have
> the library supply the specs file instead, like what is done for
> newlib and libgloss?

Several architectures do include support for newlib's libgloss in their
gcc configuration today (i386, m32r, microblaze, nds32, pru, riscv and
sh), so I wondered if it made sense to add support for picolibc's
target-specific libraries as well.

Picolibc does deliver a spec file fragment which implements this
functionality, but that requires the addition of --specs=picolibc.specs
to the gcc command line instead of being built-in to gcc itself. When
creating an integrated toolchain using picolibc, it seems a bit odd to
require an option for the toolchain to work.

> What OS libraries are not included in libc? I trying to figure out why
> this needs to be special cased here.

As a general-purpose embedded C library, picolibc doesn't include any
OS-specific code. Instead, it defines a limited subset of POSIX
interfaces which are to be supplied by the target platform.

Picolibc itself supplies sample implementations of these ABIs that can
run on top of bare metal systems with semihosting support which are used
while testing picolibc itself.

This is similar to newlib's libgloss: the C library is built atop
another library which needs to follow it in the linker command line for
symbol resolution to work correctly. Making this work requires a change
in the *lib spec file fragment.

Adjusting the *lib fragment can either be done in an externally provided
specs file, or built-in to gcc. Both of these mechanisms are present in
the gcc ecosystem today, leading me to wonder whether the gcc community
would be interested in having an integrated option available.
  

I am thinking that maybe we should add
-mlibc=[newlib|newlib-nano|picolibc|unknown] option to bare-matel
toolchain, one reason is having an unify interface to select libc
implementation between clang/LLVM, spec file is a GCC specific stuff,
that cause very bad compatibility between GCC and clang/LLVM, and
having option to control that would be better since clang/LLVM don't
have those configure time option.

For linux toolchain, it uses -mbionic, -muclibc, -mglibc and -mmusl,
maybe we could also use an unify -mlibc= option for that?



On Thu, Aug 25, 2022 at 3:32 AM Keith Packard via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Andrew Pinski <pinskia@gmail.com> writes:
>
> (removing gcc@ as not appropriate for patch discussions)
>
> Thanks for reviewing my patches; I appreciate the time you have taken to
> think about this.
>
> > Why do you need to change the specs to support picolibc? Why not have
> > the library supply the specs file instead, like what is done for
> > newlib and libgloss?
>
> Several architectures do include support for newlib's libgloss in their
> gcc configuration today (i386, m32r, microblaze, nds32, pru, riscv and
> sh), so I wondered if it made sense to add support for picolibc's
> target-specific libraries as well.
>
> Picolibc does deliver a spec file fragment which implements this
> functionality, but that requires the addition of --specs=picolibc.specs
> to the gcc command line instead of being built-in to gcc itself. When
> creating an integrated toolchain using picolibc, it seems a bit odd to
> require an option for the toolchain to work.
>
> > What OS libraries are not included in libc? I trying to figure out why
> > this needs to be special cased here.
>
> As a general-purpose embedded C library, picolibc doesn't include any
> OS-specific code. Instead, it defines a limited subset of POSIX
> interfaces which are to be supplied by the target platform.
>
> Picolibc itself supplies sample implementations of these ABIs that can
> run on top of bare metal systems with semihosting support which are used
> while testing picolibc itself.
>
> This is similar to newlib's libgloss: the C library is built atop
> another library which needs to follow it in the linker command line for
> symbol resolution to work correctly. Making this work requires a change
> in the *lib spec file fragment.
>
> Adjusting the *lib fragment can either be done in an externally provided
> specs file, or built-in to gcc. Both of these mechanisms are present in
> the gcc ecosystem today, leading me to wonder whether the gcc community
> would be interested in having an integrated option available.
>
> --
> -keith
  

Kito Cheng <kito.cheng@gmail.com> writes:

> I am thinking that maybe we should add
> -mlibc=[newlib|newlib-nano|picolibc|unknown] option to bare-matel
> toolchain, one reason is having an unify interface to select libc
> implementation between clang/LLVM, spec file is a GCC specific stuff,
> that cause very bad compatibility between GCC and clang/LLVM, and
> having option to control that would be better since clang/LLVM don't
> have those configure time option.

You can create specs file for each library so that you select the
library with --specs=picolibc.specs or --specs=newlib-nano.specs. That
seems sufficient as you can mess with various compiler options and set
the header paths. Crosstool-ng does this and it enables delivering
newlib, newlib-nano and picolibc in the same toolchain.
  

Keith Packard via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Picolibc is a C library for embedded systems based on code from newlib
> and avr libc. To connect some system-dependent picolibc functions
> (like stdio) to an underlying platform, the platform may provide an OS
> library.
>
> This OS library must follow the C library in the link command line. In
> current picolibc, that is done by providing an alternate .specs file
> which can rewrite the *lib spec to insert the OS library in the right
> spot.
>
> This patch series adds the ability to specify the OS library on the
> gcc command line when GCC is configured to us picolibc as the default
> C library, and then hooks that up for arm, nds32, riscv and sh targets.

Not really my area, but the approach LGTM FWIW.  Main question/points:

- In:

  +case "${with_default_libc}" in
  +glibc)
  +    default_libc=LIBC_GLIBC
  +    ;;

  should there be a default case that raises an error for unrecognised
  libcs?  Command-line checking for configure isn't very tight, but we
  do raise similar errors for things like invalid --enable-threads values.

- I'm not sure either way about adding LIBC_NEWLIB.  On the one hand
  it makes sense for completeness, but on the other it's write-only.
  Adding it means that --with-default-libc=newlib toolchains have a
  different macro configuration from a default toolchain even in cases
  where newlib is the default.

  On balance I think it would be better to accept
  --with-default-libc=newlib but set default_libc to the empty string.

- Should we raise an error for toolchains that don't support the given
  C library?  It feels like we should, but I realise that could be
  difficult to do.

- Very minor, but in lines like:

  +#if defined(DEFAULT_LIBC) && defined(LIBC_PICOLIBC) && DEFAULT_LIBC == LIBC_PICOLIBC

  is LIBC_PICOLIB ever undefined?  It looks like config.gcc provides
  an unconditional definition.  If it is always defined then:

  #if DEFAULT_LIBC == LIBC_PICOLIB

  would be clearer.

Thanks,
Richard

>
> Keith Packard (3):
>   Allow default libc to be specified to configure
>   Add newlib and picolibc as default C library choices
>   Add '--oslib=' option when default C library is picolibc
>
>  gcc/config.gcc            | 56 ++++++++++++++++++++++++++++++++-------
>  gcc/config/arm/elf.h      |  5 ++++
>  gcc/config/nds32/elf.h    |  4 +++
>  gcc/config/picolibc.opt   | 26 ++++++++++++++++++
>  gcc/config/riscv/elf.h    |  4 +++
>  gcc/config/sh/embed-elf.h |  5 ++++
>  gcc/configure.ac          |  4 +++
>  7 files changed, 95 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/config/picolibc.opt
  

Richard Sandiford <richard.sandiford@arm.com> writes:

Thanks much for reviewing this series. I really appreciate it.

>   should there be a default case that raises an error for unrecognised
>   libcs?  Command-line checking for configure isn't very tight, but we
>   do raise similar errors for things like invalid --enable-threads
>   values.

Good thinking. It's way to easy to introduce a typo somewhere and have
it get missed.

>   On balance I think it would be better to accept
>   --with-default-libc=newlib but set default_libc to the empty string.

That also makes good sense -- leave existing configurations unchanged.

> - Should we raise an error for toolchains that don't support the given
>   C library?  It feels like we should, but I realise that could be
>   difficult to do.

That would be nice, but it also feels like making it reliable enough to
be useful would be difficult to maintain in the future, even if we did
manage to make it mostly work today.

> - Very minor, but in lines like:
>
>   +#if defined(DEFAULT_LIBC) && defined(LIBC_PICOLIBC) && DEFAULT_LIBC == LIBC_PICOLIBC
>
>   is LIBC_PICOLIB ever undefined?  It looks like config.gcc provides
>   an unconditional definition.  If it is always defined then:
>
>   #if DEFAULT_LIBC == LIBC_PICOLIB
>
>   would be clearer.

Agreed. Thanks again for taking a look; I'll send a new series with
these issues fixed shortly.
  

Picolibc is a C library for embedded systems based on code from newlib
and avr libc. To connect some system-dependent picolibc functions
(like stdio) to an underlying platform, the platform may provide an OS
library.

This OS library must follow the C library in the link command line. In
current picolibc, that is done by providing an alternate .specs file
which can rewrite the *lib spec to insert the OS library in the right
spot.

This patch series adds the ability to specify the OS library on the
gcc command line when GCC is configured to us picolibc as the default
C library, and then hooks that up for arm, nds32, riscv and sh targets.

This is the second version of these patches which address several
issues raised in review by Richard Sandiford.

Keith Packard (3):
  Allow default libc to be specified to configure
  Add newlib and picolibc as default C library choices
  Add '--oslib=' option when default C library is picolibc

 gcc/config.gcc            | 65 +++++++++++++++++++++++++++++++++------
 gcc/config/arm/elf.h      |  5 +++
 gcc/config/nds32/elf.h    |  4 +++
 gcc/config/picolibc.opt   | 26 ++++++++++++++++
 gcc/config/riscv/elf.h    |  4 +++
 gcc/config/sh/embed-elf.h |  5 +++
 gcc/configure.ac          |  4 +++
 7 files changed, 104 insertions(+), 9 deletions(-)
 create mode 100644 gcc/config/picolibc.opt