Re: [PATCH v3 2/5] tools/nolibc: Add statx() and make stat() rely on statx() if necessary

[Willy Tarreau]

 Hi Feiyang,

On Mon, Feb 13, 2023 at 09:06:36AM +0800, Feiyang Chen wrote:
> On Mon, 13 Feb 2023 at 05:12, Willy Tarreau <w@xxxxxx> wrote:
> >
> > Hi Feiyang,
> >
> > On Thu, Feb 09, 2023 at 11:24:13AM +0800, chris.chenfeiyang@xxxxxxxxx wrote:
> > > From: Feiyang Chen <chenfeiyang@xxxxxxxxxxx>
> > >
> > > LoongArch and RISC-V 32-bit only have statx(). ARC, Hexagon, Nios2 and
> > > OpenRISC have statx() and stat64() but not stat() or newstat(). Add
> > > statx() and make stat() rely on statx() if necessary to make them happy.
> > > We may just use statx() for all architectures in the future.
> > >
> > > Signed-off-by: Feiyang Chen <chenfeiyang@xxxxxxxxxxx>
> > > ---
> > >  tools/include/nolibc/sys.h | 56 ++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
> > > index c4818a9c8823..70c30d457952 100644
> > > --- a/tools/include/nolibc/sys.h
> > > +++ b/tools/include/nolibc/sys.h
> > > @@ -20,6 +20,7 @@
> > >  #include <linux/time.h>
> > >  #include <linux/auxvec.h>
> > >  #include <linux/fcntl.h> // for O_* and AT_*
> > > +#include <linux/stat.h>  // for statx()
> >
> > This one causes build warnings on all archs but x86_64:
> >
> >   /f/tc/nolibc/gcc-11.3.0-nolibc/aarch64-linux/bin/aarch64-linux-gcc -Os -fno-ident -fno-asynchronous-unwind-tables  -s -o nolibc-test \
> >     -nostdlib -static -Isysroot/arm64/include nolibc-test.c -lgcc
> >   In file included from sysroot/arm64/include/sys.h:23,
> >                    from sysroot/arm64/include/nolibc.h:99,
> >                    from sysroot/arm64/include/errno.h:26,
> >                    from sysroot/arm64/include/stdio.h:14,
> >                    from nolibc-test.c:15:
> >   sysroot/arm64/include/linux/stat.h:9: warning: "S_IFMT" redefined
> >       9 | #define S_IFMT  00170000
> >         |
> >   In file included from sysroot/arm64/include/nolibc.h:98,
> >                    from sysroot/arm64/include/errno.h:26,
> >                    from sysroot/arm64/include/stdio.h:14,
> >                    from nolibc-test.c:15:
> >   sysroot/arm64/include/types.h:27: note: this is the location of the previous definition
> >
> > This is caused by the definitions for S_IF* and S_IS* in types.h. However
> > if I remove them I'm seeing x86_64 fail on S_IFCHR not defined. The root
> > cause is that the x86_64 toolchain falls back to /usr/include for the
> > include_next <limits.h> that others do not do (probably that when built
> > it thought it was a native compiler instead of a cross-compiler). I'm
> > apparently able to work around this by ifdefing out the definitions but
> > it makes me feel like I'm hiding the dust under the carpet. Instead I'm
> > thinking of reusing Vincent's work who added stdint and the definitions
> > for the various INT*MAX values that are normally found in limits.h and
> > providing our own limits.h so that this issue is globally addressed.
> >
> > I'm going to experiment a little bit about this and will propose something
> > once I'm satisfied with a solution that we can queue for 6.4. Most likely
> > it will involve merging a variant of Vincent's series first, a few changes
> > to have limits.h then your series.
> >
> 
> Hi, Willy,
> 
> OK. Thank you very much!

You're welcome. I finally figured the root cause of the problem. As
mentioned above, the cross-compiler mistakenly includes some glibc
entries (regardless of me defining limits.h), and linux/stat.h sees
glibc defined so it refrains from defining S_I* because this conflict
was apparently already identified in the past. In order to work around
the problem without touching the uapi headers, I preferred to modify
the nolibc one to add a similar test and detect whether linux/stat.h
had already provided them (see patch below). This way it remains simple
to understand and still pretty effective. And now your patchset works
fine with no modification.

I'll send all of this to Paul next week-end once he's back.

Thanks,
Willy

--