Problem building libstdc++ for the avr target

Problem building libstdc++ for the avr target

[Vladimir V]

Dear libstdc++ developers,

I have recently experimented with building 'hosted' variant of the
libstdc++ for the avr target.
Practically, it looks like, after minor extension of the avr-libc, it can
be built.
Unfortunately, as a temporary workaround I had to disable dlopen test in
the configure script as I encountered the error:

'checking for shl_load... configure: error: Link tests are not allowed
after GCC_NO_EXECUTABLES'

while building with '--with-avrlibc' flag. The issue is described in
https://gcc.gnu.org/legacy-ml/gcc/2008-03/msg00515.html
and was resolved for the newlib. The fix was not applied for the avr-libc
targets although the rationale behind the solution
seems to be applicable there as well. I would like to ask what do you think
about extending the scope of the solution to work
with avr-libc based builds as well?

Thank you for considering my question.

With best regards,
Vladimir

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 07/12/20 18:32 +0100, Vladimir V via Libstdc++ wrote:
>Dear libstdc++ developers,
>
>I have recently experimented with building 'hosted' variant of the
>libstdc++ for the avr target.
>Practically, it looks like, after minor extension of the avr-libc, it can
>be built.
>Unfortunately, as a temporary workaround I had to disable dlopen test in
>the configure script as I encountered the error:
>
>'checking for shl_load... configure: error: Link tests are not allowed
>after GCC_NO_EXECUTABLES'
>
>while building with '--with-avrlibc' flag. The issue is described in
>https://gcc.gnu.org/legacy-ml/gcc/2008-03/msg00515.html
>and was resolved for the newlib. The fix was not applied for the avr-libc
>targets although the rationale behind the solution
>seems to be applicable there as well. I would like to ask what do you think
>about extending the scope of the solution to work
>with avr-libc based builds as well?

Yes, I think it makes sense.

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 07/12/20 20:30 +0000, Jonathan Wakely wrote:
>On 07/12/20 18:32 +0100, Vladimir V via Libstdc++ wrote:
>>Dear libstdc++ developers,
>>
>>I have recently experimented with building 'hosted' variant of the
>>libstdc++ for the avr target.
>>Practically, it looks like, after minor extension of the avr-libc, it can
>>be built.
>>Unfortunately, as a temporary workaround I had to disable dlopen test in
>>the configure script as I encountered the error:
>>
>>'checking for shl_load... configure: error: Link tests are not allowed
>>after GCC_NO_EXECUTABLES'
>>
>>while building with '--with-avrlibc' flag. The issue is described in
>>https://gcc.gnu.org/legacy-ml/gcc/2008-03/msg00515.html
>>and was resolved for the newlib. The fix was not applied for the avr-libc
>>targets although the rationale behind the solution
>>seems to be applicable there as well. I would like to ask what do you think
>>about extending the scope of the solution to work
>>with avr-libc based builds as well?
>
>Yes, I think it makes sense.

I forgot to add that be95b3555aa82872565a1e1ea85ff826a20c8e2e was the
commit that added the check for ${with_newlib}.

Re: Problem building libstdc++ for the avr target

[Keith Packard]

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

Jonathan Wakely via Libstdc++ <libstdc++@gcc.gnu.org> writes:

>>and was resolved for the newlib. The fix was not applied for the avr-libc
>>targets although the rationale behind the solution
>>seems to be applicable there as well. I would like to ask what do you think
>>about extending the scope of the solution to work
>>with avr-libc based builds as well?
>
> Yes, I think it makes sense.

I've been working on making libstdc++ work with picolibc, which shares a
bunch of stdio code with avr-libc. I haven't tried my patches with
avr-libc, but I wonder if that would be a good thing to do? With those
patches, I'm able to use libstdc++'s iostream.

-- 
-keith

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

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 07/12/20 12:41 -0800, Keith Packard via Libstdc++ wrote:
>Jonathan Wakely via Libstdc++ <libstdc++@gcc.gnu.org> writes:
>
>>>and was resolved for the newlib. The fix was not applied for the avr-libc
>>>targets although the rationale behind the solution
>>>seems to be applicable there as well. I would like to ask what do you think
>>>about extending the scope of the solution to work
>>>with avr-libc based builds as well?
>>
>> Yes, I think it makes sense.
>
>I've been working on making libstdc++ work with picolibc, which shares a
>bunch of stdio code with avr-libc. I haven't tried my patches with
>avr-libc, but I wonder if that would be a good thing to do? With those
>patches, I'm able to use libstdc++'s iostream.

That might be interesting to try.

I'm very much in favour of changes to libstdc++ that allow it to be
used on small systems. While some C++ features may not be suitable for
those systems, I think there's a lot of the standard library that is
still useful, but isn't part of the current "freestanding" subset. And
I'd like libstdc++ to be able to scale from very small to very large,
rather than needing to fork it to meet those needs.

Re: Problem building libstdc++ for the avr target

[Vladimir V]

It looks like the existing stdio was at least sufficient to build the
iostream.
I was mostly focusing on basic headers so haven't yet tested if it is
functional.




пн, 7 дек. 2020 г. в 21:41, Keith Packard <keithp@keithp.com>:

> Jonathan Wakely via Libstdc++ <libstdc++@gcc.gnu.org> writes:
>
> >>and was resolved for the newlib. The fix was not applied for the avr-libc
> >>targets although the rationale behind the solution
> >>seems to be applicable there as well. I would like to ask what do you
> think
> >>about extending the scope of the solution to work
> >>with avr-libc based builds as well?
> >
> > Yes, I think it makes sense.
>
> I've been working on making libstdc++ work with picolibc, which shares a
> bunch of stdio code with avr-libc. I haven't tried my patches with
> avr-libc, but I wonder if that would be a good thing to do? With those
> patches, I'm able to use libstdc++'s iostream.
>
> --
> -keith
>

Re: Problem building libstdc++ for the avr target

[Keith Packard]

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

Vladimir V <vv.os.swe@gmail.com> writes:

> It looks like the existing stdio was at least sufficient to build the
> iostream.
> I was mostly focusing on basic headers so haven't yet tested if it is
> functional.

It "shouldn't" work as iostream uses fileno to get a POSIX file
descriptor, then uses read and write directly, instead of stdio
functions like putc and getc. Presumably it compiles because there are
definitions of all of the relevant interfaces somewhere?

-- 
-keith

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

Re: Problem building libstdc++ for the avr target

[Vladimir V]

The library provides unistd header with a small subset of POSIX io function
declarations. I had to extend it to satisfy build dependencies of the
filesystem.
Might be my patch covered iostream dependencies as well.

Vladimir

вт, 8 дек. 2020 г. в 00:00, Keith Packard <keithp@keithp.com>:

> Vladimir V <vv.os.swe@gmail.com> writes:
>
> > It looks like the existing stdio was at least sufficient to build the
> > iostream.
> > I was mostly focusing on basic headers so haven't yet tested if it is
> > functional.
>
> It "shouldn't" work as iostream uses fileno to get a POSIX file
> descriptor, then uses read and write directly, instead of stdio
> functions like putc and getc. Presumably it compiles because there are
> definitions of all of the relevant interfaces somewhere?
>
> --
> -keith
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you.
I am verifying my patch with the upstream version now.

пн, 7 дек. 2020 г. в 21:32, Jonathan Wakely <jwakely@redhat.com>:

> On 07/12/20 20:30 +0000, Jonathan Wakely wrote:
> >On 07/12/20 18:32 +0100, Vladimir V via Libstdc++ wrote:
> >>Dear libstdc++ developers,
> >>
> >>I have recently experimented with building 'hosted' variant of the
> >>libstdc++ for the avr target.
> >>Practically, it looks like, after minor extension of the avr-libc, it can
> >>be built.
> >>Unfortunately, as a temporary workaround I had to disable dlopen test in
> >>the configure script as I encountered the error:
> >>
> >>'checking for shl_load... configure: error: Link tests are not allowed
> >>after GCC_NO_EXECUTABLES'
> >>
> >>while building with '--with-avrlibc' flag. The issue is described in
> >>https://gcc.gnu.org/legacy-ml/gcc/2008-03/msg00515.html
> >>and was resolved for the newlib. The fix was not applied for the avr-libc
> >>targets although the rationale behind the solution
> >>seems to be applicable there as well. I would like to ask what do you
> think
> >>about extending the scope of the solution to work
> >>with avr-libc based builds as well?
> >
> >Yes, I think it makes sense.
>
> I forgot to add that be95b3555aa82872565a1e1ea85ff826a20c8e2e was the
> commit that added the check for ${with_newlib}.
>
>
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Hello.

While testing with the current upstream I encountered a compilation issue.
Although I build with "--disable-threads" flag the following error occurs:

../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
sleep function known for this target"

Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
prevented the error from happening (in my case with gcc v10.1),
So I would like to ask if the thread.cc should be involved in the build if
the threads support is configured to be disabled?
And if it should, then can the condition be reworked to cover the described
case?

Thank you.

Vladimir

ср, 9 дек. 2020 г. в 01:28, Vladimir V <vv.os.swe@gmail.com>:

> Thank you.
> I am verifying my patch with the upstream version now.
>
> пн, 7 дек. 2020 г. в 21:32, Jonathan Wakely <jwakely@redhat.com>:
>
>> On 07/12/20 20:30 +0000, Jonathan Wakely wrote:
>> >On 07/12/20 18:32 +0100, Vladimir V via Libstdc++ wrote:
>> >>Dear libstdc++ developers,
>> >>
>> >>I have recently experimented with building 'hosted' variant of the
>> >>libstdc++ for the avr target.
>> >>Practically, it looks like, after minor extension of the avr-libc, it
>> can
>> >>be built.
>> >>Unfortunately, as a temporary workaround I had to disable dlopen test in
>> >>the configure script as I encountered the error:
>> >>
>> >>'checking for shl_load... configure: error: Link tests are not allowed
>> >>after GCC_NO_EXECUTABLES'
>> >>
>> >>while building with '--with-avrlibc' flag. The issue is described in
>> >>https://gcc.gnu.org/legacy-ml/gcc/2008-03/msg00515.html
>> >>and was resolved for the newlib. The fix was not applied for the
>> avr-libc
>> >>targets although the rationale behind the solution
>> >>seems to be applicable there as well. I would like to ask what do you
>> think
>> >>about extending the scope of the solution to work
>> >>with avr-libc based builds as well?
>> >
>> >Yes, I think it makes sense.
>>
>> I forgot to add that be95b3555aa82872565a1e1ea85ff826a20c8e2e was the
>> commit that added the check for ${with_newlib}.
>>
>>
>>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 09/12/20 13:32 +0100, Vladimir V wrote:
>Hello.
>
>While testing with the current upstream I encountered a compilation issue.
>Although I build with "--disable-threads" flag the following error occurs:
>
>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
>sleep function known for this target"
>
>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
>prevented the error from happening (in my case with gcc v10.1),
>So I would like to ask if the thread.cc should be involved in the build if
>the threads support is configured to be disabled?

Yes, the file is always built, but which definitions it contains
depends on what is configured for the target.

The std::this_thread::sleep_for and std::this_thread::sleep_until
functions don't actually depend on threads at all. They just sleep.

But that still requires target support, just different support from
threads.

>And if it should, then can the condition be reworked to cover the described
>case?

Yes, I'll do that. Thanks for bringing it to my attention.

I assume we can't use avr-libc's delay functions, because they depend
on the CPU clock frequency, which isn't known when we compile
libstdc++. So I'll just suppress the declarations of those functions
and remove the #error.

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

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

On 09/12/20 12:49 +0000, Jonathan Wakely wrote:
>On 09/12/20 13:32 +0100, Vladimir V wrote:
>>Hello.
>>
>>While testing with the current upstream I encountered a compilation issue.
>>Although I build with "--disable-threads" flag the following error occurs:
>>
>>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
>>sleep function known for this target"
>>
>>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
>>prevented the error from happening (in my case with gcc v10.1),
>>So I would like to ask if the thread.cc should be involved in the build if
>>the threads support is configured to be disabled?
>
>Yes, the file is always built, but which definitions it contains
>depends on what is configured for the target.
>
>The std::this_thread::sleep_for and std::this_thread::sleep_until
>functions don't actually depend on threads at all. They just sleep.
>
>But that still requires target support, just different support from
>threads.
>
>>And if it should, then can the condition be reworked to cover the described
>>case?
>
>Yes, I'll do that. Thanks for bringing it to my attention.
>
>I assume we can't use avr-libc's delay functions, because they depend
>on the CPU clock frequency, which isn't known when we compile
>libstdc++. So I'll just suppress the declarations of those functions
>and remove the #error.

The attached patch adds a new _GLIBCXX_NO_SLEEP configure macro which
should get defined for your hosted AVR build. That should mean that
std::this_thread::sleep_for is not defined, and src/c++11/thread.cc
will no longer insist on some way to sleep being supported.

I've only tested this on powerpc64le-linux, so please let me know if
it works for you.

Pushed to master.



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

commit 0aa1786d34b891c8e1e219fb11255af5358013c4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Dec 9 16:53:18 2020

    libstdc++: Fix build failure for target with no way to sleep
    
    In previous releases the std::this_thread::sleep_for function was only
    declared if the target supports multiple threads. I changed that
    recently in r11-2649-g5bbb1f3000c57fd4d95969b30fa0e35be6d54ffb so that
    sleep_for could be used single-threaded. But that means that targets
    using --disable-threads are now required to provide some way to sleep.
    This breaks the build for (at least) AVR when trying to build a hosted
    library.
    
    This patch adds a new autoconf macro that is defined when no way to
    sleep is available, and uses that to suppress the sleeping functions in
    std::this_thread.
    
    The #error in src/c++11/thread.cc is retained for the case where there
    is no sleep function available but multiple threads are supported. This
    is consistent with previous releases, but that #error could probably be
    removed without any consequences.
    
    libstdc++-v3/ChangeLog:
    
            * acinclude.m4 (GLIBCXX_ENABLE_LIBSTDCXX_TIME): Define NO_SLEEP
            if none of nanosleep, sleep and Sleep is available.
            * config.h.in: Regenerate.
            * configure: Regenerate.
            * include/std/thread [_GLIBCXX_NO_SLEEP] (__sleep_for): Do
            not declare.
            [_GLIBCXX_NO_SLEEP] (sleep_for, sleep_until): Do not
            define.
            * src/c++11/thread.cc [_GLIBCXX_NO_SLEEP] (__sleep_for): Do
            not define.

diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index fcd9ea3d23a..61191812c92 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -1626,16 +1626,22 @@ AC_DEFUN([GLIBCXX_ENABLE_LIBSTDCXX_TIME], [
   fi
 
   if test x"$ac_has_nanosleep$ac_has_sleep" = x"nono"; then
+      ac_no_sleep=yes
       AC_MSG_CHECKING([for Sleep])
       AC_TRY_COMPILE([#include <windows.h>],
                      [Sleep(1)],
                      [ac_has_win32_sleep=yes],[ac_has_win32_sleep=no])
       if test x"$ac_has_win32_sleep" = x"yes"; then
         AC_DEFINE(HAVE_WIN32_SLEEP,1, [Defined if Sleep exists.])
+	ac_no_sleep=no
       fi
       AC_MSG_RESULT($ac_has_win32_sleep)
   fi
 
+  if test x"$ac_no_sleep" = x"yes"; then
+    AC_DEFINE(NO_SLEEP,1, [Defined if no way to sleep is available.])
+  fi
+
   AC_SUBST(GLIBCXX_LIBS)
 
   CXXFLAGS="$ac_save_CXXFLAGS"
diff --git a/libstdc++-v3/include/std/thread b/libstdc++-v3/include/std/thread
index 6ea8a51c0cf..8d0ede2b6c2 100644
--- a/libstdc++-v3/include/std/thread
+++ b/libstdc++-v3/include/std/thread
@@ -122,8 +122,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
    */
   namespace this_thread
   {
+#ifndef _GLIBCXX_NO_SLEEP
+
+#ifndef _GLIBCXX_USE_NANOSLEEP
     void
     __sleep_for(chrono::seconds, chrono::nanoseconds);
+#endif
 
     /// this_thread::sleep_for
     template<typename _Rep, typename _Period>
@@ -168,7 +172,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	    __now = _Clock::now();
 	  }
       }
-  }
+  } // namespace this_thread
+#endif // ! NO_SLEEP
 
 #ifdef __cpp_lib_jthread
 
diff --git a/libstdc++-v3/src/c++11/thread.cc b/libstdc++-v3/src/c++11/thread.cc
index a9c92804959..62f6ddcd802 100644
--- a/libstdc++-v3/src/c++11/thread.cc
+++ b/libstdc++-v3/src/c++11/thread.cc
@@ -35,7 +35,8 @@
 #  include <unistd.h>
 # elif defined(_GLIBCXX_HAVE_WIN32_SLEEP)
 #  include <windows.h>
-# else
+# elif defined _GLIBCXX_NO_SLEEP && defined _GLIBCXX_HAS_GTHREADS
+// We expect to be able to sleep for targets that support multiple threads:
 #  error "No sleep function known for this target"
 # endif
 #endif
@@ -196,6 +197,7 @@ _GLIBCXX_END_NAMESPACE_VERSION
 
 #endif // _GLIBCXX_HAS_GTHREADS
 
+#ifndef _GLIBCXX_NO_SLEEP
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
@@ -252,3 +254,4 @@ namespace this_thread
 }
 _GLIBCXX_END_NAMESPACE_VERSION
 } // namespace std
+#endif // ! NO_SLEEP

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the quick response.
The patch solves the problem.

ср, 9 дек. 2020 г. в 18:01, Jonathan Wakely <jwakely@redhat.com>:

> On 09/12/20 12:49 +0000, Jonathan Wakely wrote:
> >On 09/12/20 13:32 +0100, Vladimir V wrote:
> >>Hello.
> >>
> >>While testing with the current upstream I encountered a compilation
> issue.
> >>Although I build with "--disable-threads" flag the following error
> occurs:
> >>
> >>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
> >>sleep function known for this target"
> >>
> >>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
> >>prevented the error from happening (in my case with gcc v10.1),
> >>So I would like to ask if the thread.cc should be involved in the build
> if
> >>the threads support is configured to be disabled?
> >
> >Yes, the file is always built, but which definitions it contains
> >depends on what is configured for the target.
> >
> >The std::this_thread::sleep_for and std::this_thread::sleep_until
> >functions don't actually depend on threads at all. They just sleep.
> >
> >But that still requires target support, just different support from
> >threads.
> >
> >>And if it should, then can the condition be reworked to cover the
> described
> >>case?
> >
> >Yes, I'll do that. Thanks for bringing it to my attention.
> >
> >I assume we can't use avr-libc's delay functions, because they depend
> >on the CPU clock frequency, which isn't known when we compile
> >libstdc++. So I'll just suppress the declarations of those functions
> >and remove the #error.
>
> The attached patch adds a new _GLIBCXX_NO_SLEEP configure macro which
> should get defined for your hosted AVR build. That should mean that
> std::this_thread::sleep_for is not defined, and src/c++11/thread.cc
> will no longer insist on some way to sleep being supported.
>
> I've only tested this on powerpc64le-linux, so please let me know if
> it works for you.
>
> Pushed to master.
>
>
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

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

Hello.

Could you please have a look at my trivial patch.
It works as intended with avr-libc and doesn't seem to introduce
regressions for x86_64 hosts.
What would be your suggestions for testing?

Thank you
Vladimir

чт, 10 дек. 2020 г. в 00:00, Vladimir V <vv.os.swe@gmail.com>:

> Thank you for the quick response.
> The patch solves the problem.
>
> ср, 9 дек. 2020 г. в 18:01, Jonathan Wakely <jwakely@redhat.com>:
>
>> On 09/12/20 12:49 +0000, Jonathan Wakely wrote:
>> >On 09/12/20 13:32 +0100, Vladimir V wrote:
>> >>Hello.
>> >>
>> >>While testing with the current upstream I encountered a compilation
>> issue.
>> >>Although I build with "--disable-threads" flag the following error
>> occurs:
>> >>
>> >>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
>> >>sleep function known for this target"
>> >>
>> >>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
>> >>prevented the error from happening (in my case with gcc v10.1),
>> >>So I would like to ask if the thread.cc should be involved in the build
>> if
>> >>the threads support is configured to be disabled?
>> >
>> >Yes, the file is always built, but which definitions it contains
>> >depends on what is configured for the target.
>> >
>> >The std::this_thread::sleep_for and std::this_thread::sleep_until
>> >functions don't actually depend on threads at all. They just sleep.
>> >
>> >But that still requires target support, just different support from
>> >threads.
>> >
>> >>And if it should, then can the condition be reworked to cover the
>> described
>> >>case?
>> >
>> >Yes, I'll do that. Thanks for bringing it to my attention.
>> >
>> >I assume we can't use avr-libc's delay functions, because they depend
>> >on the CPU clock frequency, which isn't known when we compile
>> >libstdc++. So I'll just suppress the declarations of those functions
>> >and remove the #error.
>>
>> The attached patch adds a new _GLIBCXX_NO_SLEEP configure macro which
>> should get defined for your hosted AVR build. That should mean that
>> std::this_thread::sleep_for is not defined, and src/c++11/thread.cc
>> will no longer insist on some way to sleep being supported.
>>
>> I've only tested this on powerpc64le-linux, so please let me know if
>> it works for you.
>>
>> Pushed to master.
>>
>>
>>

[-- Attachment #2: Disabling-AC_LIBTOOL_DLOPEN-check-for-avr.txt --]
[-- Type: text/plain, Size: 1127 bytes --]

From fbb2144b56625adf594f8812189b983fa66c910a Mon Sep 17 00:00:00 2001
From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
Date: Tue, 8 Dec 2020 21:45:26 +0100
Subject: [PATCH] Disabling AC_LIBTOOL_DLOPEN check if building with avr-libc

The AC_LIBTOOL_DLOPEN checks were previously disabled for newlib targets.
The patch applies similar logic to avr-libc based builds.

2020-12-08 Vladimir Vishnevsky <vv.os.swe@gmail.com>

libstdc++-v3/ChangeLog:
        Disabling AC_LIBTOOL_DLOPEN check if building with avr-libc.
        * configure.ac: Skip AC_LIBTOOL_DLOPEN check if avr-libc is used.
        * configure: Regenerate.
---
 libstdc++-v3/configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
index cbfdf4c6bad..771814110a1 100644
--- a/libstdc++-v3/configure.ac
+++ b/libstdc++-v3/configure.ac
@@ -90,7 +90,7 @@ AC_SYS_LARGEFILE
 GLIBCXX_CONFIGURE
 
 # Libtool setup.
-if test "x${with_newlib}" != "xyes"; then
+if test "x${with_newlib}" != "xyes" && test "x${with_avrlibc}" != "xyes"; then
   AC_LIBTOOL_DLOPEN
 fi
 AM_PROG_LIBTOOL
-- 
2.17.1

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 10/12/20 18:39 +0100, Vladimir V via Libstdc++ wrote:
>Hello.
>
>Could you please have a look at my trivial patch.
>It works as intended with avr-libc and doesn't seem to introduce
>regressions for x86_64 hosts.
>What would be your suggestions for testing?

If you've tested it on AVR then I think that's good enough, as I've
already been testing an identical patch locally :-)

I'll get this committed after the weekend, thanks for the patch!

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you!

чт, 10 дек. 2020 г. в 21:15, Jonathan Wakely <jwakely@redhat.com>:

> On 10/12/20 18:39 +0100, Vladimir V via Libstdc++ wrote:
> >Hello.
> >
> >Could you please have a look at my trivial patch.
> >It works as intended with avr-libc and doesn't seem to introduce
> >regressions for x86_64 hosts.
> >What would be your suggestions for testing?
>
> If you've tested it on AVR then I think that's good enough, as I've
> already been testing an identical patch locally :-)
>
> I'll get this committed after the weekend, thanks for the patch!
>
>
>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 10/12/20 18:39 +0100, Vladimir V via Libstdc++ wrote:
>Hello.
>
>Could you please have a look at my trivial patch.
>It works as intended with avr-libc and doesn't seem to introduce
>regressions for x86_64 hosts.

I've pushed this to master now, thanks for the patch.

>What would be your suggestions for testing?
>
>Thank you
>Vladimir
>
>чт, 10 дек. 2020 г. в 00:00, Vladimir V <vv.os.swe@gmail.com>:
>
>> Thank you for the quick response.
>> The patch solves the problem.
>>
>> ср, 9 дек. 2020 г. в 18:01, Jonathan Wakely <jwakely@redhat.com>:
>>
>>> On 09/12/20 12:49 +0000, Jonathan Wakely wrote:
>>> >On 09/12/20 13:32 +0100, Vladimir V wrote:
>>> >>Hello.
>>> >>
>>> >>While testing with the current upstream I encountered a compilation
>>> issue.
>>> >>Although I build with "--disable-threads" flag the following error
>>> occurs:
>>> >>
>>> >>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error "No
>>> >>sleep function known for this target"
>>> >>
>>> >>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS that
>>> >>prevented the error from happening (in my case with gcc v10.1),
>>> >>So I would like to ask if the thread.cc should be involved in the build
>>> if
>>> >>the threads support is configured to be disabled?
>>> >
>>> >Yes, the file is always built, but which definitions it contains
>>> >depends on what is configured for the target.
>>> >
>>> >The std::this_thread::sleep_for and std::this_thread::sleep_until
>>> >functions don't actually depend on threads at all. They just sleep.
>>> >
>>> >But that still requires target support, just different support from
>>> >threads.
>>> >
>>> >>And if it should, then can the condition be reworked to cover the
>>> described
>>> >>case?
>>> >
>>> >Yes, I'll do that. Thanks for bringing it to my attention.
>>> >
>>> >I assume we can't use avr-libc's delay functions, because they depend
>>> >on the CPU clock frequency, which isn't known when we compile
>>> >libstdc++. So I'll just suppress the declarations of those functions
>>> >and remove the #error.
>>>
>>> The attached patch adds a new _GLIBCXX_NO_SLEEP configure macro which
>>> should get defined for your hosted AVR build. That should mean that
>>> std::this_thread::sleep_for is not defined, and src/c++11/thread.cc
>>> will no longer insist on some way to sleep being supported.
>>>
>>> I've only tested this on powerpc64le-linux, so please let me know if
>>> it works for you.
>>>
>>> Pushed to master.
>>>
>>>
>>>

From fbb2144b56625adf594f8812189b983fa66c910a Mon Sep 17 00:00:00 2001
>From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
>Date: Tue, 8 Dec 2020 21:45:26 +0100
>Subject: [PATCH] Disabling AC_LIBTOOL_DLOPEN check if building with avr-libc
>
>The AC_LIBTOOL_DLOPEN checks were previously disabled for newlib targets.
>The patch applies similar logic to avr-libc based builds.
>
>2020-12-08 Vladimir Vishnevsky <vv.os.swe@gmail.com>
>
>libstdc++-v3/ChangeLog:
>        Disabling AC_LIBTOOL_DLOPEN check if building with avr-libc.
>        * configure.ac: Skip AC_LIBTOOL_DLOPEN check if avr-libc is used.
>        * configure: Regenerate.
>---
> libstdc++-v3/configure.ac | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
>index cbfdf4c6bad..771814110a1 100644
>--- a/libstdc++-v3/configure.ac
>+++ b/libstdc++-v3/configure.ac
>@@ -90,7 +90,7 @@ AC_SYS_LARGEFILE
> GLIBCXX_CONFIGURE
>
> # Libtool setup.
>-if test "x${with_newlib}" != "xyes"; then
>+if test "x${with_newlib}" != "xyes" && test "x${with_avrlibc}" != "xyes"; then
>   AC_LIBTOOL_DLOPEN
> fi
> AM_PROG_LIBTOOL
>-- 
>2.17.1
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you very much.

вт, 15 дек. 2020 г. в 12:48, Jonathan Wakely <jwakely@redhat.com>:

> On 10/12/20 18:39 +0100, Vladimir V via Libstdc++ wrote:
> >Hello.
> >
> >Could you please have a look at my trivial patch.
> >It works as intended with avr-libc and doesn't seem to introduce
> >regressions for x86_64 hosts.
>
> I've pushed this to master now, thanks for the patch.
>
> >What would be your suggestions for testing?
> >
> >Thank you
> >Vladimir
> >
> >чт, 10 дек. 2020 г. в 00:00, Vladimir V <vv.os.swe@gmail.com>:
> >
> >> Thank you for the quick response.
> >> The patch solves the problem.
> >>
> >> ср, 9 дек. 2020 г. в 18:01, Jonathan Wakely <jwakely@redhat.com>:
> >>
> >>> On 09/12/20 12:49 +0000, Jonathan Wakely wrote:
> >>> >On 09/12/20 13:32 +0100, Vladimir V wrote:
> >>> >>Hello.
> >>> >>
> >>> >>While testing with the current upstream I encountered a compilation
> >>> issue.
> >>> >>Although I build with "--disable-threads" flag the following error
> >>> occurs:
> >>> >>
> >>> >>../../../../../libstdc++-v3/src/c++11/thread.cc:39:4: error: #error
> "No
> >>> >>sleep function known for this target"
> >>> >>
> >>> >>Previously the check was inside the  #ifdef _GLIBCXX_HAS_GTHREADS
> that
> >>> >>prevented the error from happening (in my case with gcc v10.1),
> >>> >>So I would like to ask if the thread.cc should be involved in the
> build
> >>> if
> >>> >>the threads support is configured to be disabled?
> >>> >
> >>> >Yes, the file is always built, but which definitions it contains
> >>> >depends on what is configured for the target.
> >>> >
> >>> >The std::this_thread::sleep_for and std::this_thread::sleep_until
> >>> >functions don't actually depend on threads at all. They just sleep.
> >>> >
> >>> >But that still requires target support, just different support from
> >>> >threads.
> >>> >
> >>> >>And if it should, then can the condition be reworked to cover the
> >>> described
> >>> >>case?
> >>> >
> >>> >Yes, I'll do that. Thanks for bringing it to my attention.
> >>> >
> >>> >I assume we can't use avr-libc's delay functions, because they depend
> >>> >on the CPU clock frequency, which isn't known when we compile
> >>> >libstdc++. So I'll just suppress the declarations of those functions
> >>> >and remove the #error.
> >>>
> >>> The attached patch adds a new _GLIBCXX_NO_SLEEP configure macro which
> >>> should get defined for your hosted AVR build. That should mean that
> >>> std::this_thread::sleep_for is not defined, and src/c++11/thread.cc
> >>> will no longer insist on some way to sleep being supported.
> >>>
> >>> I've only tested this on powerpc64le-linux, so please let me know if
> >>> it works for you.
> >>>
> >>> Pushed to master.
> >>>
> >>>
> >>>
>
> >From fbb2144b56625adf594f8812189b983fa66c910a Mon Sep 17 00:00:00 2001
> >From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
> >Date: Tue, 8 Dec 2020 21:45:26 +0100
> >Subject: [PATCH] Disabling AC_LIBTOOL_DLOPEN check if building with
> avr-libc
> >
> >The AC_LIBTOOL_DLOPEN checks were previously disabled for newlib targets.
> >The patch applies similar logic to avr-libc based builds.
> >
> >2020-12-08 Vladimir Vishnevsky <vv.os.swe@gmail.com>
> >
> >libstdc++-v3/ChangeLog:
> >        Disabling AC_LIBTOOL_DLOPEN check if building with avr-libc.
> >        * configure.ac: Skip AC_LIBTOOL_DLOPEN check if avr-libc is used.
> >        * configure: Regenerate.
> >---
> > libstdc++-v3/configure.ac | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac
> >index cbfdf4c6bad..771814110a1 100644
> >--- a/libstdc++-v3/configure.ac
> >+++ b/libstdc++-v3/configure.ac
> >@@ -90,7 +90,7 @@ AC_SYS_LARGEFILE
> > GLIBCXX_CONFIGURE
> >
> > # Libtool setup.
> >-if test "x${with_newlib}" != "xyes"; then
> >+if test "x${with_newlib}" != "xyes" && test "x${with_avrlibc}" !=
> "xyes"; then
> >   AC_LIBTOOL_DLOPEN
> > fi
> > AM_PROG_LIBTOOL
> >--
> >2.17.1
> >
>
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Hello and Happy New Year!

Getting back to the discussion we had about scalable libstdc++.
For sure it will be very helpful for embedded targets. There are huge
components with
well defined boundaries that are unlikely to be often used in minimalist
settings but require missing platform support.
One particular example I'm looking at right now is 'filesystem'. Although
it resorts to some
default stubs if corresponding API is not present, I think it would be
correct to remove it from the library
completely if it can not/should not be used on the target.
In case of avr-libc it is worse as the unistd is partially present but not
sufficient to satisfy all filesystem
dependencies.

It looks like that after the filesystem support was implemented for c++17
standard there is no option to remove
it from the build (unlike for the filesystem-ts). Please correct me if I am
wrong but If it is like that I would like to make this configurable.
Would it be acceptable?

Thank you.

пн, 7 дек. 2020 г. в 22:07, Jonathan Wakely <jwakely@redhat.com>:

> On 07/12/20 12:41 -0800, Keith Packard via Libstdc++ wrote:
> >Jonathan Wakely via Libstdc++ <libstdc++@gcc.gnu.org> writes:
> >
> >>>and was resolved for the newlib. The fix was not applied for the
> avr-libc
> >>>targets although the rationale behind the solution
> >>>seems to be applicable there as well. I would like to ask what do you
> think
> >>>about extending the scope of the solution to work
> >>>with avr-libc based builds as well?
> >>
> >> Yes, I think it makes sense.
> >
> >I've been working on making libstdc++ work with picolibc, which shares a
> >bunch of stdio code with avr-libc. I haven't tried my patches with
> >avr-libc, but I wonder if that would be a good thing to do? With those
> >patches, I'm able to use libstdc++'s iostream.
>
> That might be interesting to try.
>
> I'm very much in favour of changes to libstdc++ that allow it to be
> used on small systems. While some C++ features may not be suitable for
> those systems, I think there's a lot of the standard library that is
> still useful, but isn't part of the current "freestanding" subset. And
> I'd like libstdc++ to be able to scale from very small to very large,
> rather than needing to fork it to meet those needs.
>
>
>
>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 04/01/21 12:28 +0100, Vladimir V wrote:
>Hello and Happy New Year!
>
>Getting back to the discussion we had about scalable libstdc++.
>For sure it will be very helpful for embedded targets. There are huge
>components with
>well defined boundaries that are unlikely to be often used in minimalist
>settings but require missing platform support.
>One particular example I'm looking at right now is 'filesystem'. Although
>it resorts to some
>default stubs if corresponding API is not present, I think it would be
>correct to remove it from the library
>completely if it can not/should not be used on the target.
>In case of avr-libc it is worse as the unistd is partially present but not
>sufficient to satisfy all filesystem
>dependencies.
>
>It looks like that after the filesystem support was implemented for c++17
>standard there is no option to remove
>it from the build (unlike for the filesystem-ts). Please correct me if I am
>wrong but If it is like that I would like to make this configurable.

You're right.

My thinking was that the C++ standard requires the <filesystem> header
to be present for a hosted implementation. It doesn't necessarily
require the contents to *do* much, so it's conforming for them to
exist but always report errors.

>Would it be acceptable?

I'm unsure.

Currently we support "hosted" and "freestanding", but not really
anything more fine-grained. The point of the configure switch
--disable-libstdcxx-filesystem-ts was not really to allow people to
build without it, but more to enable it for targets where it wasn't on
by default because it hadn't been tested yet.

I've recently been trying to move away from having missing features,
and make things available unconditionally, but to use useless stubs or
return errors for targets that can't support it. For example, GCC 11
always defines the std::thread type, even if the target can't actually
use it to create new threads. The idea is to always provide a complete
implementation, not have the feature set vary by target (I see a lot
of confusion on places like StackOverflow, where the answer is "yeah
sorry, that header is empty on your OS").

What you're suggesting would lead to a more modular libstdc++ where
you can enable/disable components, similar to what newlib does. If we
did that for more than just the std::filesystem library things would
get complicated (it increases the maintenance and testing burden, and
we need to be careful not to create dependencies on anything from a
conditionally-present feature).

Would your suggestion actually result in smaller/faster binaries for
AVR? Or just remove features that aren't fully functional if you
attempt to use them? If having those features in the library doesn't
actually cause bigger/slower binaries, then I don't really see a
problem with them being present.

Tangentially, the direction the C++ standard seems to be going is to
grow the feature set of freestanding. That would allow more things to
be used in freestanding environments, rather than the very limited set
of features currently guaranteed to be available. That doesn't really
help you though, as you want a hosted env with things like basic I/O,
but not the full set of features usually present for hosted.

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the detailed clarification.

My primary idea was to reduce the build dependencies between hosted
libstdc++
and target libc if some components can not be supported or never will be
used.
Also my general vision is that if something doesn't work it shouldn't be
available
in the environment. But I perfectly understand the rationale you explained
and requirements applied by standard.

So I think not much can be done here at this point and further work should
be on the avr-libc side.

Thank you.

пт, 8 янв. 2021 г. в 19:21, Jonathan Wakely <jwakely@redhat.com>:

> On 04/01/21 12:28 +0100, Vladimir V wrote:
> >Hello and Happy New Year!
> >
> >Getting back to the discussion we had about scalable libstdc++.
> >For sure it will be very helpful for embedded targets. There are huge
> >components with
> >well defined boundaries that are unlikely to be often used in minimalist
> >settings but require missing platform support.
> >One particular example I'm looking at right now is 'filesystem'. Although
> >it resorts to some
> >default stubs if corresponding API is not present, I think it would be
> >correct to remove it from the library
> >completely if it can not/should not be used on the target.
> >In case of avr-libc it is worse as the unistd is partially present but not
> >sufficient to satisfy all filesystem
> >dependencies.
> >
> >It looks like that after the filesystem support was implemented for c++17
> >standard there is no option to remove
> >it from the build (unlike for the filesystem-ts). Please correct me if I
> am
> >wrong but If it is like that I would like to make this configurable.
>
> You're right.
>
> My thinking was that the C++ standard requires the <filesystem> header
> to be present for a hosted implementation. It doesn't necessarily
> require the contents to *do* much, so it's conforming for them to
> exist but always report errors.
>
> >Would it be acceptable?
>
> I'm unsure.
>
> Currently we support "hosted" and "freestanding", but not really
> anything more fine-grained. The point of the configure switch
> --disable-libstdcxx-filesystem-ts was not really to allow people to
> build without it, but more to enable it for targets where it wasn't on
> by default because it hadn't been tested yet.
>
> I've recently been trying to move away from having missing features,
> and make things available unconditionally, but to use useless stubs or
> return errors for targets that can't support it. For example, GCC 11
> always defines the std::thread type, even if the target can't actually
> use it to create new threads. The idea is to always provide a complete
> implementation, not have the feature set vary by target (I see a lot
> of confusion on places like StackOverflow, where the answer is "yeah
> sorry, that header is empty on your OS").
>
> What you're suggesting would lead to a more modular libstdc++ where
> you can enable/disable components, similar to what newlib does. If we
> did that for more than just the std::filesystem library things would
> get complicated (it increases the maintenance and testing burden, and
> we need to be careful not to create dependencies on anything from a
> conditionally-present feature).
>
> Would your suggestion actually result in smaller/faster binaries for
> AVR? Or just remove features that aren't fully functional if you
> attempt to use them? If having those features in the library doesn't
> actually cause bigger/slower binaries, then I don't really see a
> problem with them being present.
>
> Tangentially, the direction the C++ standard seems to be going is to
> grow the feature set of freestanding. That would allow more things to
> be used in freestanding environments, rather than the very limited set
> of features currently guaranteed to be available. That doesn't really
> help you though, as you want a hosted env with things like basic I/O,
> but not the full set of features usually present for hosted.
>
>
>
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

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

Hello,

I finally managed to test building for AVR with Keith's patch and
unistd.h removed (to enforce usage of stabs in filesystem).

I identified a couple of minor build issues that are obvious from the
attached patch, Could you please have a look? They work with AVR target
and doesn't seem to cause regressions for x64 build.

Thank you.

пт, 22 янв. 2021 г. в 15:46, Vladimir V <vv.os.swe@gmail.com>:

> Thank you for the detailed clarification.
>
> My primary idea was to reduce the build dependencies between hosted
> libstdc++
> and target libc if some components can not be supported or never will be
> used.
> Also my general vision is that if something doesn't work it shouldn't be
> available
> in the environment. But I perfectly understand the rationale you explained
> and requirements applied by standard.
>
> So I think not much can be done here at this point and further work should
> be on the avr-libc side.
>
> Thank you.
>
> пт, 8 янв. 2021 г. в 19:21, Jonathan Wakely <jwakely@redhat.com>:
>
>> On 04/01/21 12:28 +0100, Vladimir V wrote:
>> >Hello and Happy New Year!
>> >
>> >Getting back to the discussion we had about scalable libstdc++.
>> >For sure it will be very helpful for embedded targets. There are huge
>> >components with
>> >well defined boundaries that are unlikely to be often used in minimalist
>> >settings but require missing platform support.
>> >One particular example I'm looking at right now is 'filesystem'. Although
>> >it resorts to some
>> >default stubs if corresponding API is not present, I think it would be
>> >correct to remove it from the library
>> >completely if it can not/should not be used on the target.
>> >In case of avr-libc it is worse as the unistd is partially present but
>> not
>> >sufficient to satisfy all filesystem
>> >dependencies.
>> >
>> >It looks like that after the filesystem support was implemented for c++17
>> >standard there is no option to remove
>> >it from the build (unlike for the filesystem-ts). Please correct me if I
>> am
>> >wrong but If it is like that I would like to make this configurable.
>>
>> You're right.
>>
>> My thinking was that the C++ standard requires the <filesystem> header
>> to be present for a hosted implementation. It doesn't necessarily
>> require the contents to *do* much, so it's conforming for them to
>> exist but always report errors.
>>
>> >Would it be acceptable?
>>
>> I'm unsure.
>>
>> Currently we support "hosted" and "freestanding", but not really
>> anything more fine-grained. The point of the configure switch
>> --disable-libstdcxx-filesystem-ts was not really to allow people to
>> build without it, but more to enable it for targets where it wasn't on
>> by default because it hadn't been tested yet.
>>
>> I've recently been trying to move away from having missing features,
>> and make things available unconditionally, but to use useless stubs or
>> return errors for targets that can't support it. For example, GCC 11
>> always defines the std::thread type, even if the target can't actually
>> use it to create new threads. The idea is to always provide a complete
>> implementation, not have the feature set vary by target (I see a lot
>> of confusion on places like StackOverflow, where the answer is "yeah
>> sorry, that header is empty on your OS").
>>
>> What you're suggesting would lead to a more modular libstdc++ where
>> you can enable/disable components, similar to what newlib does. If we
>> did that for more than just the std::filesystem library things would
>> get complicated (it increases the maintenance and testing burden, and
>> we need to be careful not to create dependencies on anything from a
>> conditionally-present feature).
>>
>> Would your suggestion actually result in smaller/faster binaries for
>> AVR? Or just remove features that aren't fully functional if you
>> attempt to use them? If having those features in the library doesn't
>> actually cause bigger/slower binaries, then I don't really see a
>> problem with them being present.
>>
>> Tangentially, the direction the C++ standard seems to be going is to
>> grow the feature set of freestanding. That would allow more things to
>> be used in freestanding environments, rather than the very limited set
>> of features currently guaranteed to be available. That doesn't really
>> help you though, as you want a hosted env with things like basic I/O,
>> but not the full set of features usually present for hosted.
>>
>>
>>
>>

[-- Attachment #2: libstdc-Fix-build-failure-for-targets-without-unistd.txt --]
[-- Type: text/plain, Size: 1847 bytes --]

From ac970e6a376def43c510f9091d342a014f23fe8f Mon Sep 17 00:00:00 2001
From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
Date: Sat, 6 Feb 2021 12:25:24 +0100
Subject: [PATCH] libstdc++: Fix build failure for targets without unistd.h

The patch fixes build issues occuring if build parameter
"--enable-cstdio=stdio_pure" is specified and no unistd.h is
present in the environment.

2021-02-07 Vladimir Vishnevsky <vv.os.swe@gmail.com>

libstdc++-v3/ChangeLog:
        Fix build failure for targets without unistd.h.
        * include/ext/stdio_sync_filebuf.h: Unused <unistd.h> include removed.
        * src/c++17/fs_ops.cc: Proper namespace specified for locally defined
        mode_t.
---
 libstdc++-v3/include/ext/stdio_sync_filebuf.h | 1 -
 libstdc++-v3/src/c++17/fs_ops.cc              | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/ext/stdio_sync_filebuf.h b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
index 178b471957a..90765e55831 100644
--- a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
+++ b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
@@ -32,7 +32,6 @@
 #pragma GCC system_header
 
 #include <streambuf>
-#include <unistd.h>
 #include <cstdio>
 #include <bits/c++io.h>  // For __c_file
 #include <bits/move.h>   // For __exchange
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 72755c98a5a..04a559ab1d6 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
 #else
   if (nofollow && is_symlink(st))
     ec = std::make_error_code(std::errc::not_supported);
-  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
+  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
     err = errno;
 #endif
 
-- 
2.17.1

Re: Problem building libstdc++ for the avr target

[Vladimir V]

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

Slightly corrected comment in the patch.

вс, 7 февр. 2021 г. в 14:55, Vladimir V <vv.os.swe@gmail.com>:

> Hello,
>
> I finally managed to test building for AVR with Keith's patch and
> unistd.h removed (to enforce usage of stabs in filesystem).
>
> I identified a couple of minor build issues that are obvious from the
> attached patch, Could you please have a look? They work with AVR target
> and doesn't seem to cause regressions for x64 build.
>
> Thank you.
>
> пт, 22 янв. 2021 г. в 15:46, Vladimir V <vv.os.swe@gmail.com>:
>
>> Thank you for the detailed clarification.
>>
>> My primary idea was to reduce the build dependencies between hosted
>> libstdc++
>> and target libc if some components can not be supported or never will be
>> used.
>> Also my general vision is that if something doesn't work it shouldn't be
>> available
>> in the environment. But I perfectly understand the rationale you explained
>> and requirements applied by standard.
>>
>> So I think not much can be done here at this point and further work
>> should be on the avr-libc side.
>>
>> Thank you.
>>
>> пт, 8 янв. 2021 г. в 19:21, Jonathan Wakely <jwakely@redhat.com>:
>>
>>> On 04/01/21 12:28 +0100, Vladimir V wrote:
>>> >Hello and Happy New Year!
>>> >
>>> >Getting back to the discussion we had about scalable libstdc++.
>>> >For sure it will be very helpful for embedded targets. There are huge
>>> >components with
>>> >well defined boundaries that are unlikely to be often used in minimalist
>>> >settings but require missing platform support.
>>> >One particular example I'm looking at right now is 'filesystem'.
>>> Although
>>> >it resorts to some
>>> >default stubs if corresponding API is not present, I think it would be
>>> >correct to remove it from the library
>>> >completely if it can not/should not be used on the target.
>>> >In case of avr-libc it is worse as the unistd is partially present but
>>> not
>>> >sufficient to satisfy all filesystem
>>> >dependencies.
>>> >
>>> >It looks like that after the filesystem support was implemented for
>>> c++17
>>> >standard there is no option to remove
>>> >it from the build (unlike for the filesystem-ts). Please correct me if
>>> I am
>>> >wrong but If it is like that I would like to make this configurable.
>>>
>>> You're right.
>>>
>>> My thinking was that the C++ standard requires the <filesystem> header
>>> to be present for a hosted implementation. It doesn't necessarily
>>> require the contents to *do* much, so it's conforming for them to
>>> exist but always report errors.
>>>
>>> >Would it be acceptable?
>>>
>>> I'm unsure.
>>>
>>> Currently we support "hosted" and "freestanding", but not really
>>> anything more fine-grained. The point of the configure switch
>>> --disable-libstdcxx-filesystem-ts was not really to allow people to
>>> build without it, but more to enable it for targets where it wasn't on
>>> by default because it hadn't been tested yet.
>>>
>>> I've recently been trying to move away from having missing features,
>>> and make things available unconditionally, but to use useless stubs or
>>> return errors for targets that can't support it. For example, GCC 11
>>> always defines the std::thread type, even if the target can't actually
>>> use it to create new threads. The idea is to always provide a complete
>>> implementation, not have the feature set vary by target (I see a lot
>>> of confusion on places like StackOverflow, where the answer is "yeah
>>> sorry, that header is empty on your OS").
>>>
>>> What you're suggesting would lead to a more modular libstdc++ where
>>> you can enable/disable components, similar to what newlib does. If we
>>> did that for more than just the std::filesystem library things would
>>> get complicated (it increases the maintenance and testing burden, and
>>> we need to be careful not to create dependencies on anything from a
>>> conditionally-present feature).
>>>
>>> Would your suggestion actually result in smaller/faster binaries for
>>> AVR? Or just remove features that aren't fully functional if you
>>> attempt to use them? If having those features in the library doesn't
>>> actually cause bigger/slower binaries, then I don't really see a
>>> problem with them being present.
>>>
>>> Tangentially, the direction the C++ standard seems to be going is to
>>> grow the feature set of freestanding. That would allow more things to
>>> be used in freestanding environments, rather than the very limited set
>>> of features currently guaranteed to be available. That doesn't really
>>> help you though, as you want a hosted env with things like basic I/O,
>>> but not the full set of features usually present for hosted.
>>>
>>>
>>>
>>>

[-- Attachment #2: libstdc-Fix-build-failure-for-targets-without-unistd.txt --]
[-- Type: text/plain, Size: 1816 bytes --]

From 6c158aa448e084112b768fd9bc3306b64102d537 Mon Sep 17 00:00:00 2001
From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
Date: Sat, 6 Feb 2021 12:25:24 +0100
Subject: [PATCH] libstdc++: Fix build failure for targets without unistd.h

The patch fixes build issues occuring if build parameter
"--enable-cstdio=stdio_pure" is specified and no unistd.h is
present in the environment.

2021-02-07 Vladimir Vishnevsky <vv.os.swe@gmail.com>

libstdc++-v3/ChangeLog:
        Fix build failure for targets without unistd.h.
        * include/ext/stdio_sync_filebuf.h: Unused <unistd.h> include removed.
        * src/c++17/fs_ops.cc: Namespace specified for mode_t.
---
 libstdc++-v3/include/ext/stdio_sync_filebuf.h | 1 -
 libstdc++-v3/src/c++17/fs_ops.cc              | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/ext/stdio_sync_filebuf.h b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
index 178b471957a..90765e55831 100644
--- a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
+++ b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
@@ -32,7 +32,6 @@
 #pragma GCC system_header
 
 #include <streambuf>
-#include <unistd.h>
 #include <cstdio>
 #include <bits/c++io.h>  // For __c_file
 #include <bits/move.h>   // For __exchange
diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 72755c98a5a..04a559ab1d6 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
 #else
   if (nofollow && is_symlink(st))
     ec = std::make_error_code(std::errc::not_supported);
-  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
+  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
     err = errno;
 #endif
 
-- 
2.17.1

Re: Problem building libstdc++ for the avr target

[Keith Packard]

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

Vladimir V <vv.os.swe@gmail.com> writes:

> Hello,
>
> I finally managed to test building for AVR with Keith's patch and
> unistd.h removed (to enforce usage of stabs in filesystem).
>
> I identified a couple of minor build issues that are obvious from the
> attached patch, Could you please have a look? They work with AVR target
> and doesn't seem to cause regressions for x64 build.

Hrm. Removing unistd.h from an installed file might affect applications
using that file, so that doesn't seem ideal.

Picolibc has a unistd.h to define the relevant interfaces, even though
it doesn't require them. Perhaps avrlibc could do the same? Or even an
empty file that would satisfy this requirement?

> --- a/libstdc++-v3/src/c++17/fs_ops.cc
> +++ b/libstdc++-v3/src/c++17/fs_ops.cc
> @@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
>  #else
>    if (nofollow && is_symlink(st))
>      ec = std::make_error_code(std::errc::not_supported);
> -  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
> +  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
>      err = errno;
>  #endif

I'm afraid I don't understand what this piece does without studying the
underlying code a bunch. But, if it's just namespace stuff in an
internal implementation file, that seems fine to me.

-- 
-keith

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

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the feedback.

>> Picolibc has a unistd.h to define the relevant interfaces, even though
>> it doesn't require them. Perhaps avrlibc could do the same? Or even an
>> empty file that would satisfy this requirement?

The avr-libc has unistd.h with some declarations required by streams and it
should be further extended to support <filesystem> requirements.

My findings are the result of the experiment where I wanted to identify
dependencies on
<unistd.h> for AVR build. As for non-posix targets unistd.h might be
absent, I decided
to bring it to attention.

>> Hrm. Removing unistd.h from an installed file might affect applications
>>using that file, so that doesn't seem ideal.

Thanks for pointing that out. It indeed looks like potentially breaking
change.
Although the declarations from <unistd.h> does not seem to be used in the
file itself, some client code could rely on implicit include.
But If the GNU C++ standard library extensions where stdio_sync_filebuf
belongs
supports non-posix targets then maybe <unistd.h> include should be
moved inside  _GLIBCXX_HAVE_UNISTD_H guard?
This should provide backward compatibility.

>> I'm afraid I don't understand what this piece does without studying the
>> underlying code a bunch. But, if it's just namespace stuff in an
>> internal implementation file, that seems fine to me.

It is really just the namespace missing in the conditionally-compiled
part.

Thank you.

вс, 7 февр. 2021 г. в 19:23, Keith Packard <keithp@keithp.com>:

> Vladimir V <vv.os.swe@gmail.com> writes:
>
> > Hello,
> >
> > I finally managed to test building for AVR with Keith's patch and
> > unistd.h removed (to enforce usage of stabs in filesystem).
> >
> > I identified a couple of minor build issues that are obvious from the
> > attached patch, Could you please have a look? They work with AVR target
> > and doesn't seem to cause regressions for x64 build.
>
> Hrm. Removing unistd.h from an installed file might affect applications
> using that file, so that doesn't seem ideal.
>
> Picolibc has a unistd.h to define the relevant interfaces, even though
> it doesn't require them. Perhaps avrlibc could do the same? Or even an
> empty file that would satisfy this requirement?
>
> > --- a/libstdc++-v3/src/c++17/fs_ops.cc
> > +++ b/libstdc++-v3/src/c++17/fs_ops.cc
> > @@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms,
> perm_options opts,
> >  #else
> >    if (nofollow && is_symlink(st))
> >      ec = std::make_error_code(std::errc::not_supported);
> > -  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
> > +  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
> >      err = errno;
> >  #endif
>
> I'm afraid I don't understand what this piece does without studying the
> underlying code a bunch. But, if it's just namespace stuff in an
> internal implementation file, that seems fine to me.
>
> --
> -keith
>

Re: Problem building libstdc++ for the avr target

[Keith Packard]

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

Vladimir V <vv.os.swe@gmail.com> writes:

> But If the GNU C++ standard library extensions where stdio_sync_filebuf
> belongs
> supports non-posix targets then maybe <unistd.h> include should be
> moved inside  _GLIBCXX_HAVE_UNISTD_H guard?
> This should provide backward compatibility.

I thought about using a #ifdef guard, but that relies on that definition
being available when building applications using the header file, and I
don't whether the definition is only available while building libstdc++
or is also available when building applications using libstdc++


-- 
-keith

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

Re: Problem building libstdc++ for the avr target

[Vladimir V]

It should be available in installation in bits/c++config.h.


пн, 8 февр. 2021 г. в 00:06, Keith Packard <keithp@keithp.com>:

> Vladimir V <vv.os.swe@gmail.com> writes:
>
> > But If the GNU C++ standard library extensions where stdio_sync_filebuf
> > belongs
> > supports non-posix targets then maybe <unistd.h> include should be
> > moved inside  _GLIBCXX_HAVE_UNISTD_H guard?
> > This should provide backward compatibility.
>
> I thought about using a #ifdef guard, but that relies on that definition
> being available when building applications using the header file, and I
> don't whether the definition is only available while building libstdc++
> or is also available when building applications using libstdc++
>
>
> --
> -keith
>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 07/02/21 10:23 -0800, Keith Packard wrote:
>Vladimir V <vv.os.swe@gmail.com> writes:
>
>> Hello,
>>
>> I finally managed to test building for AVR with Keith's patch and
>> unistd.h removed (to enforce usage of stabs in filesystem).
>>
>> I identified a couple of minor build issues that are obvious from the
>> attached patch, Could you please have a look? They work with AVR target
>> and doesn't seem to cause regressions for x64 build.

I'll take a look ASAP, thanks.

>Hrm. Removing unistd.h from an installed file might affect applications
>using that file, so that doesn't seem ideal.

We routinely remove such transitive includes. Any code assuming that
<fstream> defines the contents of <unistd.h> is wrong and should be
fixed.

For example:
https://gcc.gnu.org/gcc-11/porting_to.html#header-dep-changes
https://gcc.gnu.org/gcc-10/porting_to.html#header-dep-changes
https://gcc.gnu.org/gcc-7/porting_to.html#header-dep-changes
https://gcc.gnu.org/gcc-6/porting_to.html#header-dep-changes


>Picolibc has a unistd.h to define the relevant interfaces, even though
>it doesn't require them. Perhaps avrlibc could do the same? Or even an
>empty file that would satisfy this requirement?
>
>> --- a/libstdc++-v3/src/c++17/fs_ops.cc
>> +++ b/libstdc++-v3/src/c++17/fs_ops.cc
>> @@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
>>  #else
>>    if (nofollow && is_symlink(st))
>>      ec = std::make_error_code(std::errc::not_supported);
>> -  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
>> +  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
>>      err = errno;
>>  #endif
>
>I'm afraid I don't understand what this piece does without studying the
>underlying code a bunch. But, if it's just namespace stuff in an
>internal implementation file, that seems fine to me.

The libstdc++-v3/src/filesystem/ops-common.h header declares a few
POSIX-y things in namespace __gnu_posix (which is then aliased as just
"posix"). Then when the C++ standard says filesystem operations behave
"as if by POSIX rename" or "as if by POSIX lstat" the code can be
written in terms of posix::rename or POSIX::lstat.

In this instance, we declare something as posix::mode_t and so should
refer to it using that name too.

I'll look at the rest of the patch ASAP.

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 07/02/21 14:55 +0100, Vladimir V wrote:
>Hello,
>
>I finally managed to test building for AVR with Keith's patch and
>unistd.h removed (to enforce usage of stabs in filesystem).
>
>I identified a couple of minor build issues that are obvious from the
>attached patch, Could you please have a look? They work with AVR target
>and doesn't seem to cause regressions for x64 build.
>
>Thank you.
>
>пт, 22 янв. 2021 г. в 15:46, Vladimir V <vv.os.swe@gmail.com>:
>
>> Thank you for the detailed clarification.
>>
>> My primary idea was to reduce the build dependencies between hosted
>> libstdc++
>> and target libc if some components can not be supported or never will be
>> used.
>> Also my general vision is that if something doesn't work it shouldn't be
>> available
>> in the environment. But I perfectly understand the rationale you explained
>> and requirements applied by standard.
>>
>> So I think not much can be done here at this point and further work should
>> be on the avr-libc side.
>>
>> Thank you.
>>
>> пт, 8 янв. 2021 г. в 19:21, Jonathan Wakely <jwakely@redhat.com>:
>>
>>> On 04/01/21 12:28 +0100, Vladimir V wrote:
>>> >Hello and Happy New Year!
>>> >
>>> >Getting back to the discussion we had about scalable libstdc++.
>>> >For sure it will be very helpful for embedded targets. There are huge
>>> >components with
>>> >well defined boundaries that are unlikely to be often used in minimalist
>>> >settings but require missing platform support.
>>> >One particular example I'm looking at right now is 'filesystem'. Although
>>> >it resorts to some
>>> >default stubs if corresponding API is not present, I think it would be
>>> >correct to remove it from the library
>>> >completely if it can not/should not be used on the target.
>>> >In case of avr-libc it is worse as the unistd is partially present but
>>> not
>>> >sufficient to satisfy all filesystem
>>> >dependencies.
>>> >
>>> >It looks like that after the filesystem support was implemented for c++17
>>> >standard there is no option to remove
>>> >it from the build (unlike for the filesystem-ts). Please correct me if I
>>> am
>>> >wrong but If it is like that I would like to make this configurable.
>>>
>>> You're right.
>>>
>>> My thinking was that the C++ standard requires the <filesystem> header
>>> to be present for a hosted implementation. It doesn't necessarily
>>> require the contents to *do* much, so it's conforming for them to
>>> exist but always report errors.
>>>
>>> >Would it be acceptable?
>>>
>>> I'm unsure.
>>>
>>> Currently we support "hosted" and "freestanding", but not really
>>> anything more fine-grained. The point of the configure switch
>>> --disable-libstdcxx-filesystem-ts was not really to allow people to
>>> build without it, but more to enable it for targets where it wasn't on
>>> by default because it hadn't been tested yet.
>>>
>>> I've recently been trying to move away from having missing features,
>>> and make things available unconditionally, but to use useless stubs or
>>> return errors for targets that can't support it. For example, GCC 11
>>> always defines the std::thread type, even if the target can't actually
>>> use it to create new threads. The idea is to always provide a complete
>>> implementation, not have the feature set vary by target (I see a lot
>>> of confusion on places like StackOverflow, where the answer is "yeah
>>> sorry, that header is empty on your OS").
>>>
>>> What you're suggesting would lead to a more modular libstdc++ where
>>> you can enable/disable components, similar to what newlib does. If we
>>> did that for more than just the std::filesystem library things would
>>> get complicated (it increases the maintenance and testing burden, and
>>> we need to be careful not to create dependencies on anything from a
>>> conditionally-present feature).
>>>
>>> Would your suggestion actually result in smaller/faster binaries for
>>> AVR? Or just remove features that aren't fully functional if you
>>> attempt to use them? If having those features in the library doesn't
>>> actually cause bigger/slower binaries, then I don't really see a
>>> problem with them being present.
>>>
>>> Tangentially, the direction the C++ standard seems to be going is to
>>> grow the feature set of freestanding. That would allow more things to
>>> be used in freestanding environments, rather than the very limited set
>>> of features currently guaranteed to be available. That doesn't really
>>> help you though, as you want a hosted env with things like basic I/O,
>>> but not the full set of features usually present for hosted.
>>>
>>>
>>>
>>>

From ac970e6a376def43c510f9091d342a014f23fe8f Mon Sep 17 00:00:00 2001
>From: Vladimir Vishnevsky <vv.os.swe@gmail.com>
>Date: Sat, 6 Feb 2021 12:25:24 +0100
>Subject: [PATCH] libstdc++: Fix build failure for targets without unistd.h
>
>The patch fixes build issues occuring if build parameter
>"--enable-cstdio=stdio_pure" is specified and no unistd.h is
>present in the environment.
>
>2021-02-07 Vladimir Vishnevsky <vv.os.swe@gmail.com>
>
>libstdc++-v3/ChangeLog:
>        Fix build failure for targets without unistd.h.
>        * include/ext/stdio_sync_filebuf.h: Unused <unistd.h> include removed.
>        * src/c++17/fs_ops.cc: Proper namespace specified for locally defined
>        mode_t.
>---
> libstdc++-v3/include/ext/stdio_sync_filebuf.h | 1 -
> libstdc++-v3/src/c++17/fs_ops.cc              | 2 +-
> 2 files changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/libstdc++-v3/include/ext/stdio_sync_filebuf.h b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>index 178b471957a..90765e55831 100644
>--- a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>+++ b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>@@ -32,7 +32,6 @@
> #pragma GCC system_header
>
> #include <streambuf>
>-#include <unistd.h>

I don't see why this file includes <unistd.h>, it doesn't *seem* to
need it. Even if it's needed, the correct way to include it is:

#ifdef _GLIBCXX_HAVE_UNISTD_H
# include <unistd.h>
#endif

So we should do that at least, even if we don't remove it.


> #include <cstdio>
> #include <bits/c++io.h>  // For __c_file
> #include <bits/move.h>   // For __exchange
>diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
>index 72755c98a5a..04a559ab1d6 100644
>--- a/libstdc++-v3/src/c++17/fs_ops.cc
>+++ b/libstdc++-v3/src/c++17/fs_ops.cc
>@@ -1130,7 +1130,7 @@ fs::permissions(const path& p, perms prms, perm_options opts,
> #else
>   if (nofollow && is_symlink(st))
>     ec = std::make_error_code(std::errc::not_supported);
>-  else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
>+  else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
>     err = errno;
> #endif

Yes, this is correct, thanks.

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 08/02/21 17:45 +0000, Jonathan Wakely wrote:
>On 07/02/21 14:55 +0100, Vladimir V wrote:
>>diff --git a/libstdc++-v3/include/ext/stdio_sync_filebuf.h b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>>index 178b471957a..90765e55831 100644
>>--- a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>>+++ b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
>>@@ -32,7 +32,6 @@
>>#pragma GCC system_header
>>
>>#include <streambuf>
>>-#include <unistd.h>
>
>I don't see why this file includes <unistd.h>, it doesn't *seem* to
>need it. Even if it's needed, the correct way to include it is:
>
>#ifdef _GLIBCXX_HAVE_UNISTD_H
># include <unistd.h>
>#endif
>
>So we should do that at least, even if we don't remove it.

Ah, I see you already said that downthread. I should have finished
reading the thread, sorry.

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the reply.

>> I don't see why this file includes <unistd.h>, it doesn't *seem* to
>> need it. Even if it's needed, the correct way to include it is:

So it was there since the initial commit but during my quick walk through
the history
I didn't find what API from <unistd.h> was used.

So, in the patch I just deleted the include but as Keith suggested it can
affect the client code.
If the policy is:

>> We routinely remove such transitive includes. Any code assuming that
>> <fstream> defines the contents of <unistd.h> is wrong and should be
>> fixed.

then I would keep it like that.
Or would it be better to update the patch to move it inside the guard?



пн, 8 февр. 2021 г. в 18:47, Jonathan Wakely <jwakely@redhat.com>:

> On 08/02/21 17:45 +0000, Jonathan Wakely wrote:
> >On 07/02/21 14:55 +0100, Vladimir V wrote:
> >>diff --git a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
> b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
> >>index 178b471957a..90765e55831 100644
> >>--- a/libstdc++-v3/include/ext/stdio_sync_filebuf.h
> >>+++ b/libstdc++-v3/include/ext/stdio_sync_filebuf.h
> >>@@ -32,7 +32,6 @@
> >>#pragma GCC system_header
> >>
> >>#include <streambuf>
> >>-#include <unistd.h>
> >
> >I don't see why this file includes <unistd.h>, it doesn't *seem* to
> >need it. Even if it's needed, the correct way to include it is:
> >
> >#ifdef _GLIBCXX_HAVE_UNISTD_H
> ># include <unistd.h>
> >#endif
> >
> >So we should do that at least, even if we don't remove it.
>
> Ah, I see you already said that downthread. I should have finished
> reading the thread, sorry.
>
>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 08/02/21 23:25 +0100, Vladimir V wrote:
>Thank you for the reply.
>
>>> I don't see why this file includes <unistd.h>, it doesn't *seem* to
>>> need it. Even if it's needed, the correct way to include it is:
>
>So it was there since the initial commit but during my quick walk through
>the history
>I didn't find what API from <unistd.h> was used.

My best guess is that it is there because that file previously used
lstat and the linux lstat(2) man page (IMHO incorrectly) says that
<unistd.h> is needed.

>So, in the patch I just deleted the include but as Keith suggested it can
>affect the client code.
>If the policy is:
>
>>> We routinely remove such transitive includes. Any code assuming that
>>> <fstream> defines the contents of <unistd.h> is wrong and should be
>>> fixed.
>
>then I would keep it like that.
>Or would it be better to update the patch to move it inside the guard?

I will commit your patch unchanged. In practice it is unlikely to
affect anybody, because that header is only used when compiling
libstdc++ itself. I don't see any code in the Debian archives using it
except for GCC itself and one testcase in the Clang repo.

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the review.

> My best guess is that it is there because that file previously used
> lstat and the linux lstat(2) man page (IMHO incorrectly) says that
> <unistd.h> is needed.

I got the same feeling as well.



вт, 9 февр. 2021 г. в 11:48, Jonathan Wakely <jwakely@redhat.com>:

> On 08/02/21 23:25 +0100, Vladimir V wrote:
> >Thank you for the reply.
> >
> >>> I don't see why this file includes <unistd.h>, it doesn't *seem* to
> >>> need it. Even if it's needed, the correct way to include it is:
> >
> >So it was there since the initial commit but during my quick walk through
> >the history
> >I didn't find what API from <unistd.h> was used.
>
> My best guess is that it is there because that file previously used
> lstat and the linux lstat(2) man page (IMHO incorrectly) says that
> <unistd.h> is needed.
>
> >So, in the patch I just deleted the include but as Keith suggested it can
> >affect the client code.
> >If the policy is:
> >
> >>> We routinely remove such transitive includes. Any code assuming that
> >>> <fstream> defines the contents of <unistd.h> is wrong and should be
> >>> fixed.
> >
> >then I would keep it like that.
> >Or would it be better to update the patch to move it inside the guard?
>
> I will commit your patch unchanged. In practice it is unlikely to
> affect anybody, because that header is only used when compiling
> libstdc++ itself. I don't see any code in the Debian archives using it
> except for GCC itself and one testcase in the Clang repo.
>
>
>

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Hello.

Recently I tried the 'freestanding' build of the libstdc++ for the avr to
verify that my fixes were sufficient for it as well.
Unfortunately, I encountered the following error:
../../../../libstdc++-v3/libsupc++/new_opa.cc: In function 'void*
__gnu_cxx::aligned_alloc(std::size_t, std::size_t)':
../../../../libstdc++-v3/libsupc++/new_opa.cc:97:28: error: 'malloc' was
not declared in this scope
   97 |   void* const malloc_ptr = malloc(sz + al);

My investigation showed that in the chain of includes the stdlib.h of the
avr-libc is not present.
It looks like includes are resolved in the following way (here are the
original paths as they are linked from out folders during build):
new_opa.cc -> libstdc++-v3/include/c_compatibility/stdlib.h ->
libstdc++-v3/include/c_global/cstdlib
In cstdlib, for non-hosted builds stdlib.h is not included.

This problem is not present if the non-hosted build is done with
'with-newlib' flag instead of 'with-avrlibc' (common workaround as the
avrlibc build was fixed only recently).
Might it be that some include order depends on the target libc? Or maybe
some implicit conditions are involved?

Could you please support me with this issue?

Thank you.




вт, 9 февр. 2021 г. в 11:54, Vladimir V <vv.os.swe@gmail.com>:

> Thank you for the review.
>
> > My best guess is that it is there because that file previously used
> > lstat and the linux lstat(2) man page (IMHO incorrectly) says that
> > <unistd.h> is needed.
>
> I got the same feeling as well.
>
>
>
> вт, 9 февр. 2021 г. в 11:48, Jonathan Wakely <jwakely@redhat.com>:
>
>> On 08/02/21 23:25 +0100, Vladimir V wrote:
>> >Thank you for the reply.
>> >
>> >>> I don't see why this file includes <unistd.h>, it doesn't *seem* to
>> >>> need it. Even if it's needed, the correct way to include it is:
>> >
>> >So it was there since the initial commit but during my quick walk through
>> >the history
>> >I didn't find what API from <unistd.h> was used.
>>
>> My best guess is that it is there because that file previously used
>> lstat and the linux lstat(2) man page (IMHO incorrectly) says that
>> <unistd.h> is needed.
>>
>> >So, in the patch I just deleted the include but as Keith suggested it can
>> >affect the client code.
>> >If the policy is:
>> >
>> >>> We routinely remove such transitive includes. Any code assuming that
>> >>> <fstream> defines the contents of <unistd.h> is wrong and should be
>> >>> fixed.
>> >
>> >then I would keep it like that.
>> >Or would it be better to update the patch to move it inside the guard?
>>
>> I will commit your patch unchanged. In practice it is unlikely to
>> affect anybody, because that header is only used when compiling
>> libstdc++ itself. I don't see any code in the Debian archives using it
>> except for GCC itself and one testcase in the Clang repo.
>>
>>
>>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 25/03/21 10:27 +0100, Vladimir V wrote:
>Hello.
>
>Recently I tried the 'freestanding' build of the libstdc++ for the avr to
>verify that my fixes were sufficient for it as well.
>Unfortunately, I encountered the following error:
>../../../../libstdc++-v3/libsupc++/new_opa.cc: In function 'void*
>__gnu_cxx::aligned_alloc(std::size_t, std::size_t)':
>../../../../libstdc++-v3/libsupc++/new_opa.cc:97:28: error: 'malloc' was
>not declared in this scope
>   97 |   void* const malloc_ptr = malloc(sz + al);
>
>My investigation showed that in the chain of includes the stdlib.h of the
>avr-libc is not present.
>It looks like includes are resolved in the following way (here are the
>original paths as they are linked from out folders during build):
>new_opa.cc -> libstdc++-v3/include/c_compatibility/stdlib.h ->
>libstdc++-v3/include/c_global/cstdlib
>In cstdlib, for non-hosted builds stdlib.h is not included.
>
>This problem is not present if the non-hosted build is done with
>'with-newlib' flag instead of 'with-avrlibc' (common workaround as the
>avrlibc build was fixed only recently).
>Might it be that some include order depends on the target libc? Or maybe
>some implicit conditions are involved?
>
>Could you please support me with this issue?

I think this is needed:

--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -54,6 +54,10 @@ extern "C"
    void *posix_memalign(void **, size_t alignment, size_t size);
  # elif _GLIBCXX_HAVE_MEMALIGN
    void *memalign(size_t alignment, size_t size);
+#else
+  // A freestanding C runtime may not provide "malloc" -- but there is no
+  // other reasonable way to implement "operator new".
+  void *malloc (std::size_t);   with GCC 7.5
  # endif
  }
  #endif

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

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

On 25/03/21 12:36 +0000, Jonathan Wakely wrote:
>On 25/03/21 10:27 +0100, Vladimir V wrote:
>>Hello.
>>
>>Recently I tried the 'freestanding' build of the libstdc++ for the avr to
>>verify that my fixes were sufficient for it as well.
>>Unfortunately, I encountered the following error:
>>../../../../libstdc++-v3/libsupc++/new_opa.cc: In function 'void*
>>__gnu_cxx::aligned_alloc(std::size_t, std::size_t)':
>>../../../../libstdc++-v3/libsupc++/new_opa.cc:97:28: error: 'malloc' was
>>not declared in this scope
>>  97 |   void* const malloc_ptr = malloc(sz + al);
>>
>>My investigation showed that in the chain of includes the stdlib.h of the
>>avr-libc is not present.
>>It looks like includes are resolved in the following way (here are the
>>original paths as they are linked from out folders during build):
>>new_opa.cc -> libstdc++-v3/include/c_compatibility/stdlib.h ->
>>libstdc++-v3/include/c_global/cstdlib
>>In cstdlib, for non-hosted builds stdlib.h is not included.
>>
>>This problem is not present if the non-hosted build is done with
>>'with-newlib' flag instead of 'with-avrlibc' (common workaround as the
>>avrlibc build was fixed only recently).
>>Might it be that some include order depends on the target libc? Or maybe
>>some implicit conditions are involved?
>>
>>Could you please support me with this issue?
>
>I think this is needed:
>
>--- a/libstdc++-v3/libsupc++/new_opa.cc
>+++ b/libstdc++-v3/libsupc++/new_opa.cc
>@@ -54,6 +54,10 @@ extern "C"
>   void *posix_memalign(void **, size_t alignment, size_t size);
> # elif _GLIBCXX_HAVE_MEMALIGN
>   void *memalign(size_t alignment, size_t size);
>+#else
>+  // A freestanding C runtime may not provide "malloc" -- but there is no
>+  // other reasonable way to implement "operator new".
>+  void *malloc (std::size_t);   with GCC 7.5

Oops, there was a stray "with GCC 7.5" that got into my clipboard
there!

I've pushed the attached patch, please test and confirm if it fixes
your problem.



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

commit 15d649f79d6b6dc336f6a32eec242b652a262a82
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Mar 25 18:24:37 2021

    libstdc++: Declare malloc for freestanding
    
    For a target with none of aligned_alloc, memalign etc. we defined our
    own aligned_alloc using malloc, so we need a declaration of malloc. As
    in libsupc++/new_op.cc we need to declare it ourselves for freestanding
    environments.
    
    libstdc++-v3/ChangeLog:
    
            * libsupc++/new_opa.cc [!_GLIBCXX_HOSTED]: Declare malloc.

diff --git a/libstdc++-v3/libsupc++/new_opa.cc b/libstdc++-v3/libsupc++/new_opa.cc
index 59296226e83..6b78729dc24 100644
--- a/libstdc++-v3/libsupc++/new_opa.cc
+++ b/libstdc++-v3/libsupc++/new_opa.cc
@@ -54,6 +54,10 @@ extern "C"
   void *posix_memalign(void **, size_t alignment, size_t size);
 # elif _GLIBCXX_HAVE_MEMALIGN
   void *memalign(size_t alignment, size_t size);
+# else
+  // A freestanding C runtime may not provide "malloc" -- but there is no
+  // other reasonable way to implement "operator new".
+  void *malloc(size_t);
 # endif
 }
 #endif

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Thank you for the quick reply.

I am wondering if the freestanding libstdc++ requires that only
freestanding subset of the underlying libc is used?
avr-libc has stdlib.h with malloc declaration but it is not 'visible' in
this code. I was thinking that maybe something like that
would work (works in my case):

diff --git a/libstdc++-v3/include/c_global/cstdlib
b/libstdc++-v3/include/c_global/cstdlib
index 47b954cf2..f04fd08de 100644
--- a/libstdc++-v3/include/c_global/cstdlib
+++ b/libstdc++-v3/include/c_global/cstdlib
@@ -43,7 +43,8 @@
 #ifndef _GLIBCXX_CSTDLIB
 #define _GLIBCXX_CSTDLIB 1

-#if !_GLIBCXX_HOSTED
+#if !_GLIBCXX_HOSTED && !_GLIBCXX_HAVE_STDLIB_H
+
 // The C standard does not require a freestanding implementation to
 // provide <stdlib.h>.  However, the C++ standard does still require
 // <cstdlib> -- but only the functionality mentioned in

It includes the stdlib.h from target libc if it is present.


чт, 25 мар. 2021 г. в 19:27, Jonathan Wakely <jwakely@redhat.com>:

> On 25/03/21 12:36 +0000, Jonathan Wakely wrote:
> >On 25/03/21 10:27 +0100, Vladimir V wrote:
> >>Hello.
> >>
> >>Recently I tried the 'freestanding' build of the libstdc++ for the avr to
> >>verify that my fixes were sufficient for it as well.
> >>Unfortunately, I encountered the following error:
> >>../../../../libstdc++-v3/libsupc++/new_opa.cc: In function 'void*
> >>__gnu_cxx::aligned_alloc(std::size_t, std::size_t)':
> >>../../../../libstdc++-v3/libsupc++/new_opa.cc:97:28: error: 'malloc' was
> >>not declared in this scope
> >>  97 |   void* const malloc_ptr = malloc(sz + al);
> >>
> >>My investigation showed that in the chain of includes the stdlib.h of the
> >>avr-libc is not present.
> >>It looks like includes are resolved in the following way (here are the
> >>original paths as they are linked from out folders during build):
> >>new_opa.cc -> libstdc++-v3/include/c_compatibility/stdlib.h ->
> >>libstdc++-v3/include/c_global/cstdlib
> >>In cstdlib, for non-hosted builds stdlib.h is not included.
> >>
> >>This problem is not present if the non-hosted build is done with
> >>'with-newlib' flag instead of 'with-avrlibc' (common workaround as the
> >>avrlibc build was fixed only recently).
> >>Might it be that some include order depends on the target libc? Or maybe
> >>some implicit conditions are involved?
> >>
> >>Could you please support me with this issue?
> >
> >I think this is needed:
> >
> >--- a/libstdc++-v3/libsupc++/new_opa.cc
> >+++ b/libstdc++-v3/libsupc++/new_opa.cc
> >@@ -54,6 +54,10 @@ extern "C"
> >   void *posix_memalign(void **, size_t alignment, size_t size);
> > # elif _GLIBCXX_HAVE_MEMALIGN
> >   void *memalign(size_t alignment, size_t size);
> >+#else
> >+  // A freestanding C runtime may not provide "malloc" -- but there is no
> >+  // other reasonable way to implement "operator new".
> >+  void *malloc (std::size_t);   with GCC 7.5
>
> Oops, there was a stray "with GCC 7.5" that got into my clipboard
> there!
>
> I've pushed the attached patch, please test and confirm if it fixes
> your problem.
>
>
>

Re: Problem building libstdc++ for the avr target

[Jonathan Wakely]

On 25/03/21 22:37 +0100, Vladimir V via Libstdc++ wrote:
>Thank you for the quick reply.
>
>I am wondering if the freestanding libstdc++ requires that only
>freestanding subset of the underlying libc is used?

The freestanding library is always allowed to provide more features
than the minimum set required. So it is certainly possible to expose
the whole of the libc <stdlib.h> through <cstdlib> if we wish to.

We shouldn't do that at this stage of the GCC 11 cycle but we could
consider it for GCC 12.

Does the change I pushed earlier solve your build error with
new_opa.cc though?

Re: Problem building libstdc++ for the avr target

[Vladimir V]

Yes, thank you. It solves the issue.

пт, 26 мар. 2021 г. в 00:02, Jonathan Wakely <jwakely@redhat.com>:

> On 25/03/21 22:37 +0100, Vladimir V via Libstdc++ wrote:
> >Thank you for the quick reply.
> >
> >I am wondering if the freestanding libstdc++ requires that only
> >freestanding subset of the underlying libc is used?
>
> The freestanding library is always allowed to provide more features
> than the minimum set required. So it is certainly possible to expose
> the whole of the libc <stdlib.h> through <cstdlib> if we wish to.
>
> We shouldn't do that at this stage of the GCC 11 cycle but we could
> consider it for GCC 12.
>
> Does the change I pushed earlier solve your build error with
> new_opa.cc though?
>
>