| Message ID | 20221027162839.410720-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar | expand |
On Thu, Oct 27, 2022 at 9:28 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > /usr/bin/ar terminated with signal 13 [Broken pipe] > > Nathan Chancellor reported the latest AR=llvm-ar shows > error: write on a pipe with no reader > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > to the default SIGPIPE handler"). > > The resulting vmlinux is correct, but it is better to silence it. > > 'head -n1' exits after reading the first line, so the pipe is closed. > > Use 'sed -n 1p' to eat the stream till the end. > > Fixes: 321648455061 ("kbuild: use obj-y instead extra-y for objects placed at the head") > Link: https://github.com/ClangBuiltLinux/linux/issues/1651 > Reported-by: Jiri Slaby <jirislaby@kernel.org> > Reported-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> Looks great! Thanks all. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > Changes in v2: > - Update commit description to mention llvm-ar > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e90bb2b38607..e9e7eff906a5 100644 > --- a/Makefile > +++ b/Makefile > @@ -1218,7 +1218,7 @@ quiet_cmd_ar_vmlinux.a = AR $@ > cmd_ar_vmlinux.a = \ > rm -f $@; \ > $(AR) cDPrST $@ $(KBUILD_VMLINUX_OBJS); \ > - $(AR) mPiT $$($(AR) t $@ | head -n1) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > + $(AR) mPiT $$($(AR) t $@ | sed -n 1p) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > > targets += vmlinux.a > vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt autoksyms_recursive FORCE > -- > 2.34.1 >
On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > /usr/bin/ar terminated with signal 13 [Broken pipe] > > Nathan Chancellor reported the latest AR=llvm-ar shows > error: write on a pipe with no reader > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > to the default SIGPIPE handler"). > > The resulting vmlinux is correct, but it is better to silence it. > > 'head -n1' exits after reading the first line, so the pipe is closed. > > Use 'sed -n 1p' to eat the stream till the end. > > Fixes: 321648455061 ("kbuild: use obj-y instead extra-y for objects placed at the head") > Link: https://github.com/ClangBuiltLinux/linux/issues/1651 > Reported-by: Jiri Slaby <jirislaby@kernel.org> > Reported-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Nathan Chancellor <nathan@kernel.org> > --- > > Changes in v2: > - Update commit description to mention llvm-ar > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e90bb2b38607..e9e7eff906a5 100644 > --- a/Makefile > +++ b/Makefile > @@ -1218,7 +1218,7 @@ quiet_cmd_ar_vmlinux.a = AR $@ > cmd_ar_vmlinux.a = \ > rm -f $@; \ > $(AR) cDPrST $@ $(KBUILD_VMLINUX_OBJS); \ > - $(AR) mPiT $$($(AR) t $@ | head -n1) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > + $(AR) mPiT $$($(AR) t $@ | sed -n 1p) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > > targets += vmlinux.a > vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt autoksyms_recursive FORCE > -- > 2.34.1 >
On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > /usr/bin/ar terminated with signal 13 [Broken pipe] > > Nathan Chancellor reported the latest AR=llvm-ar shows > error: write on a pipe with no reader > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > to the default SIGPIPE handler"). > > The resulting vmlinux is correct, but it is better to silence it. > > 'head -n1' exits after reading the first line, so the pipe is closed. > > Use 'sed -n 1p' to eat the stream till the end. I think this is wrong because it needlessly consumes CPU time. SIGPIPE is _needed_ to stop a process after we found what we needed, but it's up to the caller (the shell here) to determine what to do about it. Similarly, that LLVM commit is wrong -- tools should _not_ catch their own SIGPIPEs. They should be caught by their callers. For example, see: $ seq 10000 | head -n1 1 ^^^ no warnings from the shell (caller of "seq") And you can see it _is_ being killed by SIGPIPE: $ strace seq 1000 | head -n1 ... write(1, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 8192) = 8192 1 write(1, "\n1861\n1862\n1863\n1864\n1865\n1866\n1"..., 4096) = -1 EPIPE (Broken pipe) --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=3503448, si_uid=1000} --- +++ killed by SIGPIPE +++ If we use "sed -n 1p" seq will continue to run, consuming needless time and CPU resources. So, I strongly think this is the wrong solution. SIGPIPE should be ignored for ar, and LLVM should _not_ catch its own SIGPIPE. -Kees > > Fixes: 321648455061 ("kbuild: use obj-y instead extra-y for objects placed at the head") > Link: https://github.com/ClangBuiltLinux/linux/issues/1651 > Reported-by: Jiri Slaby <jirislaby@kernel.org> > Reported-by: Nathan Chancellor <nathan@kernel.org> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Tested-by: Nick Desaulniers <ndesaulniers@google.com> > --- > > Changes in v2: > - Update commit description to mention llvm-ar > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index e90bb2b38607..e9e7eff906a5 100644 > --- a/Makefile > +++ b/Makefile > @@ -1218,7 +1218,7 @@ quiet_cmd_ar_vmlinux.a = AR $@ > cmd_ar_vmlinux.a = \ > rm -f $@; \ > $(AR) cDPrST $@ $(KBUILD_VMLINUX_OBJS); \ > - $(AR) mPiT $$($(AR) t $@ | head -n1) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > + $(AR) mPiT $$($(AR) t $@ | sed -n 1p) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) > > targets += vmlinux.a > vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt autoksyms_recursive FORCE > -- > 2.34.1 >
On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <keescook@chromium.org> wrote: > > On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: > > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > > /usr/bin/ar terminated with signal 13 [Broken pipe] > > > > Nathan Chancellor reported the latest AR=llvm-ar shows > > error: write on a pipe with no reader > > > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > > to the default SIGPIPE handler"). > > > > The resulting vmlinux is correct, but it is better to silence it. > > > > 'head -n1' exits after reading the first line, so the pipe is closed. > > > > Use 'sed -n 1p' to eat the stream till the end. > > I think this is wrong because it needlessly consumes CPU time. SIGPIPE > is _needed_ to stop a process after we found what we needed, but it's up > to the caller (the shell here) to determine what to do about it. > > Similarly, that LLVM commit is wrong -- tools should _not_ catch their > own SIGPIPEs. They should be caught by their callers. > > For example, see: > > $ seq 10000 | head -n1 > 1 > > ^^^ no warnings from the shell (caller of "seq") > And you can see it _is_ being killed by SIGPIPE: > > $ strace seq 1000 | head -n1 > ... > write(1, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 8192) = 8192 > 1 > write(1, "\n1861\n1862\n1863\n1864\n1865\n1866\n1"..., 4096) = -1 EPIPE (Broken pipe) > --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=3503448, si_uid=1000} --- > +++ killed by SIGPIPE +++ > > If we use "sed -n 1p" seq will continue to run, consuming needless time > and CPU resources. > > So, I strongly think this is the wrong solution. SIGPIPE should be > ignored for ar, and LLVM should _not_ catch its own SIGPIPE. > > -Kees I thought of this - it is just wasting CPU time, but I did not come up with a better idea on the kbuild side. I do not want to use 2>/dev/null because it may hide non-SIGPIPE (i.e. real) errors. I think you guys will be keen on fixing llvm. I hope gcc as well?
On Thu, Nov 17, 2022 at 05:37:31AM +0900, Masahiro Yamada wrote: > On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <keescook@chromium.org> wrote: > > > > On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: > > > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > > > /usr/bin/ar terminated with signal 13 [Broken pipe] > > > > > > Nathan Chancellor reported the latest AR=llvm-ar shows > > > error: write on a pipe with no reader > > > > > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > > > to the default SIGPIPE handler"). > > > > > > The resulting vmlinux is correct, but it is better to silence it. > > > > > > 'head -n1' exits after reading the first line, so the pipe is closed. > > > > > > Use 'sed -n 1p' to eat the stream till the end. > > > > I think this is wrong because it needlessly consumes CPU time. SIGPIPE > > is _needed_ to stop a process after we found what we needed, but it's up > > to the caller (the shell here) to determine what to do about it. > > > > Similarly, that LLVM commit is wrong -- tools should _not_ catch their > > own SIGPIPEs. They should be caught by their callers. > > > > For example, see: > > > > $ seq 10000 | head -n1 > > 1 > > > > ^^^ no warnings from the shell (caller of "seq") > > And you can see it _is_ being killed by SIGPIPE: > > > > $ strace seq 1000 | head -n1 > > ... > > write(1, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 8192) = 8192 > > 1 > > write(1, "\n1861\n1862\n1863\n1864\n1865\n1866\n1"..., 4096) = -1 EPIPE (Broken pipe) > > --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=3503448, si_uid=1000} --- > > +++ killed by SIGPIPE +++ > > > > If we use "sed -n 1p" seq will continue to run, consuming needless time > > and CPU resources. > > > > So, I strongly think this is the wrong solution. SIGPIPE should be > > ignored for ar, and LLVM should _not_ catch its own SIGPIPE. > > > > -Kees > > > I thought of this - it is just wasting CPU time, > but I did not come up with a better idea on the kbuild side. > > I do not want to use 2>/dev/null because it may hide > non-SIGPIPE (i.e. real) errors. Yes, I've opened an upstream LLVM bug for this: https://github.com/llvm/llvm-project/issues/59037
On Thu, Nov 17, 2022 at 7:07 AM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Nov 17, 2022 at 05:37:31AM +0900, Masahiro Yamada wrote: > > On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: > > > > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: > > > > /usr/bin/ar terminated with signal 13 [Broken pipe] > > > > > > > > Nathan Chancellor reported the latest AR=llvm-ar shows > > > > error: write on a pipe with no reader > > > > > > > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message > > > > to the default SIGPIPE handler"). > > > > > > > > The resulting vmlinux is correct, but it is better to silence it. > > > > > > > > 'head -n1' exits after reading the first line, so the pipe is closed. > > > > > > > > Use 'sed -n 1p' to eat the stream till the end. > > > > > > I think this is wrong because it needlessly consumes CPU time. SIGPIPE > > > is _needed_ to stop a process after we found what we needed, but it's up > > > to the caller (the shell here) to determine what to do about it. > > > > > > Similarly, that LLVM commit is wrong -- tools should _not_ catch their > > > own SIGPIPEs. They should be caught by their callers. > > > > > > For example, see: > > > > > > $ seq 10000 | head -n1 > > > 1 > > > > > > ^^^ no warnings from the shell (caller of "seq") > > > And you can see it _is_ being killed by SIGPIPE: > > > > > > $ strace seq 1000 | head -n1 > > > ... > > > write(1, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 8192) = 8192 > > > 1 > > > write(1, "\n1861\n1862\n1863\n1864\n1865\n1866\n1"..., 4096) = -1 EPIPE (Broken pipe) > > > --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=3503448, si_uid=1000} --- > > > +++ killed by SIGPIPE +++ > > > > > > If we use "sed -n 1p" seq will continue to run, consuming needless time > > > and CPU resources. > > > > > > So, I strongly think this is the wrong solution. SIGPIPE should be > > > ignored for ar, and LLVM should _not_ catch its own SIGPIPE. > > > > > > -Kees > > > > > > I thought of this - it is just wasting CPU time, > > but I did not come up with a better idea on the kbuild side. > > > > I do not want to use 2>/dev/null because it may hide > > non-SIGPIPE (i.e. real) errors. > > Yes, I've opened an upstream LLVM bug for this: > https://github.com/llvm/llvm-project/issues/59037 > > -- > Kees Cook BTW, Python does something similar by default. (noisy back-trace for SIGPIPE) masahiro@zoe:/tmp$ cat test.py #!/usr/bin/python3 for i in range(4000): print(i) masahiro@zoe:/tmp$ ./test.py | head -n1 0 Traceback (most recent call last): File "/tmp/./test.py", line 3, in <module> print(i) BrokenPipeError: [Errno 32] Broken pipe This page https://www.geeksforgeeks.org/broken-pipe-error-in-python/ suggests some workarounds. Python scripts potentially have this issue. $ ./scripts/diffconfig .config.old .config | head -n1 -104_QUAD_8 m Traceback (most recent call last): File "/home/masahiro/ref/linux/./scripts/diffconfig", line 132, in <module> main() File "/home/masahiro/ref/linux/./scripts/diffconfig", line 111, in main print_config("-", config, a[config], None) File "/home/masahiro/ref/linux/./scripts/diffconfig", line 62, in print_config print("-%s %s" % (config, value)) BrokenPipeError: [Errno 32] Broken pipe What would you suggest for python scripts?
On December 5, 2022 8:24:41 PM PST, Masahiro Yamada <masahiroy@kernel.org> wrote: >On Thu, Nov 17, 2022 at 7:07 AM Kees Cook <keescook@chromium.org> wrote: >> >> On Thu, Nov 17, 2022 at 05:37:31AM +0900, Masahiro Yamada wrote: >> > On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <keescook@chromium.org> wrote: >> > > >> > > On Fri, Oct 28, 2022 at 01:28:39AM +0900, Masahiro Yamada wrote: >> > > > Jiri Slaby reported that building the kernel with AR=gcc-ar shows: >> > > > /usr/bin/ar terminated with signal 13 [Broken pipe] >> > > > >> > > > Nathan Chancellor reported the latest AR=llvm-ar shows >> > > > error: write on a pipe with no reader >> > > > >> > > > The latter occurs since LLVM commit 51b557adc131 ("Add an error message >> > > > to the default SIGPIPE handler"). >> > > > >> > > > The resulting vmlinux is correct, but it is better to silence it. >> > > > >> > > > 'head -n1' exits after reading the first line, so the pipe is closed. >> > > > >> > > > Use 'sed -n 1p' to eat the stream till the end. >> > > >> > > I think this is wrong because it needlessly consumes CPU time. SIGPIPE >> > > is _needed_ to stop a process after we found what we needed, but it's up >> > > to the caller (the shell here) to determine what to do about it. >> > > >> > > Similarly, that LLVM commit is wrong -- tools should _not_ catch their >> > > own SIGPIPEs. They should be caught by their callers. >> > > >> > > For example, see: >> > > >> > > $ seq 10000 | head -n1 >> > > 1 >> > > >> > > ^^^ no warnings from the shell (caller of "seq") >> > > And you can see it _is_ being killed by SIGPIPE: >> > > >> > > $ strace seq 1000 | head -n1 >> > > ... >> > > write(1, "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14"..., 8192) = 8192 >> > > 1 >> > > write(1, "\n1861\n1862\n1863\n1864\n1865\n1866\n1"..., 4096) = -1 EPIPE (Broken pipe) >> > > --- SIGPIPE {si_signo=SIGPIPE, si_code=SI_USER, si_pid=3503448, si_uid=1000} --- >> > > +++ killed by SIGPIPE +++ >> > > >> > > If we use "sed -n 1p" seq will continue to run, consuming needless time >> > > and CPU resources. >> > > >> > > So, I strongly think this is the wrong solution. SIGPIPE should be >> > > ignored for ar, and LLVM should _not_ catch its own SIGPIPE. >> > > >> > > -Kees >> > >> > >> > I thought of this - it is just wasting CPU time, >> > but I did not come up with a better idea on the kbuild side. >> > >> > I do not want to use 2>/dev/null because it may hide >> > non-SIGPIPE (i.e. real) errors. >> >> Yes, I've opened an upstream LLVM bug for this: >> https://github.com/llvm/llvm-project/issues/59037 >> >> -- >> Kees Cook > > > >BTW, Python does something similar by default. >(noisy back-trace for SIGPIPE) > > > > > >masahiro@zoe:/tmp$ cat test.py >#!/usr/bin/python3 >for i in range(4000): > print(i) > >masahiro@zoe:/tmp$ ./test.py | head -n1 >0 >Traceback (most recent call last): > File "/tmp/./test.py", line 3, in <module> > print(i) >BrokenPipeError: [Errno 32] Broken pipe Eww. Well, same problem, IMO. For any Python scripts that are going to have potentially truncated output, they need to do: from signal import signal, SIGPIPE, SIG_DFL signal(SIGPIPE,SIG_DFL) >This page >https://www.geeksforgeeks.org/broken-pipe-error-in-python/ > >suggests some workarounds. (As suggested in this page.) >What would you suggest for python scripts? They need to be fixed. A command line tool internally catching SIGPIPE is wrong. :) -Kees
diff --git a/Makefile b/Makefile index e90bb2b38607..e9e7eff906a5 100644 --- a/Makefile +++ b/Makefile @@ -1218,7 +1218,7 @@ quiet_cmd_ar_vmlinux.a = AR $@ cmd_ar_vmlinux.a = \ rm -f $@; \ $(AR) cDPrST $@ $(KBUILD_VMLINUX_OBJS); \ - $(AR) mPiT $$($(AR) t $@ | head -n1) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) + $(AR) mPiT $$($(AR) t $@ | sed -n 1p) $@ $$($(AR) t $@ | grep -F -f $(srctree)/scripts/head-object-list.txt) targets += vmlinux.a vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt autoksyms_recursive FORCE
Jiri Slaby reported that building the kernel with AR=gcc-ar shows: /usr/bin/ar terminated with signal 13 [Broken pipe] Nathan Chancellor reported the latest AR=llvm-ar shows error: write on a pipe with no reader The latter occurs since LLVM commit 51b557adc131 ("Add an error message to the default SIGPIPE handler"). The resulting vmlinux is correct, but it is better to silence it. 'head -n1' exits after reading the first line, so the pipe is closed. Use 'sed -n 1p' to eat the stream till the end. Fixes: 321648455061 ("kbuild: use obj-y instead extra-y for objects placed at the head") Link: https://github.com/ClangBuiltLinux/linux/issues/1651 Reported-by: Jiri Slaby <jirislaby@kernel.org> Reported-by: Nathan Chancellor <nathan@kernel.org> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Tested-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes in v2: - Update commit description to mention llvm-ar Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)