| Message ID | 1687443219-11946-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8386f58f8deda81110283798a387fb53ec21957c |
| Headers | show |
| Series | Unify uapi bitsperlong.h | expand |
Hi Tiezhu and Arnd, On Thu, Jun 22, 2023 at 10:13:38PM +0800, Tiezhu Yang wrote: > Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 > in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are > usable, it is probably fine to unify the definition of __BITS_PER_LONG as > (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h. > > In order to keep safe and avoid regression, only unify uapi bitsperlong.h > for some archs such as arm64, riscv and loongarch which are using newer > toolchains that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. > > Suggested-by: Xi Ruoyao <xry111@xry111.site> > Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.camel@xry111.site/ > Suggested-by: Arnd Bergmann <arnd@arndb.de> > Link: https://lore.kernel.org/linux-arch/a3a4f48a-07d4-4ed9-bc53-5d383428bdd2@app.fastmail.com/ > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > arch/arm64/include/uapi/asm/bitsperlong.h | 24 ---------------------- > arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- > arch/riscv/include/uapi/asm/bitsperlong.h | 14 ------------- > include/uapi/asm-generic/bitsperlong.h | 13 +++++++++++- > tools/arch/arm64/include/uapi/asm/bitsperlong.h | 24 ---------------------- > .../arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- > tools/arch/riscv/include/uapi/asm/bitsperlong.h | 14 ------------- > tools/include/uapi/asm-generic/bitsperlong.h | 14 ++++++++++++- > tools/include/uapi/asm/bitsperlong.h | 6 ------ > 9 files changed, 25 insertions(+), 102 deletions(-) > delete mode 100644 arch/arm64/include/uapi/asm/bitsperlong.h > delete mode 100644 arch/loongarch/include/uapi/asm/bitsperlong.h > delete mode 100644 arch/riscv/include/uapi/asm/bitsperlong.h > delete mode 100644 tools/arch/arm64/include/uapi/asm/bitsperlong.h > delete mode 100644 tools/arch/loongarch/include/uapi/asm/bitsperlong.h > delete mode 100644 tools/arch/riscv/include/uapi/asm/bitsperlong.h I think this change has backwards compatibility concerns, as it breaks building certain host tools on the stable releases (at least 6.4 and 6.1, as that is where I noticed this). I see the following error on my aarch64 system: $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- mrproper defconfig prepare In file included from /usr/include/asm/bitsperlong.h:1, from /usr/include/asm-generic/int-ll64.h:12, from /usr/include/asm-generic/types.h:7, from /usr/include/asm/types.h:1, from tools/include/linux/types.h:13, from tools/arch/x86/include/asm/orc_types.h:9, from scripts/sorttable.h:96, from scripts/sorttable.c:201: tools/include/asm-generic/bitsperlong.h:14:2: error: #error Inconsistent word size. Check asm/bitsperlong.h 14 | #error Inconsistent word size. Check asm/bitsperlong.h | ^~~~~ A reverse bisect of 6.4 to 6.5-rc1 points to this patch. This Fedora rawhide container has kernel-headers 6.5.0-0.rc2.git0.1.fc39 and the error disappears when I downgrade to 6.4.0-0.rc7.git0.1.fc39. I have not done a ton of triage/debugging so far, as I am currently hunting down other regressions, but I figured I would get an initial report out, since I noticed it when validating LLVM from the new release/17.x branch. If there is any additional information I can provide or patches I can test, I am more than happy to do so. Cheers, Nathan
On Thu, Jul 27, 2023, at 23:36, Nathan Chancellor wrote: > Hi Tiezhu and Arnd, > > On Thu, Jun 22, 2023 at 10:13:38PM +0800, Tiezhu Yang wrote: >> Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 >> in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are >> usable, it is probably fine to unify the definition of __BITS_PER_LONG as >> (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h. >> >> In order to keep safe and avoid regression, only unify uapi bitsperlong.h >> for some archs such as arm64, riscv and loongarch which are using newer >> toolchains that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. >> >> Suggested-by: Xi Ruoyao <xry111@xry111.site> >> Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.camel@xry111.site/ >> Suggested-by: Arnd Bergmann <arnd@arndb.de> >> Link: https://lore.kernel.org/linux-arch/a3a4f48a-07d4-4ed9-bc53-5d383428bdd2@app.fastmail.com/ >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> >> --- > > I think this change has backwards compatibility concerns, as it breaks > building certain host tools on the stable releases (at least 6.4 and > 6.1, as that is where I noticed this). I see the following error on my > aarch64 system: > > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- > mrproper defconfig prepare > In file included from /usr/include/asm/bitsperlong.h:1, > from /usr/include/asm-generic/int-ll64.h:12, > from /usr/include/asm-generic/types.h:7, > from /usr/include/asm/types.h:1, > from tools/include/linux/types.h:13, > from tools/arch/x86/include/asm/orc_types.h:9, > from scripts/sorttable.h:96, > from scripts/sorttable.c:201: > tools/include/asm-generic/bitsperlong.h:14:2: error: #error > Inconsistent word size. Check asm/bitsperlong.h > 14 | #error Inconsistent word size. Check asm/bitsperlong.h > | ^~~~~ Thanks for the report. I'm still struggling to figure out what exactly is going wrong here, and if this is a bug in the patch I merged, or an existing bug that now causes a build failure instead of some other problem. > A reverse bisect of 6.4 to 6.5-rc1 points to this patch. This Fedora > rawhide container has kernel-headers 6.5.0-0.rc2.git0.1.fc39 and the > error disappears when I downgrade to 6.4.0-0.rc7.git0.1.fc39. I have not > done a ton of triage/debugging so far, as I am currently hunting down > other regressions, but I figured I would get an initial report out, > since I noticed it when validating LLVM from the new release/17.x > branch. If there is any additional information I can provide or patches > I can test, I am more than happy to do so. One thing I think is going wrong here is that scripts/sorttable.c is meant to run on the host (arm64) but includes the target (x86) orc_Types.h header and the kernel-internal asm/bitsperlong.h instead of the uapi version. The sanity check in the kernel-side header is intended to cross-check the CONFIG_64BIT value against the __BITS_PER_LONG constant from the header. My first guess would be that this only worked by accident if the headers defaulted to "#define __BITS_PER_LONG 32" in and #undef CONFIG_64BIT" when include/generated/autoconf.h, but now the __BITS_PER_LONG value is actually correct. Arnd
On Fri, Jul 28, 2023 at 01:00:30PM +0200, Arnd Bergmann wrote: > On Thu, Jul 27, 2023, at 23:36, Nathan Chancellor wrote: > > Hi Tiezhu and Arnd, > > > > On Thu, Jun 22, 2023 at 10:13:38PM +0800, Tiezhu Yang wrote: > >> Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 > >> in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are > >> usable, it is probably fine to unify the definition of __BITS_PER_LONG as > >> (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h. > >> > >> In order to keep safe and avoid regression, only unify uapi bitsperlong.h > >> for some archs such as arm64, riscv and loongarch which are using newer > >> toolchains that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. > >> > >> Suggested-by: Xi Ruoyao <xry111@xry111.site> > >> Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.camel@xry111.site/ > >> Suggested-by: Arnd Bergmann <arnd@arndb.de> > >> Link: https://lore.kernel.org/linux-arch/a3a4f48a-07d4-4ed9-bc53-5d383428bdd2@app.fastmail.com/ > >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > >> --- > > > > > I think this change has backwards compatibility concerns, as it breaks > > building certain host tools on the stable releases (at least 6.4 and > > 6.1, as that is where I noticed this). I see the following error on my > > aarch64 system: > > > > $ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- > > mrproper defconfig prepare > > In file included from /usr/include/asm/bitsperlong.h:1, > > from /usr/include/asm-generic/int-ll64.h:12, > > from /usr/include/asm-generic/types.h:7, > > from /usr/include/asm/types.h:1, > > from tools/include/linux/types.h:13, > > from tools/arch/x86/include/asm/orc_types.h:9, > > from scripts/sorttable.h:96, > > from scripts/sorttable.c:201: > > tools/include/asm-generic/bitsperlong.h:14:2: error: #error > > Inconsistent word size. Check asm/bitsperlong.h > > 14 | #error Inconsistent word size. Check asm/bitsperlong.h > > | ^~~~~ > > Thanks for the report. I'm still struggling to figure out what > exactly is going wrong here, and if this is a bug in the patch > I merged, or an existing bug that now causes a build failure instead > of some other problem. Totally understandable, I was really confused at first too. > > A reverse bisect of 6.4 to 6.5-rc1 points to this patch. This Fedora > > rawhide container has kernel-headers 6.5.0-0.rc2.git0.1.fc39 and the > > error disappears when I downgrade to 6.4.0-0.rc7.git0.1.fc39. I have not > > done a ton of triage/debugging so far, as I am currently hunting down > > other regressions, but I figured I would get an initial report out, > > since I noticed it when validating LLVM from the new release/17.x > > branch. If there is any additional information I can provide or patches > > I can test, I am more than happy to do so. > > One thing I think is going wrong here is that scripts/sorttable.c is > meant to run on the host (arm64) but includes the target (x86) > orc_Types.h header and the kernel-internal asm/bitsperlong.h instead Right. I will note sorttable is not the only utility that has this issue, I see the same problem coming from several files in tools/lib/subcmd when building several different architectures and arch/x86/entry/vdso/vdso2c.c at the very least. > of the uapi version. The sanity check in the kernel-side header > is intended to cross-check the CONFIG_64BIT value against the > __BITS_PER_LONG constant from the header. > > My first guess would be that this only worked by accident if the headers > defaulted to "#define __BITS_PER_LONG 32" in and #undef CONFIG_64BIT" > when include/generated/autoconf.h, but now the __BITS_PER_LONG value > is actually correct. That seems like a reasonable theory. I am still busy looking into other things today but I can try to double back to this on Monday if you don't make any progress. Cheers, Nathan
On Fri, Jul 28, 2023, at 19:31, Nathan Chancellor wrote: > On Fri, Jul 28, 2023 at 01:00:30PM +0200, Arnd Bergmann wrote: >> >> of the uapi version. The sanity check in the kernel-side header >> is intended to cross-check the CONFIG_64BIT value against the >> __BITS_PER_LONG constant from the header. >> >> My first guess would be that this only worked by accident if the headers >> defaulted to "#define __BITS_PER_LONG 32" in and #undef CONFIG_64BIT" >> when include/generated/autoconf.h, but now the __BITS_PER_LONG value >> is actually correct. > > That seems like a reasonable theory. I am still busy looking into other > things today but I can try to double back to this on Monday if you don't > make any progress. I tried reproducing this today on arm64 Debian with linux-6.5-rc3 and clang-14.0.6 but I don't see the problem here. With 'make V=1' I see command for building scripts/sorttable is clang -Wp,-MMD,scripts/.sorttable.d -Wall -Wmissing-prototypes \ -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu11 \ -I./tools/include -I./tools/arch/x86/include -DUNWINDER_ORC_ENABLED \ -o scripts/sorttable scripts/sorttable.c -lpthread which does create an arm64 executable but includes the x86 headers, which is clearly a bug by itself, it just doesn't trigger the problem for me. I also noticed that your command line includes CROSS_COMPILE=x86_64-linux- rather than CROSS_COMPILE=x86_64-linux-gnu-, and I think we've had problems with that in the past, when "clang --target=x86_64-linux" fails to find the glibc system headers. Arnd
On Fri, Jul 28, 2023 at 10:56:38PM +0200, Arnd Bergmann wrote: > On Fri, Jul 28, 2023, at 19:31, Nathan Chancellor wrote: > > On Fri, Jul 28, 2023 at 01:00:30PM +0200, Arnd Bergmann wrote: > >> > >> of the uapi version. The sanity check in the kernel-side header > >> is intended to cross-check the CONFIG_64BIT value against the > >> __BITS_PER_LONG constant from the header. > >> > >> My first guess would be that this only worked by accident if the headers > >> defaulted to "#define __BITS_PER_LONG 32" in and #undef CONFIG_64BIT" > >> when include/generated/autoconf.h, but now the __BITS_PER_LONG value > >> is actually correct. > > > > That seems like a reasonable theory. I am still busy looking into other > > things today but I can try to double back to this on Monday if you don't > > make any progress. > > I tried reproducing this today on arm64 Debian with linux-6.5-rc3 > and clang-14.0.6 but I don't see the problem here. With 'make V=1' > I see command for building scripts/sorttable is > > clang -Wp,-MMD,scripts/.sorttable.d -Wall -Wmissing-prototypes \ > -Wstrict-prototypes -O2 -fomit-frame-pointer -std=gnu11 \ > -I./tools/include -I./tools/arch/x86/include -DUNWINDER_ORC_ENABLED \ > -o scripts/sorttable scripts/sorttable.c -lpthread > > which does create an arm64 executable but includes the x86 headers, > which is clearly a bug by itself, it just doesn't trigger the problem > for me. I could not initially reproduce this on Debian either but I figured out why that might be: the default include paths on Debian look different from Fedora so just doing 'headers_install' into /usr will not reproduce this. If I add '-H' to that GCC command, Debian shows (I highlighted the key difference): . /linux-stable/scripts/sorttable.h .. /linux-stable/tools/arch/x86/include/asm/orc_types.h ... /linux-stable/tools/include/linux/types.h .... /usr/lib/gcc/aarch64-linux-gnu/12/include/stdbool.h .... /usr/lib/gcc/aarch64-linux-gnu/12/include/stddef.h .... /usr/include/aarch64-linux-gnu/asm/types.h ..... /usr/include/asm-generic/types.h ...... /usr/include/asm-generic/int-ll64.h ....... /usr/include/aarch64-linux-gnu/asm/bitsperlong.h <- ........ /linux-stable/tools/include/asm-generic/bitsperlong.h ......... /linux-stable/tools/include/uapi/asm-generic/bitsperlong.h Whereas Fedora shows: . /linux-stable/scripts/sorttable.h .. /linux-stable/tools/arch/x86/include/asm/orc_types.h ... /linux-stable/tools/include/linux/types.h .... /usr/lib/gcc/aarch64-redhat-linux/13/include/stdbool.h .... /usr/lib/gcc/aarch64-redhat-linux/13/include/stddef.h .... /usr/include/asm/types.h ..... /usr/include/asm-generic/types.h ...... /usr/include/asm-generic/int-ll64.h ....... /usr/include/asm/bitsperlong.h <- ........ /linux-stable/tools/include/asm-generic/bitsperlong.h ......... /linux-stable/tools/include/uapi/asm-generic/bitsperlong.h Running 'gcc -fsyntax-only -v -x c /dev/null' shows: Debian: #include <...> search starts here: /usr/lib/gcc/aarch64-linux-gnu/12/include /usr/local/include /usr/include/aarch64-linux-gnu /usr/include End of search list. Fedora: #include <...> search starts here: /usr/lib/gcc/aarch64-redhat-linux/13/include /usr/local/include /usr/include End of search list. It looks like Debian installs the architecture asm files into an architecture specific subdirectory, which headers_install does not know about, so the new "problematic" bitsperlong.h file gets installed to the default location but the older one actually gets used because it has higher priority in the include search path. https://salsa.debian.org/kernel-team/linux/-/blob/36b9562acea404ecdc2911aeb2c4539402f441a3/debian/rules.real#L334-336 If I install/manipulate the headers as Debian does, I can reproduce this issue in a fresh Debian container. # make -C /linux -j$(nproc) INSTALL_HDR_PATH=/usr O=/build headers_install # rm -fr /usr/include/aarch64-linux-gnu/asm # mv -v /usr/include/asm /usr/include/aarch64-linux-gnu # make -C /linux-stable -j$(nproc) ARCH=x86_64 CROSS_COMPILE=x86_64-linux-gnu- O=/build mrproper defconfig prepare ... DESCEND objtool In file included from /usr/include/aarch64-linux-gnu/asm/bitsperlong.h:1, from /usr/include/asm-generic/int-ll64.h:12, from /usr/include/asm-generic/types.h:7, from /usr/include/aarch64-linux-gnu/asm/types.h:1, from /linux-stable/tools/include/linux/types.h:13, from /linux-stable/tools/arch/x86/include/asm/orc_types.h:9, from /linux-stable/scripts/sorttable.h:96, from /linux-stable/scripts/sorttable.c:201: /linux-stable/tools/include/asm-generic/bitsperlong.h:14:2: error: #error Inconsistent word size. Check asm/bitsperlong.h 14 | #error Inconsistent word size. Check asm/bitsperlong.h | ^~~~~ make[3]: *** [/linux-stable/scripts/Makefile.host:114: scripts/sorttable] Error 1 ... > I also noticed that your command line includes CROSS_COMPILE=x86_64-linux- > rather than CROSS_COMPILE=x86_64-linux-gnu- Right, as I was reproducing this with your kernel.org GCC for CROSS_COMPILE and Fedora's GCC for HOSTCC, since I wanted to make sure this was not some issue with clang (which it does not appear to be). Cheers, Nathan
On Sat, Jul 29, 2023, at 01:44, Nathan Chancellor wrote: > On Fri, Jul 28, 2023 at 10:56:38PM +0200, Arnd Bergmann wrote: > DESCEND objtool > In file included from > /usr/include/aarch64-linux-gnu/asm/bitsperlong.h:1, > from /usr/include/asm-generic/int-ll64.h:12, > from /usr/include/asm-generic/types.h:7, > from /usr/include/aarch64-linux-gnu/asm/types.h:1, > from /linux-stable/tools/include/linux/types.h:13, > from > /linux-stable/tools/arch/x86/include/asm/orc_types.h:9, > from /linux-stable/scripts/sorttable.h:96, > from /linux-stable/scripts/sorttable.c:201: > /linux-stable/tools/include/asm-generic/bitsperlong.h:14:2: error: > #error Inconsistent word size. Check asm/bitsperlong.h > 14 | #error Inconsistent word size. Check asm/bitsperlong.h > | ^~~~~ > make[3]: *** [/linux-stable/scripts/Makefile.host:114: > scripts/sorttable] Error 1 > ... > >> I also noticed that your command line includes CROSS_COMPILE=x86_64-linux- >> rather than CROSS_COMPILE=x86_64-linux-gnu- > > Right, as I was reproducing this with your kernel.org GCC for > CROSS_COMPILE and Fedora's GCC for HOSTCC, since I wanted to make sure > this was not some issue with clang (which it does not appear to be). Ok, it's beginning to make more sense to me now. I see that the tools/arch/x86/include/asm/orc_types.h comes from the x86_64 target build and is intentional, as sorttable.c needs to access the ORC information. Here the Makefile does ifdef CONFIG_UNWINDER_ORC ifeq ($(ARCH),x86_64) ARCH := x86 endif HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED endif in order to get the ORC definitions from asm/orc_types.h, but then it looks like it also gets the uapi/asm/bitsperlong.h header from there which contains #if defined(__x86_64__) && !defined(__ILP32__) # define __BITS_PER_LONG 64 #else # define __BITS_PER_LONG 32 #endif and this would set __BITS_PER_LONG to 32 on arm64. However, I don't see this actually being included on my machine. Can you dump the sorttable.c preprocessor output with your setup, using -fdirectives-only, so we can see which of the two (__BITS_PER_LONG or BITS_PER_LONG) is actually wrong and triggers the sanity check? What I see on my machine is that both definitions come from the local tools/include/ headers, not from the installed system headers, so I'm still doing something wrong in my installation: ./tools/include/uapi/asm-generic/bitsperlong.h #define __BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) ./tools/include/asm-generic/bitsperlong.h #define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) Neither of these files actually contains the sanity check in linux-6.5-rc3, and comparing these is clearly nonsensical, as they are defined the same way (rather than checking CONFIG_64BIT), but also I don't see why there is both a uapi/ version and a non-uapi version in what is meant to be a userspace header. Arnd
On Sat, Jul 29, 2023 at 09:59:23AM +0200, Arnd Bergmann wrote: > On Sat, Jul 29, 2023, at 01:44, Nathan Chancellor wrote: > > On Fri, Jul 28, 2023 at 10:56:38PM +0200, Arnd Bergmann wrote: > > > DESCEND objtool > > In file included from > > /usr/include/aarch64-linux-gnu/asm/bitsperlong.h:1, > > from /usr/include/asm-generic/int-ll64.h:12, > > from /usr/include/asm-generic/types.h:7, > > from /usr/include/aarch64-linux-gnu/asm/types.h:1, > > from /linux-stable/tools/include/linux/types.h:13, > > from > > /linux-stable/tools/arch/x86/include/asm/orc_types.h:9, > > from /linux-stable/scripts/sorttable.h:96, > > from /linux-stable/scripts/sorttable.c:201: > > /linux-stable/tools/include/asm-generic/bitsperlong.h:14:2: error: > > #error Inconsistent word size. Check asm/bitsperlong.h > > 14 | #error Inconsistent word size. Check asm/bitsperlong.h > > | ^~~~~ > > make[3]: *** [/linux-stable/scripts/Makefile.host:114: > > scripts/sorttable] Error 1 > > ... > > > >> I also noticed that your command line includes CROSS_COMPILE=x86_64-linux- > >> rather than CROSS_COMPILE=x86_64-linux-gnu- > > > > Right, as I was reproducing this with your kernel.org GCC for > > CROSS_COMPILE and Fedora's GCC for HOSTCC, since I wanted to make sure > > this was not some issue with clang (which it does not appear to be). > > Ok, it's beginning to make more sense to me now. I see > that the tools/arch/x86/include/asm/orc_types.h comes from > the x86_64 target build and is intentional, as sorttable.c > needs to access the ORC information. Here the Makefile does > > ifdef CONFIG_UNWINDER_ORC > ifeq ($(ARCH),x86_64) > ARCH := x86 > endif > HOSTCFLAGS_sorttable.o += -I$(srctree)/tools/arch/x86/include > HOSTCFLAGS_sorttable.o += -DUNWINDER_ORC_ENABLED > endif > > in order to get the ORC definitions from asm/orc_types.h, but > then it looks like it also gets the uapi/asm/bitsperlong.h > header from there which contains > > #if defined(__x86_64__) && !defined(__ILP32__) > # define __BITS_PER_LONG 64 > #else > # define __BITS_PER_LONG 32 > #endif > > and this would set __BITS_PER_LONG to 32 on arm64. > > However, I don't see this actually being included on my > machine. Can you dump the sorttable.c preprocessor output > with your setup, using -fdirectives-only, so we can see > which of the two (__BITS_PER_LONG or BITS_PER_LONG) is > actually wrong and triggers the sanity check? Sure thing, this is the output of: $ gcc -I/linux-stable/tools/include -I/linux-stable/tools/arch/x86/include -DUNWINDER_ORC_ENABLED -I ./scripts -E -fdirectives-only /linux-stable/scripts/sorttable.c https://gist.github.com/nathanchance/d2c3e58230930317dc84aff80fef38bf > What I see on my machine is that both definitions come > from the local tools/include/ headers, not from the > installed system headers, so I'm still doing something > wrong in my installation: Just to make sure, you have the 6.5-rc1+ headers installed and you are attempting to build the host tools from an earlier Linux release than 6.5-rc1? I don't see a problem with building these host programs on mainline/6.5, I see this issue when building them in older stable releases (my reproduction so far has been on 6.4 but I see it when building all currently supported long term stable releases) when I have the 6.5-rc1+ headers installed. > ./tools/include/uapi/asm-generic/bitsperlong.h > #define __BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) Because this is the mainline version, whereas the stable version is: #ifndef _UAPI__ASM_GENERIC_BITS_PER_LONG #define _UAPI__ASM_GENERIC_BITS_PER_LONG /* * There seems to be no way of detecting this automatically from user * space, so 64 bit architectures should override this in their * bitsperlong.h. In particular, an architecture that supports * both 32 and 64 bit user space must not rely on CONFIG_64BIT * to decide it, but rather check a compiler provided macro. */ #ifndef __BITS_PER_LONG #define __BITS_PER_LONG 32 #endif #endif /* _UAPI__ASM_GENERIC_BITS_PER_LONG */ which seems to be where the mismatch is coming from? > ./tools/include/asm-generic/bitsperlong.h > #define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) > > Neither of these files actually contains the sanity > check in linux-6.5-rc3, and comparing these is clearly > nonsensical, as they are defined the same way (rather > than checking CONFIG_64BIT), but also I don't see why > there is both a uapi/ version and a non-uapi version > in what is meant to be a userspace header. May be worth looping in the tools/ folks, since that whole directory is rather special IMO... Cheers, Nathan
On Sat, Jul 29, 2023, at 19:46, Nathan Chancellor wrote: > On Sat, Jul 29, 2023 at 09:59:23AM +0200, Arnd Bergmann wrote: >> On Sat, Jul 29, 2023, at 01:44, Nathan Chancellor wrote: >> >> in order to get the ORC definitions from asm/orc_types.h, but >> then it looks like it also gets the uapi/asm/bitsperlong.h >> header from there which contains >> >> #if defined(__x86_64__) && !defined(__ILP32__) >> # define __BITS_PER_LONG 64 >> #else >> # define __BITS_PER_LONG 32 >> #endif >> >> and this would set __BITS_PER_LONG to 32 on arm64. >> >> However, I don't see this actually being included on my >> machine. Can you dump the sorttable.c preprocessor output >> with your setup, using -fdirectives-only, so we can see >> which of the two (__BITS_PER_LONG or BITS_PER_LONG) is >> actually wrong and triggers the sanity check? > > Sure thing, this is the output of: > > $ gcc -I/linux-stable/tools/include > -I/linux-stable/tools/arch/x86/include -DUNWINDER_ORC_ENABLED -I > ./scripts -E -fdirectives-only /linux-stable/scripts/sorttable.c > > https://gist.github.com/nathanchance/d2c3e58230930317dc84aff80fef38bf Ok, so what we get is that the system-wide /usr/include/aarch64-linux-gnu/asm/bitsperlong.h includes the source tree file tools/include/asm-generic/bitsperlong.h which in the old kernels only has the "32" fallback value. >> What I see on my machine is that both definitions come >> from the local tools/include/ headers, not from the >> installed system headers, so I'm still doing something >> wrong in my installation: > > Just to make sure, you have the 6.5-rc1+ headers installed and you are > attempting to build the host tools from an earlier Linux release than > 6.5-rc1? I don't see a problem with building these host programs on > mainline/6.5, I see this issue when building them in older stable > releases (my reproduction so far has been on 6.4 but I see it when > building all currently supported long term stable releases) when I have > the 6.5-rc1+ headers installed. Ok, I see. I missed that part of your description earlier. > > which seems to be where the mismatch is coming from? Right, exactly. >> ./tools/include/asm-generic/bitsperlong.h >> #define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) >> >> Neither of these files actually contains the sanity >> check in linux-6.5-rc3, and comparing these is clearly >> nonsensical, as they are defined the same way (rather >> than checking CONFIG_64BIT), but also I don't see why >> there is both a uapi/ version and a non-uapi version >> in what is meant to be a userspace header. > > May be worth looping in the tools/ folks, since that whole directory is > rather special IMO... I think the good news is that this only happens because the tools/ directory contains a copy of the kernel headers including that sanity check, and other user space won't suffer from it because they don't contain copies of kernel internal headers. Reverting the change might still end up being the easiest way out regardless, but it does seem like we should be able to address this in the tools directory by making sure it doesn't mix the installed headers with the local ones. Part of the problem I think is that the installed /usr/include/asm-generic/int-ll64.h includes /usr/include/aarch64-linux-gnu/asm/bitsperlong.h, so both of them are the uapi headers, but this one has an "include <asm-generic/bitsperlong.h>" that expects the uapi version but instead gets the kernel version from the tools directory. We could override this by adding a working tools/include/asm-generic/bitsperlong.h header, but that has to be backported to the stable kernels then. Arnd
On Sat, Jul 29, 2023 at 11:12:26PM +0200, Arnd Bergmann wrote: > On Sat, Jul 29, 2023, at 19:46, Nathan Chancellor wrote: > > On Sat, Jul 29, 2023 at 09:59:23AM +0200, Arnd Bergmann wrote: > >> On Sat, Jul 29, 2023, at 01:44, Nathan Chancellor wrote: > > >> > >> in order to get the ORC definitions from asm/orc_types.h, but > >> then it looks like it also gets the uapi/asm/bitsperlong.h > >> header from there which contains > >> > >> #if defined(__x86_64__) && !defined(__ILP32__) > >> # define __BITS_PER_LONG 64 > >> #else > >> # define __BITS_PER_LONG 32 > >> #endif > >> > >> and this would set __BITS_PER_LONG to 32 on arm64. > >> > >> However, I don't see this actually being included on my > >> machine. Can you dump the sorttable.c preprocessor output > >> with your setup, using -fdirectives-only, so we can see > >> which of the two (__BITS_PER_LONG or BITS_PER_LONG) is > >> actually wrong and triggers the sanity check? > > > > Sure thing, this is the output of: > > > > $ gcc -I/linux-stable/tools/include > > -I/linux-stable/tools/arch/x86/include -DUNWINDER_ORC_ENABLED -I > > ./scripts -E -fdirectives-only /linux-stable/scripts/sorttable.c > > > > https://gist.github.com/nathanchance/d2c3e58230930317dc84aff80fef38bf > > Ok, so what we get is that the system-wide > /usr/include/aarch64-linux-gnu/asm/bitsperlong.h > > includes the source tree file > tools/include/asm-generic/bitsperlong.h > > which in the old kernels only has the "32" fallback value. Ah, makes perfect sense. > >> What I see on my machine is that both definitions come > >> from the local tools/include/ headers, not from the > >> installed system headers, so I'm still doing something > >> wrong in my installation: > > > > Just to make sure, you have the 6.5-rc1+ headers installed and you are > > attempting to build the host tools from an earlier Linux release than > > 6.5-rc1? I don't see a problem with building these host programs on > > mainline/6.5, I see this issue when building them in older stable > > releases (my reproduction so far has been on 6.4 but I see it when > > building all currently supported long term stable releases) when I have > > the 6.5-rc1+ headers installed. > > Ok, I see. I missed that part of your description earlier. > > > > > which seems to be where the mismatch is coming from? > > Right, exactly. > > >> ./tools/include/asm-generic/bitsperlong.h > >> #define BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) > >> > >> Neither of these files actually contains the sanity > >> check in linux-6.5-rc3, and comparing these is clearly > >> nonsensical, as they are defined the same way (rather > >> than checking CONFIG_64BIT), but also I don't see why > >> there is both a uapi/ version and a non-uapi version > >> in what is meant to be a userspace header. > > > > May be worth looping in the tools/ folks, since that whole directory is > > rather special IMO... > > I think the good news is that this only happens because > the tools/ directory contains a copy of the kernel headers > including that sanity check, and other user space won't > suffer from it because they don't contain copies of kernel > internal headers. Yeah, I think you are correct. > Reverting the change might still end up being the easiest way > out regardless, but it does seem like we should be able > to address this in the tools directory by making sure it doesn't > mix the installed headers with the local ones. Agreed, although you do make a good point below that we would need the fix in the stable trees, which adds additional complexity when it comes to things like bisecting. It is already hard enough with all the various clang behavior changes we have had to adapt to over the years... > Part of the problem I think is that the installed > /usr/include/asm-generic/int-ll64.h includes > /usr/include/aarch64-linux-gnu/asm/bitsperlong.h, so both > of them are the uapi headers, but this one has an > "include <asm-generic/bitsperlong.h>" that expects the > uapi version but instead gets the kernel version from > the tools directory. We could override this by adding > a working tools/include/asm-generic/bitsperlong.h header, > but that has to be backported to the stable kernels then. but this does not sound like it would be the end of the world, I do not have to bisect too often and even if I have to, using a userspace without these headers is generally possible. Cheers, Nathan
diff --git a/arch/arm64/include/uapi/asm/bitsperlong.h b/arch/arm64/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 485d60be..0000000 --- a/arch/arm64/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,24 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -/* - * Copyright (C) 2012 ARM Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - */ -#ifndef __ASM_BITSPERLONG_H -#define __ASM_BITSPERLONG_H - -#define __BITS_PER_LONG 64 - -#include <asm-generic/bitsperlong.h> - -#endif /* __ASM_BITSPERLONG_H */ diff --git a/arch/loongarch/include/uapi/asm/bitsperlong.h b/arch/loongarch/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 00b4ba1..0000000 --- a/arch/loongarch/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,9 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -#ifndef __ASM_LOONGARCH_BITSPERLONG_H -#define __ASM_LOONGARCH_BITSPERLONG_H - -#define __BITS_PER_LONG (__SIZEOF_LONG__ * 8) - -#include <asm-generic/bitsperlong.h> - -#endif /* __ASM_LOONGARCH_BITSPERLONG_H */ diff --git a/arch/riscv/include/uapi/asm/bitsperlong.h b/arch/riscv/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 7d0b32e..0000000 --- a/arch/riscv/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */ -/* - * Copyright (C) 2012 ARM Ltd. - * Copyright (C) 2015 Regents of the University of California - */ - -#ifndef _UAPI_ASM_RISCV_BITSPERLONG_H -#define _UAPI_ASM_RISCV_BITSPERLONG_H - -#define __BITS_PER_LONG (__SIZEOF_POINTER__ * 8) - -#include <asm-generic/bitsperlong.h> - -#endif /* _UAPI_ASM_RISCV_BITSPERLONG_H */ diff --git a/include/uapi/asm-generic/bitsperlong.h b/include/uapi/asm-generic/bitsperlong.h index 693d9a4..352cb81 100644 --- a/include/uapi/asm-generic/bitsperlong.h +++ b/include/uapi/asm-generic/bitsperlong.h @@ -2,6 +2,17 @@ #ifndef _UAPI__ASM_GENERIC_BITS_PER_LONG #define _UAPI__ASM_GENERIC_BITS_PER_LONG +#ifndef __BITS_PER_LONG +/* + * In order to keep safe and avoid regression, only unify uapi + * bitsperlong.h for some archs which are using newer toolchains + * that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. + * See the following link for more info: + * https://lore.kernel.org/linux-arch/b9624545-2c80-49a1-ac3c-39264a591f7b@app.fastmail.com/ + */ +#if defined(__CHAR_BIT__) && defined(__SIZEOF_LONG__) +#define __BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) +#else /* * There seems to be no way of detecting this automatically from user * space, so 64 bit architectures should override this in their @@ -9,8 +20,8 @@ * both 32 and 64 bit user space must not rely on CONFIG_64BIT * to decide it, but rather check a compiler provided macro. */ -#ifndef __BITS_PER_LONG #define __BITS_PER_LONG 32 #endif +#endif #endif /* _UAPI__ASM_GENERIC_BITS_PER_LONG */ diff --git a/tools/arch/arm64/include/uapi/asm/bitsperlong.h b/tools/arch/arm64/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 485d60be..0000000 --- a/tools/arch/arm64/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,24 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -/* - * Copyright (C) 2012 ARM Ltd. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see <http://www.gnu.org/licenses/>. - */ -#ifndef __ASM_BITSPERLONG_H -#define __ASM_BITSPERLONG_H - -#define __BITS_PER_LONG 64 - -#include <asm-generic/bitsperlong.h> - -#endif /* __ASM_BITSPERLONG_H */ diff --git a/tools/arch/loongarch/include/uapi/asm/bitsperlong.h b/tools/arch/loongarch/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 00b4ba1..0000000 --- a/tools/arch/loongarch/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,9 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -#ifndef __ASM_LOONGARCH_BITSPERLONG_H -#define __ASM_LOONGARCH_BITSPERLONG_H - -#define __BITS_PER_LONG (__SIZEOF_LONG__ * 8) - -#include <asm-generic/bitsperlong.h> - -#endif /* __ASM_LOONGARCH_BITSPERLONG_H */ diff --git a/tools/arch/riscv/include/uapi/asm/bitsperlong.h b/tools/arch/riscv/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 0b9b58b..0000000 --- a/tools/arch/riscv/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,14 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ -/* - * Copyright (C) 2012 ARM Ltd. - * Copyright (C) 2015 Regents of the University of California - */ - -#ifndef _UAPI_ASM_RISCV_BITSPERLONG_H -#define _UAPI_ASM_RISCV_BITSPERLONG_H - -#define __BITS_PER_LONG (__SIZEOF_POINTER__ * 8) - -#include <asm-generic/bitsperlong.h> - -#endif /* _UAPI_ASM_RISCV_BITSPERLONG_H */ diff --git a/tools/include/uapi/asm-generic/bitsperlong.h b/tools/include/uapi/asm-generic/bitsperlong.h index 23e6c41..352cb81 100644 --- a/tools/include/uapi/asm-generic/bitsperlong.h +++ b/tools/include/uapi/asm-generic/bitsperlong.h @@ -1,6 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ #ifndef _UAPI__ASM_GENERIC_BITS_PER_LONG #define _UAPI__ASM_GENERIC_BITS_PER_LONG +#ifndef __BITS_PER_LONG +/* + * In order to keep safe and avoid regression, only unify uapi + * bitsperlong.h for some archs which are using newer toolchains + * that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. + * See the following link for more info: + * https://lore.kernel.org/linux-arch/b9624545-2c80-49a1-ac3c-39264a591f7b@app.fastmail.com/ + */ +#if defined(__CHAR_BIT__) && defined(__SIZEOF_LONG__) +#define __BITS_PER_LONG (__CHAR_BIT__ * __SIZEOF_LONG__) +#else /* * There seems to be no way of detecting this automatically from user * space, so 64 bit architectures should override this in their @@ -8,8 +20,8 @@ * both 32 and 64 bit user space must not rely on CONFIG_64BIT * to decide it, but rather check a compiler provided macro. */ -#ifndef __BITS_PER_LONG #define __BITS_PER_LONG 32 #endif +#endif #endif /* _UAPI__ASM_GENERIC_BITS_PER_LONG */ diff --git a/tools/include/uapi/asm/bitsperlong.h b/tools/include/uapi/asm/bitsperlong.h index da52065..c65267a 100644 --- a/tools/include/uapi/asm/bitsperlong.h +++ b/tools/include/uapi/asm/bitsperlong.h @@ -1,8 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 */ #if defined(__i386__) || defined(__x86_64__) #include "../../../arch/x86/include/uapi/asm/bitsperlong.h" -#elif defined(__aarch64__) -#include "../../../arch/arm64/include/uapi/asm/bitsperlong.h" #elif defined(__powerpc__) #include "../../../arch/powerpc/include/uapi/asm/bitsperlong.h" #elif defined(__s390__) @@ -13,12 +11,8 @@ #include "../../../arch/mips/include/uapi/asm/bitsperlong.h" #elif defined(__ia64__) #include "../../../arch/ia64/include/uapi/asm/bitsperlong.h" -#elif defined(__riscv) -#include "../../../arch/riscv/include/uapi/asm/bitsperlong.h" #elif defined(__alpha__) #include "../../../arch/alpha/include/uapi/asm/bitsperlong.h" -#elif defined(__loongarch__) -#include "../../../arch/loongarch/include/uapi/asm/bitsperlong.h" #else #include <asm-generic/bitsperlong.h> #endif
Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are usable, it is probably fine to unify the definition of __BITS_PER_LONG as (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h. In order to keep safe and avoid regression, only unify uapi bitsperlong.h for some archs such as arm64, riscv and loongarch which are using newer toolchains that have the definitions of __CHAR_BIT__ and __SIZEOF_LONG__. Suggested-by: Xi Ruoyao <xry111@xry111.site> Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.camel@xry111.site/ Suggested-by: Arnd Bergmann <arnd@arndb.de> Link: https://lore.kernel.org/linux-arch/a3a4f48a-07d4-4ed9-bc53-5d383428bdd2@app.fastmail.com/ Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/arm64/include/uapi/asm/bitsperlong.h | 24 ---------------------- arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- arch/riscv/include/uapi/asm/bitsperlong.h | 14 ------------- include/uapi/asm-generic/bitsperlong.h | 13 +++++++++++- tools/arch/arm64/include/uapi/asm/bitsperlong.h | 24 ---------------------- .../arch/loongarch/include/uapi/asm/bitsperlong.h | 9 -------- tools/arch/riscv/include/uapi/asm/bitsperlong.h | 14 ------------- tools/include/uapi/asm-generic/bitsperlong.h | 14 ++++++++++++- tools/include/uapi/asm/bitsperlong.h | 6 ------ 9 files changed, 25 insertions(+), 102 deletions(-) delete mode 100644 arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 arch/riscv/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/riscv/include/uapi/asm/bitsperlong.h