Re: [PATCH] tests/tcg/multiarch: Define _LARGEFILE64_SOURCE

[Brian Cain]

On 9/16/2024 11:05 AM, Brian Cain wrote:
> On 9/16/2024 10:47 AM, Alex Bennée wrote:
> > Brian Cain <quic_bcain@quicinc.com> writes:
> >
> > > On 9/16/2024 8:12 AM, Alex Bennée wrote:
> > > > Brian Cain <quic_bcain@quicinc.com> writes:
> > > >
> > > > > On 9/6/2024 9:39 PM, Brian Cain wrote:
> > > > > > With newer clang builds (19.x), there's a warning for implicit 
> > > > > > function
> > > > > > declarations and it rejects linux-test.c.
> > > > > >
> > > > > > glibc/musl's readdir64() declaration in dirent is guarded by
> > > > > > _LARGEFILE64_SOURCE, so we'll define it to fix the warning.
> > > > > >
> > > > > >          BUILD   hexagon-linux-user guest-tests
> > > > > > /local/mnt/workspace/upstream/toolchain_for_hexagon/qemu/tests/tcg/multiarch/linux/linux-test.c:189:14: 
> > > > > > error: call to undeclared function 'readdir64'; ISO C99 and later 
> > > > > > do not support implicit function declarations 
> > > > > > [-Wimplicit-function-declaration]
> > > > > >          189 |         de = readdir64(dir);
> > > > > >              |              ^
> > > > > >
> > > > > > Signed-off-by: Brian Cain <bcain@quicinc.com>
> > > > > > ---
> > > > > >     tests/tcg/multiarch/linux/linux-test.c | 1 +
> > > > > >     1 file changed, 1 insertion(+)
> > > > > >
> > > > > > diff --git a/tests/tcg/multiarch/linux/linux-test.c 
> > > > > > b/tests/tcg/multiarch/linux/linux-test.c
> > > > > > index 64f57cb287..4e0e862ad9 100644
> > > > > > --- a/tests/tcg/multiarch/linux/linux-test.c
> > > > > > +++ b/tests/tcg/multiarch/linux/linux-test.c
> > > > > > @@ -17,6 +17,7 @@
> > > > > >      *  along with this program; if not, see 
> > > > > > <http://www.gnu.org/licenses/>.
> > > > > >      */
> > > > > >     #define _GNU_SOURCE
> > > > > > +#define _LARGEFILE64_SOURCE
> > > > > >     #include <stdarg.h>
> > > > > >     #include <stdlib.h>
> > > > > >     #include <stdio.h>
> > > > > Alex -- what do you think about this one?
> > > > Actually scratch that, this is a 32 compat hack:
> > > >
> > > >     1f442da51e (tests/tcg/multiarch: fix 32bit linux-test on 64bit 
> > > > host)
> > > >
> > > > Is the __USE_LARGEFILE64 symbol in the hexagon headers?
> > > musl does not define/use __USE_LARGEFILE64, no.  If it's well defined
> > > I could examine whether it makes sense to add this feature to musl,
> > > though.  How does __USE_LARGEFILE64 differ from _LARGEFILE64_SOURCE?
> > > Is it more appropriate to define that here?
> > Digging into the GNU source _LARGEFILE* is the correct define, the __USE
> > flags are internal. features.h says:
> >
> >     _LARGEFILE_SOURCE    Some more functions for correct standard I/O.
> >     _LARGEFILE64_SOURCE  Additional functionality from LFS for large 
> > files.
> >
> > although looking at _LARGEFILE64_SOURCE should be defined by
> > _GNU_SOURCE which is already set for linux-test.c
> >
> > According to the musl WHATSNEW:
> >
> >    compatibility:
> >    - make _GNU_SOURCE imply _LARGEFILE64_SOURCE
>
> Yeah, I just noticed that myself. I guess I will look at how it's done 
> and see if I can fix this so it's more general and can include this case.
>
> > So is this a hexagon only thing?
> It's not - I expect it would impact any architecture using musl.

Hmm.  The _GNU_SOURCE guard was deliberately removed in 
25e6fee27f4a293728dd15b659170e7b9c7db9bc (see below).  IMO the relevant 
text is "portable software should be prepared for them not to exist" and 
" the intent is that this be a very short-term measure and that the 
macros be removed entirely in the next release cycle."  They're still 
there, guarded by only _LARGEFILE64_SOURCE.  I will bring up the 
question about what the future plan for this is on the musl list, but I 
also think that the appropriate, portable thing to do is to change the 
test case regardless of musl's plans.  If there were some other 
conformant C library it could implement it the same way.  IIUC the 
relevant specification is here 
https://unix.org/version2/whatsnew/lfs20mar.html


commit 25e6fee27f4a293728dd15b659170e7b9c7db9bc
Author: Rich Felker <dalias@aerifal.cx>
Date:   Tue Sep 27 15:04:05 2022 -0400

    remove LFS64 programming interfaces (macro-only) from _GNU_SOURCE

    these badly pollute the namespace with macros whenever _GNU_SOURCE is
    defined, which is always the case with g++, and especially tends to
    interfere with C++ constructs.

    as our implementation of these was macro-only, their removal cannot
    affect any existing binaries. at the source level, portable software
    should be prepared for them not to exist.

    for now, they are left in place with explicit _LARGEFILE64_SOURCE.
    this provides an easy temporary path for integrators/distributions to
    get packages building again right away if they break while working on
    a proper, upstreamable fix. the intent is that this be a very
    short-term measure and that the macros be removed entirely in the next
    release cycle.