[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJF2gTSotQqmwCMq8Yvg11sZLhfBYONZviJqGBf0eJ9PXKEssg@mail.gmail.com>
Date: Thu, 4 Aug 2022 10:10:06 +0800
From: Guo Ren <guoren@...nel.org>
To: Heiko Stübner <heiko@...ech.de>
Cc: Palmer Dabbelt <palmer@...osinc.com>,
Arnd Bergmann <arnd@...db.de>,
Guenter Roeck <linux@...ck-us.net>,
Palmer Dabbelt <palmer@...belt.com>,
linux-riscv <linux-riscv@...ts.infradead.org>,
linux-arch <linux-arch@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Guo Ren <guoren@...ux.alibaba.com>,
Eugene Syromiatnikov <esyr@...hat.com>,
Christoph Hellwig <hch@....de>
Subject: Re: [PATCH] uapi: Fixup strace compile error
On Mon, Jun 13, 2022 at 9:58 PM Heiko Stübner <heiko@...ech.de> wrote:
>
> Am Montag, 13. Juni 2022, 03:30:51 CEST schrieb guoren@...nel.org:
> > From: Guo Ren <guoren@...ux.alibaba.com>
> >
> > There is no CONFIG_64BIT in userspace, we shouldn't limit it with
> > __BITS_PER_LONG == 32 to break the compatibility. Just export F_*64
> > definitions to userspace permanently.
> >
> > gcc-11 -DHAVE_CONFIG_H -I./linux/x86_64 -I../../../src/linux/x86_64
> > -I./linux/generic -I../../../src/linux/generic -I. -I../../../src
> > -DIN_STRACE=1 -isystem /opt/kernel/include -Wall -Wextra
> > -Wno-missing-field-initializers -Wno-unused-parameter -Wdate-time
> > -Wformat-security -Wimplicit-fallthrough=5 -Winit-self -Wlogical-op
> > -Wmissing-prototypes -Wnested-externs -Wold-style-definition
> > -Wtrampolines -Wundef -Wwrite-strings -Werror -g -O2 -c -o
> > libstrace_a-fetch_bpf_fprog.o `test -f 'fetch_bpf_fprog.c' || echo
> > '../../../src/'`fetch_bpf_fprog.c
> > In file included from ../../../src/defs.h:404,
> > from ../../../src/fcntl.c:12:
> > ../../../src/xlat/fcntlcmds.h:54:7: error: ‘F_GETLK64’ undeclared here
> > (not in a function); did you mean ‘F_GETLK’?
> > 54 | XLAT(F_GETLK64),
> > | ^~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> > 64 | # define XLAT(val) { (unsigned)(val), #val
> > }
> > | ^~~
> > ../../../src/xlat/fcntlcmds.h:57:7: error: ‘F_SETLK64’ undeclared here
> > (not in a function); did you mean ‘F_SETLK’?
> > 57 | XLAT(F_SETLK64),
> > | ^~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> > 64 | # define XLAT(val) { (unsigned)(val), #val
> > }
> > | ^~~
> > ../../../src/xlat/fcntlcmds.h:60:7: error: ‘F_SETLKW64’ undeclared here
> > (not in a function); did you mean ‘F_SETLKW’?
> > 60 | XLAT(F_SETLKW64),
> > | ^~~~~~~~~~
> > ../../../src/xlat.h:64:54: note: in definition of macro ‘XLAT’
> > 64 | # define XLAT(val) { (unsigned)(val), #val
> > }
> > | ^~~
> > make[4]: *** [Makefile:5017: libstrace_a-fcntl.o] Error 1
> >
> > comment by Eugene:
> > Actually, it's quite the opposite: "ifndef" usage made it vailable at all
> > times to the userspace, and this change has actually broken building strace
> > with the latest kernel headers[1][2]. There could be some debate whether
> > having these F_*64 definitions exposed to the user space 64-bit
> > applications, but it seems that were no harm (as they were exposed already
> > for quite some time), and they are useful at least for strace for compat
> > application tracing purposes.
> >
> > Fixes: 306f7cc1e9061 "uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h"
> > Signed-off-by: Guo Ren <guoren@...ux.alibaba.com>
> > Reported-by: Eugene Syromiatnikov <esyr@...hat.com>
> > Cc: Christoph Hellwig <hch@....de>
> > Cc: Arnd Bergmann <arnd@...db.de>
> > Cc: Heiko Stuebner <heiko@...ech.de>
> > Cc: Palmer Dabbelt <palmer@...osinc.com>
> > ---
> > include/uapi/asm-generic/fcntl.h | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
> > index f13d37b60775..cd6bd65ec25d 100644
> > --- a/include/uapi/asm-generic/fcntl.h
> > +++ b/include/uapi/asm-generic/fcntl.h
> > @@ -116,13 +116,11 @@
> > #define F_GETSIG 11 /* for sockets. */
> > #endif
> >
> > -#if __BITS_PER_LONG == 32 || defined(__KERNEL__)
> > #ifndef F_GETLK64
> > #define F_GETLK64 12 /* using 'struct flock64' */
> > #define F_SETLK64 13
> > #define F_SETLKW64 14
> > #endif
> > -#endif /* __BITS_PER_LONG == 32 || defined(__KERNEL__) */
>
> Looks like prviously there were a number of ways these constants
> were ifdef'd - or not. A number of platforms already had no ifdef of
> any sort around them before, so this looks like the sane way to do it.
>
> Though in the original patch the special-mips-variant also gained the
> #if __BITS_PER_LONG == 32 ...
> conditional in arch/mips/include/uapi/asm/fcntl.h .
> So, is it also affected by this issue?
Not sure about mips.
>
>
> Heiko
>
>
--
Best Regards
Guo Ren
Powered by blists - more mailing lists