| Message ID | 20220527100155.1996314-4-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: yet another series of cleanups | expand |
Hello Masahiro, On 5/27/22 12:01, Masahiro Yamada wrote: > parisc overrides 'nm' with a shell script. I do not know the reason, > but anyway it is how it has worked since 2003. [1] I don't know the reason either... I assume it was that the older toolchains had bugs and kept lots of local symbols like .LC? in the object files. I did a small build without the nm script (and removed it's reference in the Makefile), and it did not seem to break anything. > A problem is that this script returns the exit code of grep instead of > ${CROSS_COMPILE}nm. Instead of fixing this, I'd suggest that you simply remove the nm script alltogether. If you like I can apply such a patch in the parisc git tree, and you just drop this specific patch. Or you change your patch to remove it. Just let me know what you prefer. Helge > grep(1) says: > Normally the exit status is 0 if a line is selected, 1 if no lines > were selected, and 2 if an error occurred. However, if the -q or > --quiet or --silent is used and a line is selected, the exit status > is 0 even if an error occurred. > > When the given object has no symbol, grep returns 1, while the true nm > returns 0. Hence, build rules using ${NM} fail on ARCH=parisc even if > the given object is valid. > > This commit corrects the exit status of the script. > > - A pipeline returns the exit status of the last command (here, grep). > The exit status of ${CROSS_COMPILE}nm is just ignored. Use bash's > pipefail flag to catch errors of ${CROSS_COMPILE}nm. > > - If grep returns 1, this script should return 0 in order to mimic > true nm. If grep returns 2, it is a real and fatal error. Return > it as is. > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=36eaa6e4c0e0b6950136b956b72fd08155b92ca3 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v7: > - New patch > > arch/parisc/Makefile | 2 +- > arch/parisc/nm | 12 ++++++++++-- > 2 files changed, 11 insertions(+), 3 deletions(-) > mode change 100644 => 100755 arch/parisc/nm > > diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile > index aca1710fd658..e7139955367d 100644 > --- a/arch/parisc/Makefile > +++ b/arch/parisc/Makefile > @@ -18,7 +18,7 @@ > boot := arch/parisc/boot > KBUILD_IMAGE := $(boot)/bzImage > > -NM = sh $(srctree)/arch/parisc/nm > +NM = $(srctree)/arch/parisc/nm > CHECKFLAGS += -D__hppa__=1 > > ifdef CONFIG_64BIT > diff --git a/arch/parisc/nm b/arch/parisc/nm > old mode 100644 > new mode 100755 > index c788308de33f..3e72238a91f3 > --- a/arch/parisc/nm > +++ b/arch/parisc/nm > @@ -1,6 +1,14 @@ > -#!/bin/sh > +#!/bin/bash > ## > # Hack to have an nm which removes the local symbols. We also rely > # on this nm being hidden out of the ordinarily executable path > ## > -${CROSS_COMPILE}nm $* | grep -v '.LC*[0-9]*$' > + > +# use pipefail to catch error of ${CROSS_COMPILE}nm > +set -o pipefail > + > +# grep exits with 1 if no lines were selected. > +# If the given object has no symbol, grep returns 1, but it is not an error. > + > +${CROSS_COMPILE}nm "$@" | > +{ grep -v '.LC*[0-9]*$' || { exit_code=$?; test $exit_code -eq 1 || exit $exit_code; } }
Hi Helge, On Fri, May 27, 2022 at 10:58 PM Helge Deller <deller@gmx.de> wrote: > > Hello Masahiro, > > On 5/27/22 12:01, Masahiro Yamada wrote: > > parisc overrides 'nm' with a shell script. I do not know the reason, > > but anyway it is how it has worked since 2003. [1] > > I don't know the reason either... > I assume it was that the older toolchains had bugs and kept lots of local > symbols like .LC? in the object files. > > I did a small build without the nm script (and removed it's reference > in the Makefile), and it did not seem to break anything. > > > A problem is that this script returns the exit code of grep instead of > > ${CROSS_COMPILE}nm. > > Instead of fixing this, I'd suggest that you simply remove the nm script > alltogether. If you like I can apply such a patch in the parisc git tree, > and you just drop this specific patch. Or you change your patch to remove it. > Just let me know what you prefer. This is a prerequisite of my kbuild work: https://lore.kernel.org/linux-kbuild/20220527100155.1996314-5-masahiroy@kernel.org/T/#u Without this, ARCH=parisc builds will be broken due to ${NM} returning 1. I will send a patch to remove arch/parisc/nm. Please give me your ack. Thank you. -- Best Regards Masahiro Yamada
diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile index aca1710fd658..e7139955367d 100644 --- a/arch/parisc/Makefile +++ b/arch/parisc/Makefile @@ -18,7 +18,7 @@ boot := arch/parisc/boot KBUILD_IMAGE := $(boot)/bzImage -NM = sh $(srctree)/arch/parisc/nm +NM = $(srctree)/arch/parisc/nm CHECKFLAGS += -D__hppa__=1 ifdef CONFIG_64BIT diff --git a/arch/parisc/nm b/arch/parisc/nm old mode 100644 new mode 100755 index c788308de33f..3e72238a91f3 --- a/arch/parisc/nm +++ b/arch/parisc/nm @@ -1,6 +1,14 @@ -#!/bin/sh +#!/bin/bash ## # Hack to have an nm which removes the local symbols. We also rely # on this nm being hidden out of the ordinarily executable path ## -${CROSS_COMPILE}nm $* | grep -v '.LC*[0-9]*$' + +# use pipefail to catch error of ${CROSS_COMPILE}nm +set -o pipefail + +# grep exits with 1 if no lines were selected. +# If the given object has no symbol, grep returns 1, but it is not an error. + +${CROSS_COMPILE}nm "$@" | +{ grep -v '.LC*[0-9]*$' || { exit_code=$?; test $exit_code -eq 1 || exit $exit_code; } }
parisc overrides 'nm' with a shell script. I do not know the reason, but anyway it is how it has worked since 2003. [1] A problem is that this script returns the exit code of grep instead of ${CROSS_COMPILE}nm. grep(1) says: Normally the exit status is 0 if a line is selected, 1 if no lines were selected, and 2 if an error occurred. However, if the -q or --quiet or --silent is used and a line is selected, the exit status is 0 even if an error occurred. When the given object has no symbol, grep returns 1, while the true nm returns 0. Hence, build rules using ${NM} fail on ARCH=parisc even if the given object is valid. This commit corrects the exit status of the script. - A pipeline returns the exit status of the last command (here, grep). The exit status of ${CROSS_COMPILE}nm is just ignored. Use bash's pipefail flag to catch errors of ${CROSS_COMPILE}nm. - If grep returns 1, this script should return 0 in order to mimic true nm. If grep returns 2, it is a real and fatal error. Return it as is. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/commit/?id=36eaa6e4c0e0b6950136b956b72fd08155b92ca3 Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v7: - New patch arch/parisc/Makefile | 2 +- arch/parisc/nm | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) mode change 100644 => 100755 arch/parisc/nm