Re: [PATCH] uapi: Fixup strace compile error

[Heiko Stübner]

Am Montag, 13. Juni 2022, 03:30:51 CEST schrieb guoren@xxxxxxxxxx:
> From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxxx>
> Reported-by: Eugene Syromiatnikov <esyr@xxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Heiko Stuebner <heiko@xxxxxxxxx>
> Cc: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> ---
>  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?


Heiko