public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
* [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
@ 2023-12-06 17:50 Hans-Peter Nilsson
  2023-12-06 19:52 ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Hans-Peter Nilsson @ 2023-12-06 17:50 UTC (permalink / raw)
  To: newlib

As you've seen in other patch submissions, there was recently a change
in gcc such that implicit function declarations and type mismatches
are now errors, no longer just warnings.

Problems are in port-specific code, as I don't see any breakage for
arm-eabi.  Patches for some other ports are in, and I guess lots more
will trickle in as port maintainers and other people find out and fix.
This two-set of patches handles newlib and libgloss for cris-elf.

A gcc regtest round shows no regressions compared to before the
"breaking" gcc commit.  Caveat: a patch-set adding a _getentropy stub
from August (IIRC) causes lots of FAILs; some of those are worked
around in gcc to get better coverage.  Film at 11.  Anyway, a
regtest-round with these patches applied to a newlib checkout before
the _getentropy patches shows no regressions for cris-elf+cris-sim,
and working around *some* of the breakage with a slightly more recent
newlib checkout (dab15f6740ff) shows that to be the only issue.

brgds, H-P

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-06 17:50 [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors Hans-Peter Nilsson
@ 2023-12-06 19:52 ` Torbjorn SVENSSON
  2023-12-15  4:24   ` Hans-Peter Nilsson
  0 siblings, 1 reply; 8+ messages in thread
From: Torbjorn SVENSSON @ 2023-12-06 19:52 UTC (permalink / raw)
  To: newlib

Hi Hans-Peter,

On 2023-12-06 18:50, Hans-Peter Nilsson wrote:
> Caveat: a patch-set adding a _getentropy stub
> from August (IIRC) causes lots of FAILs; some of those are worked
> around in gcc to get better coverage.

What problems are there with the _getentropy stubs that I've submitted?

Kind regards,
Torbjörn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-06 19:52 ` Torbjorn SVENSSON
@ 2023-12-15  4:24   ` Hans-Peter Nilsson
  2023-12-21 17:37     ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Hans-Peter Nilsson @ 2023-12-15  4:24 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: newlib

> Date: Wed, 6 Dec 2023 20:52:48 +0100
> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>

(Sorry for the late reply: the newlib mailer appears to have
directed your mail only to the newlib without even a CC to
me, so your reply got into my newlib folder, unfortunately
not visited daily.)

> Hi Hans-Peter,
> 
> On 2023-12-06 18:50, Hans-Peter Nilsson wrote:
> > Caveat: a patch-set adding a _getentropy stub
> > from August (IIRC) causes lots of FAILs; some of those are worked
> > around in gcc to get better coverage.
> 
> What problems are there with the _getentropy stubs that I've submitted?

I hope to get into details later, as indicated by the "film
at 11".  The problem is fairly visible with a standard
test-run for cris-elf (with simulator and baseboard
cris-sim): all libstdc++ tests fail with a linker warning,
as its configure tests detect a presence of _getentropy but
its reference trigs the stub warning (the .gnu.warning
thing).  I *think* it's also visible for a build with
arm-eabi+arm-sim which made me wonder how you configured and
tested (you may have stated, I haven't looked).  Adding a
patch to "prune" the stub-warning in libstdc++ prune.exp
only exposes runtime errors when _getentropy is actually
called.  Those errors are not present when _getentropy is
not detected (and not used).

Looks like I managed to actually state the gist of the
problem.  Missing are details such as (todo) proposed best
actions, besides reverting that patch, to better handle the
problem *you* say.  That's also all I can say for now.

Feel very free to wait until I come back to this: I can't
focus on this issue right now; just thought I'd reply with
what state I have at the top of my head, since you asked.
Sorry!

brgds, H-P

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-15  4:24   ` Hans-Peter Nilsson
@ 2023-12-21 17:37     ` Torbjorn SVENSSON
  2023-12-21 18:26       ` Hans-Peter Nilsson
  0 siblings, 1 reply; 8+ messages in thread
From: Torbjorn SVENSSON @ 2023-12-21 17:37 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: newlib

Hello Hans-Peter,

I finally had some time to go back to this topic.

On 2023-12-15 05:24, Hans-Peter Nilsson wrote:
>> Date: Wed, 6 Dec 2023 20:52:48 +0100
>> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
>> What problems are there with the _getentropy stubs that I've submitted?
> 
> I hope to get into details later, as indicated by the "film
> at 11".  The problem is fairly visible with a standard
> test-run for cris-elf (with simulator and baseboard
> cris-sim): all libstdc++ tests fail with a linker warning,
> as its configure tests detect a presence of _getentropy but
> its reference trigs the stub warning (the .gnu.warning
> thing).  I *think* it's also visible for a build with
> arm-eabi+arm-sim which made me wonder how you configured and
> tested (you may have stated, I haven't looked).  Adding a
> patch to "prune" the stub-warning in libstdc++ prune.exp
> only exposes runtime errors when _getentropy is actually
> called.  Those errors are not present when _getentropy is
> not detected (and not used).


When I run the tests, I supply all the syscalls functions in my fixture 
to avoid the stubs warnings. This is nothing new to the _getentropy 
function though as the same problem is visible with for example _close.

If I try to link the following minimal C application with the 
arm-none-eabi target, I get the below warnings.

$ cat foo.c
int main() {
     return 0;
}


$ .../bin/arm-none-eabi-g++ -mcpu=cortex-m4 -mfloat-abi=soft -o foo.elf 
foo.c --specs=nosys.specs
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-closer.o): 
in function `_close_r':
(.text._close_r+0xc): warning: _close is not implemented and will always 
fail
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-lseekr.o): 
in function `_lseek_r':
(.text._lseek_r+0x10): warning: _lseek is not implemented and will 
always fail
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-readr.o): 
in function `_read_r':
(.text._read_r+0x10): warning: _read is not implemented and will always fail
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
.../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-writer.o): 
in function `_write_r':
(.text._write_r+0x10): warning: _write is not implemented and will 
always fail


Do you mean that you do not get these stub warnings for the cris-elf target?
If you do get them, in what way is _getentropy different from for 
example _close?

(the above is produced using newlib 7a45daa, GCC d0603dfe9 and binutils 
80d2ef0c4)

Kind regards,
Torbjörn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-21 17:37     ` Torbjorn SVENSSON
@ 2023-12-21 18:26       ` Hans-Peter Nilsson
  2023-12-23 10:58         ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Hans-Peter Nilsson @ 2023-12-21 18:26 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: newlib

> Date: Thu, 21 Dec 2023 18:37:14 +0100
> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>

> Hello Hans-Peter,

Hi.

> I finally had some time to go back to this topic.

I don't have time to dig into this just yet.  As I mentioned
before, I'll get back to you, please be patient.

I'll still answer your questions below even though I fear
they lead off into a tangent.  I'll get back to this and add
a proposed solution.  Until then:

> On 2023-12-15 05:24, Hans-Peter Nilsson wrote:
> >> Date: Wed, 6 Dec 2023 20:52:48 +0100
> >> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
> >> What problems are there with the _getentropy stubs that I've submitted?
> > 
> > I hope to get into details later, as indicated by the "film
> > at 11".  The problem is fairly visible with a standard
> > test-run for cris-elf (with simulator and baseboard
> > cris-sim): all libstdc++ tests fail with a linker warning,
> > as its configure tests detect a presence of _getentropy but
> > its reference trigs the stub warning (the .gnu.warning
> > thing).  I *think* it's also visible for a build with
> > arm-eabi+arm-sim which made me wonder how you configured and
> > tested (you may have stated, I haven't looked).  Adding a
> > patch to "prune" the stub-warning in libstdc++ prune.exp
> > only exposes runtime errors when _getentropy is actually
> > called.  Those errors are not present when _getentropy is
> > not detected (and not used).
> 
> 
> When I run the tests, I supply all the syscalls functions in my fixture 
> to avoid the stubs warnings. This is nothing new to the _getentropy 
> function though as the same problem is visible with for example _close.

Not sure I get you here.  If your "fixture" doesn't do that
for _close, you haven't implemented a proper syscall
function for _close.

But, you shouldn't have to add special machinery, it should
already be there in your dejagnu baseboard file.

Care to share it?  What simulator are you targetting?  The
arm-sim in the gdb project notoriously doesn't support
anything but the default GCC target, not -mcpu=cortex-m4, so
I think you don't test with 'make check
RUNTESTFLAGS=--target_board=arm-sim', i.e. not with the
arm-sim.exp provided by dejagnu, right?

To wit, if you use something that gives other results than
when configuring for --target=arm-eabi and testing with
RUNTESTFLAGS=--target_board=arm-sim then your gcc testing
isn't matching the expectations in libstdc++-v3 for a newlib
target (IOW "not correct").

Some outdated information which should still help you get a
working environment is in gcc.gnu.org/simtest-howto.html
(but I suggest build and install sim+binutils separately,
with just newlib and gcc combined).

> If I try to link the following minimal C application with the 
> arm-none-eabi target, I get the below warnings.
> 
> $ cat foo.c
> int main() {
>      return 0;
> }
> 
> 
> $ .../bin/arm-none-eabi-g++ -mcpu=cortex-m4 -mfloat-abi=soft -o foo.elf 
> foo.c --specs=nosys.specs
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-closer.o): 
> in function `_close_r':
> (.text._close_r+0xc): warning: _close is not implemented and will always 
> fail
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-lseekr.o): 
> in function `_lseek_r':
> (.text._lseek_r+0x10): warning: _lseek is not implemented and will 
> always fail
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-readr.o): 
> in function `_read_r':
> (.text._read_r+0x10): warning: _read is not implemented and will always fail
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld: 
> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-writer.o): 
> in function `_write_r':
> (.text._write_r+0x10): warning: _write is not implemented and will 
> always fail
> 
> 
> Do you mean that you do not get these stub warnings for the cris-elf target?

(When *configuring* libstdc++ I'd get those warnings, that's
why linking tests are disabled for newlib there.)

But, when called as part of the testsuite, *no*: not with a
proper syscall library, such as when "-sim3" is passed when
linking (see the libsyslinux.a stuff in libgloss/cris and
e.g. LIB_SPEC in gcc/config/cris/cris.h), as happens when
running with --target_board=cris-sim.

Don't arm-eabi+arm-sim get lots of libstdc++-v3 errors that
weren't there before your newlib _getentropy patch?

Can you share your full environment such that I can repeat
your observations?

> If you do get them, in what way is _getentropy different from for 
> example _close?

_close is implemented in libsyslinux.a, _getentropy not.
(No syscall for that so the stub warning is "correct".)

> (the above is produced using newlib 7a45daa, GCC d0603dfe9 and binutils 
> 80d2ef0c4)

Again, all my observations are repeatable by anyone:
- Install binutils for --target=cris-elf --prefix=<whatever>
- Ditto the cris simulator in gdb.
- Configure a "combined" tree with gcc and newlib.
- Build gcc+newlib using the same --target=cris-elf --prefix=<whatever>
- 'make check RUNTESTFLAGS=--target_board=cris-sim'.
- Observe the multitude of libstdc++-v3 tests failing that
weren't there before your _getentropy patch.

Please share your full testing environment such that I can
repeat your observations.

I'll get back with a proper solution, one where
arm-eabi+arm-sim, cris-elf+cris-sim and your own environment
all work (hopefully, or help you get a proper testing
environment), before next summer.

brgds, H-p

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-21 18:26       ` Hans-Peter Nilsson
@ 2023-12-23 10:58         ` Torbjorn SVENSSON
  2023-12-23 16:44           ` Hans-Peter Nilsson
  0 siblings, 1 reply; 8+ messages in thread
From: Torbjorn SVENSSON @ 2023-12-23 10:58 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Newlib



On 2023-12-21 19:26, Hans-Peter Nilsson wrote:
>> Date: Thu, 21 Dec 2023 18:37:14 +0100
>> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
> 
>> Hello Hans-Peter,
> 
> Hi.
> 
>> I finally had some time to go back to this topic.
> 
> I don't have time to dig into this just yet.  As I mentioned
> before, I'll get back to you, please be patient.
> 
> I'll still answer your questions below even though I fear
> they lead off into a tangent.  I'll get back to this and add
> a proposed solution.  Until then:
> 
>> On 2023-12-15 05:24, Hans-Peter Nilsson wrote:
>>>> Date: Wed, 6 Dec 2023 20:52:48 +0100
>>>> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
>>>> What problems are there with the _getentropy stubs that I've submitted?
>>>
>>> I hope to get into details later, as indicated by the "film
>>> at 11".  The problem is fairly visible with a standard
>>> test-run for cris-elf (with simulator and baseboard
>>> cris-sim): all libstdc++ tests fail with a linker warning,
>>> as its configure tests detect a presence of _getentropy but
>>> its reference trigs the stub warning (the .gnu.warning
>>> thing).  I *think* it's also visible for a build with
>>> arm-eabi+arm-sim which made me wonder how you configured and
>>> tested (you may have stated, I haven't looked).  Adding a
>>> patch to "prune" the stub-warning in libstdc++ prune.exp
>>> only exposes runtime errors when _getentropy is actually
>>> called.  Those errors are not present when _getentropy is
>>> not detected (and not used).
>>
>>
>> When I run the tests, I supply all the syscalls functions in my fixture
>> to avoid the stubs warnings. This is nothing new to the _getentropy
>> function though as the same problem is visible with for example _close.
> 
> Not sure I get you here.  If your "fixture" doesn't do that
> for _close, you haven't implemented a proper syscall
> function for _close.
> 
> But, you shouldn't have to add special machinery, it should
> already be there in your dejagnu baseboard file.

The baseboard file *is* the fixture.

> Care to share it?  What simulator are you targetting?  The
> arm-sim in the gdb project notoriously doesn't support
> anything but the default GCC target, not -mcpu=cortex-m4, so
> I think you don't test with 'make check
> RUNTESTFLAGS=--target_board=arm-sim', i.e. not with the
> arm-sim.exp provided by dejagnu, right?

I'm using a combination of real evaluation boards and the qemu simulator.
Unfortunately, I cannot share it as it's tied to how the internal 
systems are setup at ST.

What I can share is the overall setup and that is:

1. Running a patched qemu that just adds custom machine that has the 
same address layout as a real STM32 device so that I can take the same 
binary and run it on real hardware in order to verify if a failure is 
due to qemu or if it's a real problem.
The flags used to run qemu-system-arm varies with the target, but for 
Cortex-M4, it would be:
qemu-system-arm -nographic -machine <the-custom-qemu-machine> -cpu 
cortex-m4 -semihosting -monitor /dev/null -kernel <image.elf>

2. The baseboard file defines some flags:
cflags: "[libgloss_include_flags] [newlib_include_flags]"
ldflags: " -Wl,--start-group -lc -lm -Wl,--end-group --specs=nosys.specs 
-Wl,--allow-multiple-definition -Wl,-u,_isatty,-u,_fstat"

It also adds a set of object files to link into the elf file that 
contains a custom startup code (aligned with qemu and real STM32 
targets) and the following functions:

* __initialize_args
* _clock
* _close
* _execve
* _exit
* _fork
* _getentropy
* _getpid
* _gettimeofday
* _isatty
* _kill
* _link
* _lseek
* _open
* _read
* _rename
* _sbrk
* _stat
* _swiclose
* _swilseek
* _swiopen
* _swiread
* _swistat
* _swiwrite
* _system
* _times
* _unlink
* _wait
* _write
* getcwd
* initialise_monitor_handles
* mkdir

And then finally, it uses a custom ldscript that matches the 
requirements for STM32 targets.

3. The qemu execution is always performed on a Linux system regardless 
if the dejagnu tests are executed on Windows, Linux or mac. This is to 
ensure that the qemu process does not introduce platform variations and 
makes the test results comparable between the platforms runs.

> To wit, if you use something that gives other results than
> when configuring for --target=arm-eabi and testing with
> RUNTESTFLAGS=--target_board=arm-sim then your gcc testing
> isn't matching the expectations in libstdc++-v3 for a newlib
> target (IOW "not correct").

I'm pretty sure it didn't match the expectations before either as it 
picked up the header definition for _getentropy during the configure 
stage, but when linking, there was no such function defined in the 
newlib provided archives, thus the following error (taken from the 
commit message of b9e867d088935d9f0bf312e6dbf3e4976850dfd3) was raised 
during testing of the g++ tool:

 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/bin/ld: 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libstdc++.a(random.o): 
in function `std::(anonymous namespace)::__libc_getentropy(void*)':
     (.text._ZNSt12_GLOBAL__N_117__libc_getentropyEPv+0x8): undefined 
reference to `getentropy'
 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/bin/ld: 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libstdc++.a(random.o): 
in function 
`std::random_device::_M_init(std::__cxx11::basic_string<char, 
std::char_traits<char>, std::allocator<char> > const&)':
 
(.text._ZNSt13random_device7_M_initERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE+0x58): 
undefined reference to `getentropy'
 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/bin/ld: 
/build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libc.a(libc_a-arc4random.o): 
in function `_rs_stir':
     (.text._rs_stir+0x8): undefined reference to `getentropy'
     collect2: error: ld returned 1 exit status

This error was fixed with newlib supplying the getentropy function that 
behaves just like the others by calling the low level function _getentropy.


> Some outdated information which should still help you get a
> working environment is in gcc.gnu.org/simtest-howto.html
> (but I suggest build and install sim+binutils separately,
> with just newlib and gcc combined).
> 
>> If I try to link the following minimal C application with the
>> arm-none-eabi target, I get the below warnings.
>>
>> $ cat foo.c
>> int main() {
>>       return 0;
>> }
>>
>>
>> $ .../bin/arm-none-eabi-g++ -mcpu=cortex-m4 -mfloat-abi=soft -o foo.elf
>> foo.c --specs=nosys.specs
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld:
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-closer.o):
>> in function `_close_r':
>> (.text._close_r+0xc): warning: _close is not implemented and will always
>> fail
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld:
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-lseekr.o):
>> in function `_lseek_r':
>> (.text._lseek_r+0x10): warning: _lseek is not implemented and will
>> always fail
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld:
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-readr.o):
>> in function `_read_r':
>> (.text._read_r+0x10): warning: _read is not implemented and will always fail
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/bin/ld:
>> .../bin/../lib/gcc/arm-none-eabi/14.0.0/../../../../arm-none-eabi/lib/thumb/v7e-m/nofp/libc.a(libc_a-writer.o):
>> in function `_write_r':
>> (.text._write_r+0x10): warning: _write is not implemented and will
>> always fail
>>
>>
>> Do you mean that you do not get these stub warnings for the cris-elf target?
> 
> (When *configuring* libstdc++ I'd get those warnings, that's
> why linking tests are disabled for newlib there.)

That sounds strange. This is what I get when configuring libstdc++ for 
arm-none-eabi:

configure:51567: checking for getentropy
configure:51586: .../build-native/gcc-final/./gcc/xgcc -shared-libgcc 
-B.../build-native/gcc-final/./gcc -nostdinc++ 
-L.../build-native/gcc-final/arm-none-eabi/thumb/v7e-m/nofp/libstdc++-v3/src 
-L.../build-native/gcc-final/arm-none-eabi/thumb/v7e-m/nofp/libstdc++-v3/src/.libs 
-L.../build-native/gcc-final/arm-none-eabi/thumb/v7e-m/nofp/libstdc++-v3/libsupc++/.libs 
-B.../install-native/arm-none-eabi/bin/ 
-B.../install-native/arm-none-eabi/lib/ -isystem 
.../install-native/arm-none-eabi/include -isystem 
.../install-native/arm-none-eabi/sys-include  -mthumb -march=armv7e-m 
-mfloat-abi=soft -c -g -O2  conftest.cpp >&5
configure:51586: $? = 0
configure:51618: result: yes

(this is from 
.../build-native/gcc-final/arm-none-eabi/thumb/v7e-m/nofp/libstdc++-v3/config.log).


> But, when called as part of the testsuite, *no*: not with a
> proper syscall library, such as when "-sim3" is passed when
> linking (see the libsyslinux.a stuff in libgloss/cris and
> e.g. LIB_SPEC in gcc/config/cris/cris.h), as happens when
> running with --target_board=cris-sim.

In my case (cross building and cross testing) is that without my patch, 
the failure happens in the testsuite.

> Don't arm-eabi+arm-sim get lots of libstdc++-v3 errors that
> weren't there before your newlib _getentropy patch?

I don't use arm-sim, so I don't know.

> Can you share your full environment such that I can repeat
> your observations?
> 
>> If you do get them, in what way is _getentropy different from for
>> example _close?
> 
> _close is implemented in libsyslinux.a, _getentropy not.
> (No syscall for that so the stub warning is "correct".)

So, in that case, just add the following to the library? It should be 
enough to make your simulator happy and it can still be overridden when 
using newlib built for the cris architecture in real applications.

int _getentropy(void *buf, size_t buflen)
{
   errno = ENOSYS;
   return -1;
}

>> (the above is produced using newlib 7a45daa, GCC d0603dfe9 and binutils
>> 80d2ef0c4)
> 
> Again, all my observations are repeatable by anyone:
> - Install binutils for --target=cris-elf --prefix=<whatever>
> - Ditto the cris simulator in gdb.
> - Configure a "combined" tree with gcc and newlib.
> - Build gcc+newlib using the same --target=cris-elf --prefix=<whatever>
> - 'make check RUNTESTFLAGS=--target_board=cris-sim'.
> - Observe the multitude of libstdc++-v3 tests failing that
> weren't there before your _getentropy patch.
> 
> Please share your full testing environment such that I can
> repeat your observations.

As I wrote above, I can't do that as it's tied to the ST infra.

> I'll get back with a proper solution, one where
> arm-eabi+arm-sim, cris-elf+cris-sim and your own environment
> all work (hopefully, or help you get a proper testing
> environment), before next summer.
> 
> brgds, H-p

Looking forward to your "proper solution" as this appears to work fine 
if you just define the low level implementation for your architecture.

Kind regards,
Torbjörn

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-23 10:58         ` Torbjorn SVENSSON
@ 2023-12-23 16:44           ` Hans-Peter Nilsson
  2023-12-24 10:49             ` Torbjorn SVENSSON
  0 siblings, 1 reply; 8+ messages in thread
From: Hans-Peter Nilsson @ 2023-12-23 16:44 UTC (permalink / raw)
  To: Torbjorn SVENSSON; +Cc: newlib

Half-way through your mail, I thought better put this
together.  It doesn't address all of it though.

> Date: Sat, 23 Dec 2023 11:58:42 +0100
> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>

> I'm using a combination of real evaluation boards and the qemu simulator.

(and a patched qemu, IIUC)

> Unfortunately, I cannot share it as it's tied to how the internal 
> systems are setup at ST.

That's very unfortunate.

> > To wit, if you use something that gives other results than
> > when configuring for --target=arm-eabi and testing with
> > RUNTESTFLAGS=--target_board=arm-sim then your gcc testing
> > isn't matching the expectations in libstdc++-v3 for a newlib
> > target (IOW "not correct").
> 
> I'm pretty sure it didn't match the expectations before

(Well, it did, for cris-elf+cris-sim.  I still have to check
whether something was actually missing for
arm-eabi+arm-sim.)

> either as it 
> picked up the header definition for _getentropy during the configure 
> stage,

Confusion.  There was no _getentropy (or getentropy) before
you added it, so how could it be picked up?  Not ruling out
bugs in libstdc++-v3 configuration here.  Maybe it didn't
recognize your mix as a newlib target?  (It should default
to it from the target tuple, but you can always add a
--with-newlib.)

If you didn't post your complete configure line and
config.log in previous reports and conversions to newlib@
and gcc lists, please do.

> but when linking, there was no such function defined in the 
> newlib provided archives, thus the following error (taken from the 
> commit message of b9e867d088935d9f0bf312e6dbf3e4976850dfd3) was raised 
> during testing of the g++ tool:
> 
>  
> /build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/bin/ld: 
> /build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libstdc++.a(random.o): 
> in function `std::(anonymous namespace)::__libc_getentropy(void*)':
>      (.text._ZNSt12_GLOBAL__N_117__libc_getentropyEPv+0x8): undefined 
> reference to `getentropy'

Right; that means it was *wrongly* identified as present at
configure-time.  The solution is then not to add a stub, but
to fix the wrong present-identification.

> In my case (cross building and cross testing) is that without my patch, 
> the failure happens in the testsuite.

Odd; I'll re-read that conversion.  The lack of _getentropy
(and getentropy) before, *should* not have posed any problem.

> So, in that case, just add the following to the library? It should be 
> enough to make your simulator happy and it can still be overridden when 
> using newlib built for the cris architecture in real applications.
> 
> int _getentropy(void *buf, size_t buflen)
> {
>    errno = ENOSYS;
>    return -1;
> }
> 

That won't fly.  I mentioned that pruning the stub warning
exposes runtime errors, so that'll lead to *regressions for
those libstdc++ tests that expect randomness support to
work, compared to before the _getentropy stub*.  (Before,
libstdc++ fell back to to another randomness
implementation.)

> Looking forward to your "proper solution" as this appears to work fine 
> if you just define the low level implementation for your architecture.

A proper solution is one that matches what's actually
implemented in newlib and libgloss.

It worked before the _getentropy stub, it fails afterwards.

IMHO it's preferable to target off-the-shelf buildable
toolchain combinations rather than homegrown collections; if
the latter work, then fine, but if they break what's already
there, then that collection should be adjusted, not the rest
of newlib+libgloss.

Of course, there may be a working combination for both.

Merry Xmas.

brgds, H-P

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors
  2023-12-23 16:44           ` Hans-Peter Nilsson
@ 2023-12-24 10:49             ` Torbjorn SVENSSON
  0 siblings, 0 replies; 8+ messages in thread
From: Torbjorn SVENSSON @ 2023-12-24 10:49 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: newlib



On 2023-12-23 17:44, Hans-Peter Nilsson wrote:
> Half-way through your mail, I thought better put this
> together.  It doesn't address all of it though.
> 
>> Date: Sat, 23 Dec 2023 11:58:42 +0100
>> From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
> 
>> I'm using a combination of real evaluation boards and the qemu simulator.
> 
> (and a patched qemu, IIUC)

The patch is only to add a machine definition in qemu that has a 
matching address space. It has no impact on the instructions executed.

>> Unfortunately, I cannot share it as it's tied to how the internal
>> systems are setup at ST.
> 
> That's very unfortunate.
> 
>>> To wit, if you use something that gives other results than
>>> when configuring for --target=arm-eabi and testing with
>>> RUNTESTFLAGS=--target_board=arm-sim then your gcc testing
>>> isn't matching the expectations in libstdc++-v3 for a newlib
>>> target (IOW "not correct").
>>
>> I'm pretty sure it didn't match the expectations before
> 
> (Well, it did, for cris-elf+cris-sim.  I still have to check
> whether something was actually missing for
> arm-eabi+arm-sim.)
> 
>> either as it
>> picked up the header definition for _getentropy during the configure
>> stage,
> 
> Confusion.  There was no _getentropy (or getentropy) before
> you added it, so how could it be picked up?  Not ruling out
> bugs in libstdc++-v3 configuration here.  Maybe it didn't
> recognize your mix as a newlib target?  (It should default
> to it from the target tuple, but you can always add a
> --with-newlib.)

Yes there was (pardon the typo in the previous mail as the header only 
included getentropy and not _getentropy): 
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/include/sys/unistd.h;h=5386bd49dc4f700180c94b4a6ab56c8196e6f77b;hb=70ee6b17df9b055a05cdcc4d3fe1813d7b57e2d8#l107

That prototype is picked up by the configure script in libstdc++ here: 
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/configure;h=bef2b840a595b6ec3a2c24eb3efb04f6835f62b7;hb=refs/heads/master#l51573

As I am cross compiling, it will not try to link and thus, it's 
perfectly legal (from the POV of the C compiler) to have the prototype 
but no implementation and thus, it thinks that getentropy is available 
while it's not.

> If you didn't post your complete configure line and
> config.log in previous reports and conversions to newlib@
> and gcc lists, please do.

I have not, but it's more or less the same flags that are used in the 
official arm-none-eabi build from Arm 
(https://developer.arm.com/downloads/-/arm-gnu-toolchain-downloads).

newlib:
.../src/newlib/configure --build=x86_64-linux-gnu 
--host=x86_64-linux-gnu --target=arm-none-eabi 
--prefix=.../install-native 
--infodir=.../install-native/share/doc/gcc-arm-none-eabi/info 
--mandir=.../install-native/share/doc/gcc-arm-none-eabi/man 
--htmldir=.../install-native/share/doc/gcc-arm-none-eabi/html 
--pdfdir=.../install-native/share/doc/gcc-arm-none-eabi/pdf 
--enable-newlib-io-long-long --enable-newlib-io-c99-formats 
--enable-newlib-reent-check-verify --enable-newlib-register-fini 
--enable-newlib-retargetable-locking --disable-newlib-supplied-syscalls 
--disable-nls

GCC:
.../src/gcc/configure --target=arm-none-eabi --prefix=.../install-native 
--libexecdir=.../install-native/lib 
--infodir=.../install-native/share/doc/gcc-arm-none-eabi/info 
--mandir=.../install-native/share/doc/gcc-arm-none-eabi/man 
--htmldir=.../install-native/share/doc/gcc-arm-none-eabi/html 
--pdfdir=.../install-native/share/doc/gcc-arm-none-eabi/pdf 
--enable-languages=c,c++ --enable-plugins --disable-decimal-float 
--disable-libffi --disable-libgomp --disable-libmudflap 
--disable-libquadmath --disable-libssp --disable-libstdcxx-pch 
--disable-nls --disable-shared --disable-threads --disable-tls 
--with-gnu-as --with-gnu-ld --with-newlib --with-headers=yes 
--with-python-dir=share/gcc-arm-none-eabi 
--with-sysroot=.../install-native/arm-none-eabi --build=x86_64-linux-gnu 
--host=x86_64-linux-gnu --with-gmp=.../build-native/host-libs/usr 
--with-mpfr=.../build-native/host-libs/usr 
--with-mpc=.../build-native/host-libs/usr 
--with-isl=.../build-native/host-libs/usr 
'--with-host-libstdcxx=-static-libgcc -Wl,-Bstatic,-lstdc++,-Bdynamic 
-lm' --with-multilib-list=rmprofile,aprofile

> 
>> but when linking, there was no such function defined in the
>> newlib provided archives, thus the following error (taken from the
>> commit message of b9e867d088935d9f0bf312e6dbf3e4976850dfd3) was raised
>> during testing of the g++ tool:
>>
>>   
>> /build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/bin/ld:
>> /build/gcc-13-2709-g9ac9fde961f/bin/../lib/gcc/arm-none-eabi/13.0.0/../../../../arm-none-eabi/lib/thumb/v6-m/nofp/libstdc++.a(random.o):
>> in function `std::(anonymous namespace)::__libc_getentropy(void*)':
>>       (.text._ZNSt12_GLOBAL__N_117__libc_getentropyEPv+0x8): undefined
>> reference to `getentropy'
> 
> Right; that means it was *wrongly* identified as present at
> configure-time.  The solution is then not to add a stub, but
> to fix the wrong present-identification.

Well, arc4random, that is part of newlib, also assumes that getentropy 
exists.
https://sourceware.org/git/?p=newlib-cygwin.git;a=blob;f=newlib/libc/stdlib/arc4random.c;h=09a134c3b9dabec8ae414223f082e9dfdcaaeb3a;hb=refs/heads/main#l101

This part was added by Sebastian Huber back in 2016: 
https://sourceware.org/git/?p=newlib-cygwin.git;a=commit;f=newlib/libc/stdlib/arc4random.c;h=f74cf1350e4633892b6ee6db3664eec38579a709

>> In my case (cross building and cross testing) is that without my patch,
>> the failure happens in the testsuite.
> 
> Odd; I'll re-read that conversion.  The lack of _getentropy
> (and getentropy) before, *should* not have posed any problem.

Well it does. :)
As there are other things in newlib that depend on this symbol, I though 
it was the right move to add the stub for _getentropy (and the wrapping 
getentropy function) as it "completes" the chain for other parts of 
newlib as well as allowing the proper handling in libstdc++.
If you think the correct approach is to rip out getentropy, _getentropy 
and the arc4random stuff from newlib, then I suppose you should supply a 
patch for it.

>> So, in that case, just add the following to the library? It should be
>> enough to make your simulator happy and it can still be overridden when
>> using newlib built for the cris architecture in real applications.
>>
>> int _getentropy(void *buf, size_t buflen)
>> {
>>     errno = ENOSYS;
>>     return -1;
>> }
>>
> 
> That won't fly.  I mentioned that pruning the stub warning
> exposes runtime errors, so that'll lead to *regressions for
> those libstdc++ tests that expect randomness support to
> work, compared to before the _getentropy stub*.  (Before,
> libstdc++ fell back to to another randomness
> implementation.)

No it does not.
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/src/c%2B%2B11/random.cc;h=75989bd33375800d04a5ca5788f99c103c199ea2;hb=refs/heads/master#l468
I interpret this as the program can be compiled to support a certain 
random source and then at runtime, it's verified that said random source 
is available or not.
Do you make a different interpretation?

>> Looking forward to your "proper solution" as this appears to work fine
>> if you just define the low level implementation for your architecture.
> 
> A proper solution is one that matches what's actually
> implemented in newlib and libgloss.
> 
> It worked before the _getentropy stub, it fails afterwards.
> 
> IMHO it's preferable to target off-the-shelf buildable
> toolchain combinations rather than homegrown collections; if
> the latter work, then fine, but if they break what's already
> there, then that collection should be adjusted, not the rest
> of newlib+libgloss.

I see no reason why the set of newlib, libgloss or GCC matters for this.
When doing a cross compile, the header file is all that matters for the 
support to be picked up by libstdc++ configure script. As such, it will 
later fail to link with said library as the implementation is missing.

As I said before, either we need to rip out *everything* that loops back 
to getentropy from newlib or there needs to be something that provides a 
working implementation for your use-case. As you appear to be running 
the cris target on linux, I suggest that you simply wrap it to the 
getentropy call on the host for your simulation.

Keep in mind that I don't really care about what works with a simulator 
or not, what I focus on is if real applications will work or not. My way 
of testing the arm-none-eabi target is as close as I can get to real 
application scenarios as it more or less matches how our customers will 
use the toolchain. This is the important part!
Also, I'm not just testing a particular Cortex-M target, I'm testing all 
of the Cortex-M targets that are available in the STM32 portfolio with 
and without FPU. As I'm verifying that all these combinations are 
correct (according to the dejagnu suite), you see why arm-sim will not 
be enough for me.


Kind regards,
Torbjörn

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-12-24 10:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-06 17:50 [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors Hans-Peter Nilsson
2023-12-06 19:52 ` Torbjorn SVENSSON
2023-12-15  4:24   ` Hans-Peter Nilsson
2023-12-21 17:37     ` Torbjorn SVENSSON
2023-12-21 18:26       ` Hans-Peter Nilsson
2023-12-23 10:58         ` Torbjorn SVENSSON
2023-12-23 16:44           ` Hans-Peter Nilsson
2023-12-24 10:49             ` Torbjorn SVENSSON

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).