[PATCH 0/2] Support libc with stdio-only I/O in libstdc++

[PATCH 0/2] Support libc with stdio-only I/O in libstdc++

[Keith Packard]

The current libstdc++ basic_file_stdio.cc code assumes a POSIX API
underneath the stdio implementation provided by the host libc. This
means that the host must provide a fairly broad POSIX file API,
including read, write, open, close, lseek and ioctl.

This patch changes basic_file_stdio.cc to only use basic ANSI-C stdio
functions, allowing it to be used with libc implementations like
picolibc which may not have a POSIX operating system underneath.

[PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Keith Packard]

This option directs the library to only use simple stdio APIs instead
of using fileno to get the file descriptor for use with POSIX APIs.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 libstdc++-v3/ChangeLog                     | 13 ++++++
 libstdc++-v3/acinclude.m4                  | 13 ++++++
 libstdc++-v3/config/io/basic_file_stdio.cc | 46 +++++++++++++++++++---
 libstdc++-v3/configure.ac                  |  1 +
 4 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index d3ee18ed7d1..7f6b0697534 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1147,6 +1147,19 @@
 	* include/bits/uniform_int_dist.h (uniform_int_distribution::_S_nd):
 	Use qualified-id to refer to static member functions.
 
+2020-11-04  Keith Packard  <keithp@keithp.com>
+
+	* acinclude.m4: Add GLIBCXX_ENABLE_PURE_STDIO for
+	--enable-libstdcxx-pure-stdio
+	* configure.ac: Use GLIBCXX_ENABLE_PURE_STDIO for
+	--enable-libstdcxx-pure-stdio
+	* configure: regenerate
+	* config.h.in: regenerate
+	* config/io/basic_file_stdio.cc: Add support for
+	_GLIBCXX_USE_PURE_STDIO. This makes libstdc++ use only defined
+	stdio entry points for all I/O operations, without direct calls to
+	underlying POSIX functions.
+
 2020-11-03  Jonathan Wakely  <jwakely@redhat.com>
 
 	* include/std/syncstream: Include <bits/std_mutex.h>
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index fcd9ea3d23a..863695e68e7 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2889,6 +2889,19 @@ AC_DEFUN([GLIBCXX_ENABLE_CSTDIO], [
 ])
 
 
+dnl
+dnl Check whether to use 'pure' stdio (no POSIX calls at all)
+dnl
+dnl Default is 'no'
+AC_DEFUN([GLIBCXX_ENABLE_PURE_STDIO], [
+  GLIBCXX_ENABLE(libstdcxx-pure-stdio,$1,,[use only stdio APIs])
+  if test $enable_libstdcxx_pure_stdio = yes; then
+    AC_DEFINE(_GLIBCXX_USE_PURE_STDIO, 1,
+              [Define to restrict code to stdio APIs.])
+  fi
+])
+
+
 dnl
 dnl Check for "unusual" flags to pass to the compiler while building.
 dnl
diff --git a/libstdc++-v3/config/io/basic_file_stdio.cc b/libstdc++-v3/config/io/basic_file_stdio.cc
index ba830fb9e97..d31da8497f4 100644
--- a/libstdc++-v3/config/io/basic_file_stdio.cc
+++ b/libstdc++-v3/config/io/basic_file_stdio.cc
@@ -111,13 +111,21 @@ namespace
 
   // Wrapper handling partial write.
   static std::streamsize
+#ifdef _GLIBCXX_USE_PURE_STDIO
+  xwrite(FILE *__file, const char* __s, std::streamsize __n)
+#else
   xwrite(int __fd, const char* __s, std::streamsize __n)
+#endif
   {
     std::streamsize __nleft = __n;
 
     for (;;)
       {
+#ifdef _GLIBCXX_USE_PURE_STDIO
+	const std::streamsize __ret = fwrite(__file, 1, __nleft, __file);
+#else
 	const std::streamsize __ret = write(__fd, __s, __nleft);
+#endif
 	if (__ret == -1L && errno == EINTR)
 	  continue;
 	if (__ret == -1L)
@@ -133,7 +141,7 @@ namespace
     return __n - __nleft;
   }
 
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_PURE_STDIO)
   // Wrapper handling partial writev.
   static std::streamsize
   xwritev(int __fd, const char* __s1, std::streamsize __n1,
@@ -286,9 +294,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __basic_file<char>::is_open() const throw ()
   { return _M_cfile != 0; }
 
+#ifndef _GLIBCCXX_USE_PURE_STDIO
   int
   __basic_file<char>::fd() throw ()
   { return fileno(_M_cfile); }
+#endif
 
   __c_file*
   __basic_file<char>::file() throw ()
@@ -315,28 +325,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
     streamsize __ret;
     do
+#ifdef _GLIBCXX_USE_PURE_STDIO
+      __ret = fread(__s, 1, __n, this->file());
+#else
       __ret = read(this->fd(), __s, __n);
+#endif
     while (__ret == -1L && errno == EINTR);
     return __ret;
   }
 
   streamsize
   __basic_file<char>::xsputn(const char* __s, streamsize __n)
-  { return xwrite(this->fd(), __s, __n); }
+  {
+#ifdef _GLIBCXX_USE_PURE_STDIO
+    return xwrite(this->file(), __s, __n);
+#else
+    return xwrite(this->fd(), __s, __n);
+#endif
+  }
 
   streamsize
   __basic_file<char>::xsputn_2(const char* __s1, streamsize __n1,
 			       const char* __s2, streamsize __n2)
   {
     streamsize __ret = 0;
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_PURE_STDIO)
     __ret = xwritev(this->fd(), __s1, __n1, __s2, __n2);
 #else
     if (__n1)
+#ifdef _GLIBCXX_USE_PURE_STDIO
+      __ret = xwrite(this->file(), __s1, __n1);
+#else
       __ret = xwrite(this->fd(), __s1, __n1);
+#endif
 
     if (__ret == __n1)
+#ifdef _GLIBCXX_USE_PURE_STDIO
+      __ret += xwrite(this->file(), __s2, __n2);
+#else
       __ret += xwrite(this->fd(), __s2, __n2);
+#endif
 #endif
     return __ret;
   }
@@ -350,7 +378,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     if (__off > numeric_limits<off_t>::max()
 	|| __off < numeric_limits<off_t>::min())
       return -1L;
+#ifdef _GLIBCXX_USE_PURE_STDIO
+    return fseek(this->file(), __off, __way);
+#else
     return lseek(this->fd(), __off, __way);
+#endif
 #endif
   }
 
@@ -361,7 +393,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   streamsize
   __basic_file<char>::showmanyc()
   {
-#ifndef _GLIBCXX_NO_IOCTL
+#if !defined(_GLIBCXX_NO_IOCTL) && !defined(_GLIBCXX_USE_PURE_STDIO)
 #ifdef FIONREAD
     // Pipes and sockets.
     int __num = 0;
@@ -371,7 +403,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 #endif
 
-#ifdef _GLIBCXX_HAVE_POLL
+#if defined(_GLIBCXX_HAVE_POLL) && !defined(_GLIBCXX_USE_PURE_STDIO)
     // Cheap test.
     struct pollfd __pfd[1];
     __pfd[0].fd = this->fd();
@@ -395,8 +427,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct stat __buffer;
     const int __err = fstat(this->fd(), &__buffer);
     if (!__err && _GLIBCXX_ISREG(__buffer.st_mode))
+#ifdef _GLIBCXX_USE_PURE_STDIO
+      return __buffer.st_size - fseek(this->file(), 0, ios_base::cur);
+#else
       return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
 #endif
+#endif
 #endif
     return 0;
   }
diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index cbfdf4c6bad..733154d070b 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -179,6 +179,7 @@ GLIBCXX_ENABLE_EXTERN_TEMPLATE([yes])
 GLIBCXX_ENABLE_PYTHON
 GLIBCXX_ENABLE_WERROR([no])
 GLIBCXX_ENABLE_VTABLE_VERIFY([no])
+GLIBCXX_ENABLE_PURE_STDIO
 
 # Checks for operating systems support that doesn't require linking.
 GLIBCXX_CHECK_STDIO_PROTO
-- 
2.29.2

[PATCH 2/2] Regenerate libstdc++-v3 autoconf files

[Keith Packard]

These are the changes to autoconf files for the pure-stdio patch

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 libstdc++-v3/config.h.in |  3 +++
 libstdc++-v3/configure   | 23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index 72faabfb2c1..76b1c97d2b5 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -1022,6 +1022,9 @@
 /* Define if POSIX read/write locks are available in <gthr.h>. */
 #undef _GLIBCXX_USE_PTHREAD_RWLOCK_T
 
+/* Define to restrict code to stdio APIs. */
+#undef _GLIBCXX_USE_PURE_STDIO
+
 /* Define if /dev/random and /dev/urandom are available for the random_device
    of TR1 (Chapter 5.1). */
 #undef _GLIBCXX_USE_RANDOM_TR1
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index d128de2f186..5647c986831 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -929,6 +929,7 @@ enable_extern_template
 with_python_dir
 enable_werror
 enable_vtable_verify
+enable_libstdcxx_pure_stdio
 enable_libstdcxx_time
 enable_tls
 enable_rpath
@@ -1632,6 +1633,8 @@ Optional Features:
                           enable extern template [default=yes]
   --enable-werror         turns on -Werror [default=no]
   --enable-vtable-verify  enable vtable verify [default=no]
+  --enable-libstdcxx-pure-stdio
+                          use only stdio APIs [default=]
   --enable-libstdcxx-time[=KIND]
                           use KIND for check type [default=auto]
   --enable-tls            Use thread-local storage [default=yes]
@@ -18807,6 +18810,26 @@ fi
 
 
 
+   # Check whether --enable-libstdcxx-pure-stdio was given.
+if test "${enable_libstdcxx_pure_stdio+set}" = set; then :
+  enableval=$enable_libstdcxx_pure_stdio;
+      case "$enableval" in
+       yes|no) ;;
+       *) as_fn_error $? "Argument to enable/disable libstdcxx-pure-stdio must be yes or no" "$LINENO" 5 ;;
+      esac
+
+else
+  enable_libstdcxx_pure_stdio=
+fi
+
+
+  if test $enable_libstdcxx_pure_stdio = yes; then
+
+$as_echo "#define _GLIBCXX_USE_PURE_STDIO 1" >>confdefs.h
+
+  fi
+
+
 # Checks for operating systems support that doesn't require linking.
 
 
-- 
2.29.2

Re: [PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Jonathan Wakely]

On 07/12/20 10:39 -0800, Keith Packard via Libstdc++ wrote:
>This option directs the library to only use simple stdio APIs instead
>of using fileno to get the file descriptor for use with POSIX APIs.

Makes sense, thanks.

I've replaced the gcc@ list with gcc-patches@ in the recipients.

>Signed-off-by: Keith Packard <keithp@keithp.com>
>---
> libstdc++-v3/ChangeLog                     | 13 ++++++
> libstdc++-v3/acinclude.m4                  | 13 ++++++
> libstdc++-v3/config/io/basic_file_stdio.cc | 46 +++++++++++++++++++---
> libstdc++-v3/configure.ac                  |  1 +
> 4 files changed, 68 insertions(+), 5 deletions(-)
>
>diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
>index d3ee18ed7d1..7f6b0697534 100644
>--- a/libstdc++-v3/ChangeLog
>+++ b/libstdc++-v3/ChangeLog
>@@ -1147,6 +1147,19 @@
> 	* include/bits/uniform_int_dist.h (uniform_int_distribution::_S_nd):
> 	Use qualified-id to refer to static member functions.
>
>+2020-11-04  Keith Packard  <keithp@keithp.com>
>+
>+	* acinclude.m4: Add GLIBCXX_ENABLE_PURE_STDIO for
>+	--enable-libstdcxx-pure-stdio
>+	* configure.ac: Use GLIBCXX_ENABLE_PURE_STDIO for
>+	--enable-libstdcxx-pure-stdio
>+	* configure: regenerate
>+	* config.h.in: regenerate
>+	* config/io/basic_file_stdio.cc: Add support for
>+	_GLIBCXX_USE_PURE_STDIO. This makes libstdc++ use only defined
>+	stdio entry points for all I/O operations, without direct calls to
>+	underlying POSIX functions.
>+
> 2020-11-03  Jonathan Wakely  <jwakely@redhat.com>

GCC changelog files are autogenerated now, so patches should not touch
them. Just include the ChangeLog entry in the Git commit log (which
will usually end up being quoted in the patch and/or the email body of
the mail to gcc-patches).

I think the right way to do this (or at least, the way that was
intended when basic_file_stdio.cc was added) is to provide a new file
and change GLIBCXX_ENABLE_CSTDIO in acinclude.m4 to use that new file.

The two biggest downsides of that are that it duplicates a lot of the
file (because the diffs for your changes are small) and that the
correct name for your new file is already taken!

I think we should rename basic_file_stdio.{h,cc} to basic_file_posix
to vacate the basic_file_stdio name for what you want.

However, it's rather late in the GCC 11 process to make a change like
that (even though it's really just renaming some files). Would you be
OK waiting until after GCC 11 is released (in 4-5 months) to do it
"properly"? Is this blocking something that would require doing it
sooner?

Re: [PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Keith Packard]

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

Jonathan Wakely <jwakely@redhat.com> writes:

> GCC changelog files are autogenerated now, so patches should not touch
> them. Just include the ChangeLog entry in the Git commit log (which
> will usually end up being quoted in the patch and/or the email body of
> the mail to gcc-patches).

Awesome.

> I think the right way to do this (or at least, the way that was
> intended when basic_file_stdio.cc was added) is to provide a new file
> and change GLIBCXX_ENABLE_CSTDIO in acinclude.m4 to use that new file.
>
> The two biggest downsides of that are that it duplicates a lot of the
> file (because the diffs for your changes are small) and that the
> correct name for your new file is already taken!

I can definitely see a reason to use a separate file when implementing
the basic_file interface on top of something other than stdio, but
this patch doesn't do that -- it only changes the interaction between
basic_file and stdio in a few places.

I think it makes the best long-term sense to leave everything in
basic_file_stdio.cc and avoid having the two implementations diverge in
the future.

> However, it's rather late in the GCC 11 process to make a change like
> that (even though it's really just renaming some files). Would you be
> OK waiting until after GCC 11 is released (in 4-5 months) to do it
> "properly"? Is this blocking something that would require doing it
> sooner?

This patch enables the use of C++ with picolibc, a libc designed for 32-
and 64- bit embedded systems.

Right now, I'm working on getting picolibc support integrated into
Zephyr, which uses toolchains build by crosstool-ng. I've gotten
picolibc support merged to crosstool-ng, but the Zephyr developers are
interested in having a single toolchain support three different libc
implementations (newlib, newlib-nano and picolibc), but that's blocked
on having C++ support available in all three libraries.

So, you're at the bottom of my current dependency graph :-)

I don't particularly need this released in gcc, but I would like to
get patches reviewed and the general approach agreed on so that I can
feel more confident in preparing patches to be applied to gcc in
crosstool-ng itself.

Once that's done, I'll also be able to release new Debian packages of
GCC for embedded ARM and RISC-V and have those include suitable patches
so that we can support embedded C++ development there too. 

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Jonathan Wakely]

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

On 07/12/20 12:36 -0800, Keith Packard wrote:
>Jonathan Wakely <jwakely@redhat.com> writes:
>
>> GCC changelog files are autogenerated now, so patches should not touch
>> them. Just include the ChangeLog entry in the Git commit log (which
>> will usually end up being quoted in the patch and/or the email body of
>> the mail to gcc-patches).
>
>Awesome.
>
>> I think the right way to do this (or at least, the way that was
>> intended when basic_file_stdio.cc was added) is to provide a new file
>> and change GLIBCXX_ENABLE_CSTDIO in acinclude.m4 to use that new file.
>>
>> The two biggest downsides of that are that it duplicates a lot of the
>> file (because the diffs for your changes are small) and that the
>> correct name for your new file is already taken!
>
>I can definitely see a reason to use a separate file when implementing
>the basic_file interface on top of something other than stdio, but
>this patch doesn't do that -- it only changes the interaction between
>basic_file and stdio in a few places.
>
>I think it makes the best long-term sense to leave everything in
>basic_file_stdio.cc and avoid having the two implementations diverge in
>the future.
>
>> However, it's rather late in the GCC 11 process to make a change like
>> that (even though it's really just renaming some files). Would you be
>> OK waiting until after GCC 11 is released (in 4-5 months) to do it
>> "properly"? Is this blocking something that would require doing it
>> sooner?
>
>This patch enables the use of C++ with picolibc, a libc designed for 32-
>and 64- bit embedded systems.
>
>Right now, I'm working on getting picolibc support integrated into
>Zephyr, which uses toolchains build by crosstool-ng. I've gotten
>picolibc support merged to crosstool-ng, but the Zephyr developers are
>interested in having a single toolchain support three different libc
>implementations (newlib, newlib-nano and picolibc), but that's blocked
>on having C++ support available in all three libraries.
>
>So, you're at the bottom of my current dependency graph :-)
>
>I don't particularly need this released in gcc, but I would like to
>get patches reviewed and the general approach agreed on so that I can
>feel more confident in preparing patches to be applied to gcc in
>crosstool-ng itself.
>
>Once that's done, I'll also be able to release new Debian packages of
>GCC for embedded ARM and RISC-V and have those include suitable patches
>so that we can support embedded C++ development there too.

OK. In principle, changes to avoid using the POSIX APIs are definitely
fine. I would like to combine your new configure switch with the
existing --enable-cstdio one though.

How about the attached change for acinclude.m4 which would allow you
to do --enable-cstdio=stdio_pure? (It also adds "stdio_posix" as a
more accurate alternative spelling of the current "stdio" option.)




[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1628 bytes --]

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index fcd9ea3d23a..535ffd8682f 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2862,24 +2862,30 @@ AC_DEFUN([GLIBCXX_ENABLE_PARALLEL], [
 
 
 dnl
-dnl Check for which I/O library to use:  stdio, or something specific.
+dnl Check for which I/O library to use:  stdio and POSIX, or pure stdio.
 dnl
-dnl Default is stdio.
+dnl Default is stdio_posix.
 dnl
 AC_DEFUN([GLIBCXX_ENABLE_CSTDIO], [
   AC_MSG_CHECKING([for underlying I/O to use])
   GLIBCXX_ENABLE(cstdio,stdio,[[[=PACKAGE]]],
-    [use target-specific I/O package], [permit stdio])
+    [use target-specific I/O package], [permit stdio|stdio_posix|stdio_pure])
 
-  # Now that libio has been removed, you can have any color you want as long
-  # as it's black.  This is one big no-op until other packages are added, but
-  # showing the framework never hurts.
+  # The only available I/O model is based on stdio, via basic_file_stdio.
+  # The default "stdio" is actually "stdio + POSIX" because it uses fdopen(3)
+  # to get a file descriptor and then uses read(3) and write(3) with it.
+  # The "stdio_pure" model doesn't use fdopen and only uses FILE* for I/O.
   case ${enable_cstdio} in
-    stdio)
+    stdio*)
       CSTDIO_H=config/io/c_io_stdio.h
       BASIC_FILE_H=config/io/basic_file_stdio.h
       BASIC_FILE_CC=config/io/basic_file_stdio.cc
       AC_MSG_RESULT(stdio)
+
+      if test "x$enable_cstdio" = "xstdio_pure" ; then
+	AC_DEFINE(_GLIBCXX_USE_PURE_STDIO, 1,
+		  [Define to restrict std::__basic_file<> to stdio APIs.])
+      fi
       ;;
   esac
 

Re: [PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Keith Packard]

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

Jonathan Wakely <jwakely@redhat.com> writes:

> OK. In principle, changes to avoid using the POSIX APIs are definitely
> fine. I would like to combine your new configure switch with the
> existing --enable-cstdio one though.
>
> How about the attached change for acinclude.m4 which would allow you
> to do --enable-cstdio=stdio_pure? (It also adds "stdio_posix" as a
> more accurate alternative spelling of the current "stdio" option.)

Oh, that's very nice. I'll integrate that into my patch and send it
along.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 1/2] libstdc++: Add --enable-pure-stdio-libstdcxx option

[Jonathan Wakely]

On 09/12/20 08:32 -0800, Keith Packard via Libstdc++ wrote:
>Jonathan Wakely <jwakely@redhat.com> writes:
>
>> OK. In principle, changes to avoid using the POSIX APIs are definitely
>> fine. I would like to combine your new configure switch with the
>> existing --enable-cstdio one though.
>>
>> How about the attached change for acinclude.m4 which would allow you
>> to do --enable-cstdio=stdio_pure? (It also adds "stdio_posix" as a
>> more accurate alternative spelling of the current "stdio" option.)
>
>Oh, that's very nice. I'll integrate that into my patch and send it
>along.

Great. We already have a *lot* of configure options, so piggybacking
onto an existing one that's related seems preferable to adding Yet
Another of them.

[PATCH 0/2] Support libc with stdio-only I/O in libstdc++

[Keith Packard]

The current libstdc++ basic_file_stdio.cc code assumes a POSIX API
underneath the stdio implementation provided by the host libc. This
means that the host must provide a fairly broad POSIX file API,
including read, write, open, close, lseek and ioctl.

This patch changes basic_file_stdio.cc to only use basic ANSI-C stdio
functions, allowing it to be used with libc implementations like
picolibc which may not have a POSIX operating system underneath.

This is version 2 of the patch. This version uses the existing
--enable-stdio option, extending that to add 'stdio_pure' for this
mode using a patch created by Jonathan Wakely <jwakely@redhat.com>

[PATCH 1/2] libstdc++: Add --enable-stdio=stdio_pure option [v2]

[Keith Packard]

This option directs the library to only use simple stdio APIs instead
of using fileno to get the file descriptor for use with POSIX APIs.

Aided-by: Jonathan Wakely <jwakely@redhat.com>
Signed-off-by: Keith Packard <keithp@keithp.com>

-----

v2:
	Switch from --enable-libstdcxx-pure-stdio to
	--enable-stdio=stdio_pure based on a patch from Jonathan
	Wakely <jwakely@redhat.com>.
---
 libstdc++-v3/acinclude.m4                  | 20 ++++++----
 libstdc++-v3/config/io/basic_file_stdio.cc | 46 +++++++++++++++++++---
 2 files changed, 54 insertions(+), 12 deletions(-)

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index fcd9ea3d23a..703962ce2d7 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2862,24 +2862,30 @@ AC_DEFUN([GLIBCXX_ENABLE_PARALLEL], [
 
 
 dnl
-dnl Check for which I/O library to use:  stdio, or something specific.
+dnl Check for which I/O library to use:  stdio and POSIX, or pure stdio.
 dnl
-dnl Default is stdio.
+dnl Default is stdio_posix.
 dnl
 AC_DEFUN([GLIBCXX_ENABLE_CSTDIO], [
   AC_MSG_CHECKING([for underlying I/O to use])
   GLIBCXX_ENABLE(cstdio,stdio,[[[=PACKAGE]]],
-    [use target-specific I/O package], [permit stdio])
+    [use target-specific I/O package], [permit stdio|stdio_posix|stdio_pure])
 
-  # Now that libio has been removed, you can have any color you want as long
-  # as it's black.  This is one big no-op until other packages are added, but
-  # showing the framework never hurts.
+  # The only available I/O model is based on stdio, via basic_file_stdio.
+  # The default "stdio" is actually "stdio + POSIX" because it uses fdopen(3)
+  # to get a file descriptor and then uses read(3) and write(3) with it.
+  # The "stdio_pure" model doesn't use fdopen and only uses FILE* for I/O.
   case ${enable_cstdio} in
-    stdio)
+    stdio*)
       CSTDIO_H=config/io/c_io_stdio.h
       BASIC_FILE_H=config/io/basic_file_stdio.h
       BASIC_FILE_CC=config/io/basic_file_stdio.cc
       AC_MSG_RESULT(stdio)
+
+      if test "x$enable_cstdio" = "xstdio_pure" ; then
+	AC_DEFINE(_GLIBCXX_USE_STDIO_PURE, 1,
+		  [Define to restrict std::__basic_file<> to stdio APIs.])
+      fi
       ;;
   esac
 
diff --git a/libstdc++-v3/config/io/basic_file_stdio.cc b/libstdc++-v3/config/io/basic_file_stdio.cc
index ba830fb9e97..eedffb017b6 100644
--- a/libstdc++-v3/config/io/basic_file_stdio.cc
+++ b/libstdc++-v3/config/io/basic_file_stdio.cc
@@ -111,13 +111,21 @@ namespace
 
   // Wrapper handling partial write.
   static std::streamsize
+#ifdef _GLIBCXX_USE_STDIO_PURE
+  xwrite(FILE *__file, const char* __s, std::streamsize __n)
+#else
   xwrite(int __fd, const char* __s, std::streamsize __n)
+#endif
   {
     std::streamsize __nleft = __n;
 
     for (;;)
       {
+#ifdef _GLIBCXX_USE_STDIO_PURE
+	const std::streamsize __ret = fwrite(__file, 1, __nleft, __file);
+#else
 	const std::streamsize __ret = write(__fd, __s, __nleft);
+#endif
 	if (__ret == -1L && errno == EINTR)
 	  continue;
 	if (__ret == -1L)
@@ -133,7 +141,7 @@ namespace
     return __n - __nleft;
   }
 
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_STDIO_PURE)
   // Wrapper handling partial writev.
   static std::streamsize
   xwritev(int __fd, const char* __s1, std::streamsize __n1,
@@ -286,9 +294,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __basic_file<char>::is_open() const throw ()
   { return _M_cfile != 0; }
 
+#ifndef _GLIBCCXX_USE_STDIO_PURE
   int
   __basic_file<char>::fd() throw ()
   { return fileno(_M_cfile); }
+#endif
 
   __c_file*
   __basic_file<char>::file() throw ()
@@ -315,28 +325,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
     streamsize __ret;
     do
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret = fread(__s, 1, __n, this->file());
+#else
       __ret = read(this->fd(), __s, __n);
+#endif
     while (__ret == -1L && errno == EINTR);
     return __ret;
   }
 
   streamsize
   __basic_file<char>::xsputn(const char* __s, streamsize __n)
-  { return xwrite(this->fd(), __s, __n); }
+  {
+#ifdef _GLIBCXX_USE_STDIO_PURE
+    return xwrite(this->file(), __s, __n);
+#else
+    return xwrite(this->fd(), __s, __n);
+#endif
+  }
 
   streamsize
   __basic_file<char>::xsputn_2(const char* __s1, streamsize __n1,
 			       const char* __s2, streamsize __n2)
   {
     streamsize __ret = 0;
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_STDIO_PURE)
     __ret = xwritev(this->fd(), __s1, __n1, __s2, __n2);
 #else
     if (__n1)
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret = xwrite(this->file(), __s1, __n1);
+#else
       __ret = xwrite(this->fd(), __s1, __n1);
+#endif
 
     if (__ret == __n1)
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret += xwrite(this->file(), __s2, __n2);
+#else
       __ret += xwrite(this->fd(), __s2, __n2);
+#endif
 #endif
     return __ret;
   }
@@ -350,7 +378,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     if (__off > numeric_limits<off_t>::max()
 	|| __off < numeric_limits<off_t>::min())
       return -1L;
+#ifdef _GLIBCXX_USE_STDIO_PURE
+    return fseek(this->file(), __off, __way);
+#else
     return lseek(this->fd(), __off, __way);
+#endif
 #endif
   }
 
@@ -361,7 +393,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   streamsize
   __basic_file<char>::showmanyc()
   {
-#ifndef _GLIBCXX_NO_IOCTL
+#if !defined(_GLIBCXX_NO_IOCTL) && !defined(_GLIBCXX_USE_STDIO_PURE)
 #ifdef FIONREAD
     // Pipes and sockets.
     int __num = 0;
@@ -371,7 +403,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 #endif
 
-#ifdef _GLIBCXX_HAVE_POLL
+#if defined(_GLIBCXX_HAVE_POLL) && !defined(_GLIBCXX_USE_STDIO_PURE)
     // Cheap test.
     struct pollfd __pfd[1];
     __pfd[0].fd = this->fd();
@@ -395,8 +427,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct stat __buffer;
     const int __err = fstat(this->fd(), &__buffer);
     if (!__err && _GLIBCXX_ISREG(__buffer.st_mode))
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      return __buffer.st_size - fseek(this->file(), 0, ios_base::cur);
+#else
       return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
 #endif
+#endif
 #endif
     return 0;
   }
-- 
2.29.2

[PATCH 2/2] Regenerate libstdc++-v3 autoconf files

[Keith Packard]

These are the changes to autoconf files for the stdio_pure patch

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 libstdc++-v3/config.h.in |  3 +++
 libstdc++-v3/configure   | 17 ++++++++++++-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/libstdc++-v3/config.h.in b/libstdc++-v3/config.h.in
index 72faabfb2c1..c0c166715cb 100644
--- a/libstdc++-v3/config.h.in
+++ b/libstdc++-v3/config.h.in
@@ -1022,6 +1022,9 @@
 /* Define if POSIX read/write locks are available in <gthr.h>. */
 #undef _GLIBCXX_USE_PTHREAD_RWLOCK_T
 
+/* Define to restrict std::__basic_file<> to stdio APIs. */
+#undef _GLIBCXX_USE_STDIO_PURE
+
 /* Define if /dev/random and /dev/urandom are available for the random_device
    of TR1 (Chapter 5.1). */
 #undef _GLIBCXX_USE_RANDOM_TR1
diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure
index d128de2f186..36c61a77d07 100755
--- a/libstdc++-v3/configure
+++ b/libstdc++-v3/configure
@@ -16376,7 +16376,7 @@ $as_echo_n "checking for underlying I/O to use... " >&6; }
 if test "${enable_cstdio+set}" = set; then :
   enableval=$enable_cstdio;
       case "$enableval" in
-       stdio) ;;
+       stdio|stdio_posix|stdio_pure) ;;
        *) as_fn_error $? "Unknown argument to enable/disable cstdio" "$LINENO" 5 ;;
 	  	        esac
 
@@ -16386,16 +16386,23 @@ fi
 
 
 
-  # Now that libio has been removed, you can have any color you want as long
-  # as it's black.  This is one big no-op until other packages are added, but
-  # showing the framework never hurts.
+  # The only available I/O model is based on stdio, via basic_file_stdio.
+  # The default "stdio" is actually "stdio + POSIX" because it uses fdopen(3)
+  # to get a file descriptor and then uses read(3) and write(3) with it.
+  # The "stdio_pure" model doesn't use fdopen and only uses FILE* for I/O.
   case ${enable_cstdio} in
-    stdio)
+    stdio*)
       CSTDIO_H=config/io/c_io_stdio.h
       BASIC_FILE_H=config/io/basic_file_stdio.h
       BASIC_FILE_CC=config/io/basic_file_stdio.cc
       { $as_echo "$as_me:${as_lineno-$LINENO}: result: stdio" >&5
 $as_echo "stdio" >&6; }
+
+      if test "x$enable_cstdio" = "xstdio_pure" ; then
+
+$as_echo "#define _GLIBCXX_USE_STDIO_PURE 1" >>confdefs.h
+
+      fi
       ;;
   esac
 
-- 
2.29.2

Re: [PATCH 1/2] libstdc++: Add --enable-stdio=stdio_pure option [v2]

[Jonathan Wakely]

On 09/12/20 18:46 -0800, Keith Packard wrote:
>This option directs the library to only use simple stdio APIs instead
>of using fileno to get the file descriptor for use with POSIX APIs.

This looks fine to me, even at this stage of GCC 11 (it doesn't affect
the default configurations, just adds a new one that nobody is going
to use unless they ask for it explicitly).

I'll do a bit more testing and push it next week.

Thanks for the patch!

Re: [PATCH 1/2] libstdc++: Add --enable-stdio=stdio_pure option [v2]

[Keith Packard]

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

Jonathan Wakely <jwakely@redhat.com> writes:

> I'll do a bit more testing and push it next week.

That's awesome news. Thanks so much for you help; I'm looking forward to
having real C++ support for my embedded customers!

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 1/2] libstdc++: Add --enable-stdio=stdio_pure option [v2]

[Jonathan Wakely]

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

On 10/12/20 12:56 -0800, Keith Packard via Libstdc++ wrote:
>Jonathan Wakely <jwakely@redhat.com> writes:
>
>> I'll do a bit more testing and push it next week.
>
>That's awesome news. Thanks so much for you help; I'm looking forward to
>having real C++ support for my embedded customers!

Pushed to master.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 6929 bytes --]

commit 75aee072696a711f3e5b3bd1ab1f2b10fef4c7dd
Author: Keith Packard <keithp@keithp.com>
Date:   Tue Dec 15 17:39:24 2020

    libstdc++: Support libc with stdio-only I/O in libstdc++
    
    The current libstdc++ basic_file_stdio.cc code assumes a POSIX API
    underneath the stdio implementation provided by the host libc. This
    means that the host must provide a fairly broad POSIX file API,
    including read, write, open, close, lseek and ioctl.
    
    This patch changes basic_file_stdio.cc to only use basic ANSI-C stdio
    functions, allowing it to be used with libc implementations like
    picolibc which may not have a POSIX operating system underneath.
    
    This is enabled by a new --enable-cstdio=stdio_pure configure option.
    
    Aided-by: Jonathan Wakely <jwakely@redhat.com>
    Signed-off-by: Keith Packard <keithp@keithp.com>
    
    libstdc++-v3/ChangeLog:
    
            * acinclude.m4 (GLIBCXX_ENABLE_CSTDIO): Allow "stdio_pure"
            option and define _GLIBCXX_USE_PURE_STDIO when it is used. Also
            add "stdio_posix" option as an alias for "stdio".
            * config/io/basic_file_stdio.cc [_GLIBCXX_USE_PURE_STDIO]: Only
            use defined stdio entry points for all I/O operations, without
            direct calls to underlying POSIX functions.
            * config.h.in: Regenerate.
            * configure: Regenerate.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index 61191812c92..df8be3bf805 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -2868,24 +2868,30 @@ AC_DEFUN([GLIBCXX_ENABLE_PARALLEL], [
 
 
 dnl
-dnl Check for which I/O library to use:  stdio, or something specific.
+dnl Check for which I/O library to use:  stdio and POSIX, or pure stdio.
 dnl
-dnl Default is stdio.
+dnl Default is stdio_posix.
 dnl
 AC_DEFUN([GLIBCXX_ENABLE_CSTDIO], [
   AC_MSG_CHECKING([for underlying I/O to use])
   GLIBCXX_ENABLE(cstdio,stdio,[[[=PACKAGE]]],
-    [use target-specific I/O package], [permit stdio])
+    [use target-specific I/O package], [permit stdio|stdio_posix|stdio_pure])
 
-  # Now that libio has been removed, you can have any color you want as long
-  # as it's black.  This is one big no-op until other packages are added, but
-  # showing the framework never hurts.
+  # The only available I/O model is based on stdio, via basic_file_stdio.
+  # The default "stdio" is actually "stdio + POSIX" because it uses fdopen(3)
+  # to get a file descriptor and then uses read(3) and write(3) with it.
+  # The "stdio_pure" model doesn't use fdopen and only uses FILE* for I/O.
   case ${enable_cstdio} in
-    stdio)
+    stdio*)
       CSTDIO_H=config/io/c_io_stdio.h
       BASIC_FILE_H=config/io/basic_file_stdio.h
       BASIC_FILE_CC=config/io/basic_file_stdio.cc
       AC_MSG_RESULT(stdio)
+
+      if test "x$enable_cstdio" = "xstdio_pure" ; then
+	AC_DEFINE(_GLIBCXX_USE_STDIO_PURE, 1,
+		  [Define to restrict std::__basic_file<> to stdio APIs.])
+      fi
       ;;
   esac
 
diff --git a/libstdc++-v3/config/io/basic_file_stdio.cc b/libstdc++-v3/config/io/basic_file_stdio.cc
index ba830fb9e97..eedffb017b6 100644
--- a/libstdc++-v3/config/io/basic_file_stdio.cc
+++ b/libstdc++-v3/config/io/basic_file_stdio.cc
@@ -111,13 +111,21 @@ namespace
 
   // Wrapper handling partial write.
   static std::streamsize
+#ifdef _GLIBCXX_USE_STDIO_PURE
+  xwrite(FILE *__file, const char* __s, std::streamsize __n)
+#else
   xwrite(int __fd, const char* __s, std::streamsize __n)
+#endif
   {
     std::streamsize __nleft = __n;
 
     for (;;)
       {
+#ifdef _GLIBCXX_USE_STDIO_PURE
+	const std::streamsize __ret = fwrite(__file, 1, __nleft, __file);
+#else
 	const std::streamsize __ret = write(__fd, __s, __nleft);
+#endif
 	if (__ret == -1L && errno == EINTR)
 	  continue;
 	if (__ret == -1L)
@@ -133,7 +141,7 @@ namespace
     return __n - __nleft;
   }
 
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_STDIO_PURE)
   // Wrapper handling partial writev.
   static std::streamsize
   xwritev(int __fd, const char* __s1, std::streamsize __n1,
@@ -286,9 +294,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __basic_file<char>::is_open() const throw ()
   { return _M_cfile != 0; }
 
+#ifndef _GLIBCCXX_USE_STDIO_PURE
   int
   __basic_file<char>::fd() throw ()
   { return fileno(_M_cfile); }
+#endif
 
   __c_file*
   __basic_file<char>::file() throw ()
@@ -315,28 +325,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   {
     streamsize __ret;
     do
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret = fread(__s, 1, __n, this->file());
+#else
       __ret = read(this->fd(), __s, __n);
+#endif
     while (__ret == -1L && errno == EINTR);
     return __ret;
   }
 
   streamsize
   __basic_file<char>::xsputn(const char* __s, streamsize __n)
-  { return xwrite(this->fd(), __s, __n); }
+  {
+#ifdef _GLIBCXX_USE_STDIO_PURE
+    return xwrite(this->file(), __s, __n);
+#else
+    return xwrite(this->fd(), __s, __n);
+#endif
+  }
 
   streamsize
   __basic_file<char>::xsputn_2(const char* __s1, streamsize __n1,
 			       const char* __s2, streamsize __n2)
   {
     streamsize __ret = 0;
-#ifdef _GLIBCXX_HAVE_WRITEV
+#if defined(_GLIBCXX_HAVE_WRITEV) && !defined(_GLIBCXX_USE_STDIO_PURE)
     __ret = xwritev(this->fd(), __s1, __n1, __s2, __n2);
 #else
     if (__n1)
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret = xwrite(this->file(), __s1, __n1);
+#else
       __ret = xwrite(this->fd(), __s1, __n1);
+#endif
 
     if (__ret == __n1)
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      __ret += xwrite(this->file(), __s2, __n2);
+#else
       __ret += xwrite(this->fd(), __s2, __n2);
+#endif
 #endif
     return __ret;
   }
@@ -350,7 +378,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     if (__off > numeric_limits<off_t>::max()
 	|| __off < numeric_limits<off_t>::min())
       return -1L;
+#ifdef _GLIBCXX_USE_STDIO_PURE
+    return fseek(this->file(), __off, __way);
+#else
     return lseek(this->fd(), __off, __way);
+#endif
 #endif
   }
 
@@ -361,7 +393,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   streamsize
   __basic_file<char>::showmanyc()
   {
-#ifndef _GLIBCXX_NO_IOCTL
+#if !defined(_GLIBCXX_NO_IOCTL) && !defined(_GLIBCXX_USE_STDIO_PURE)
 #ifdef FIONREAD
     // Pipes and sockets.
     int __num = 0;
@@ -371,7 +403,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
 #endif
 
-#ifdef _GLIBCXX_HAVE_POLL
+#if defined(_GLIBCXX_HAVE_POLL) && !defined(_GLIBCXX_USE_STDIO_PURE)
     // Cheap test.
     struct pollfd __pfd[1];
     __pfd[0].fd = this->fd();
@@ -395,8 +427,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     struct stat __buffer;
     const int __err = fstat(this->fd(), &__buffer);
     if (!__err && _GLIBCXX_ISREG(__buffer.st_mode))
+#ifdef _GLIBCXX_USE_STDIO_PURE
+      return __buffer.st_size - fseek(this->file(), 0, ios_base::cur);
+#else
       return __buffer.st_size - lseek(this->fd(), 0, ios_base::cur);
 #endif
+#endif
 #endif
     return 0;
   }