Re: [committed 0/2] CRIS: Fix compilation warnings that recent gcc treats as errors

[Hans-Peter Nilsson <hp@axis.com>]

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