All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
@ 2022-10-27 16:28 Masahiro Yamada
  2022-10-27 16:36 ` Nick Desaulniers
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Masahiro Yamada @ 2022-10-27 16:28 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Masahiro Yamada, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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(-)

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-10-27 16:28 [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar Masahiro Yamada
@ 2022-10-27 16:36 ` Nick Desaulniers
  2022-10-27 21:24 ` Nathan Chancellor
  2022-11-16 19:00 ` Kees Cook
  2 siblings, 0 replies; 8+ messages in thread
From: Nick Desaulniers @ 2022-10-27 16:36 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Michal Marek,
	Nicolas Schier, Tom Rix, linux-kernel, llvm

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
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-10-27 16:28 [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar Masahiro Yamada
  2022-10-27 16:36 ` Nick Desaulniers
@ 2022-10-27 21:24 ` Nathan Chancellor
  2022-11-16 19:00 ` Kees Cook
  2 siblings, 0 replies; 8+ messages in thread
From: Nathan Chancellor @ 2022-10-27 21:24 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jiri Slaby, Nick Desaulniers, Michal Marek,
	Nicolas Schier, Tom Rix, linux-kernel, llvm

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
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-10-27 16:28 [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar Masahiro Yamada
  2022-10-27 16:36 ` Nick Desaulniers
  2022-10-27 21:24 ` Nathan Chancellor
@ 2022-11-16 19:00 ` Kees Cook
  2022-11-16 20:37   ` Masahiro Yamada
  2 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2022-11-16 19:00 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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
> 

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-11-16 19:00 ` Kees Cook
@ 2022-11-16 20:37   ` Masahiro Yamada
  2022-11-16 22:07     ` Kees Cook
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2022-11-16 20:37 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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?



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-11-16 20:37   ` Masahiro Yamada
@ 2022-11-16 22:07     ` Kees Cook
  2022-12-06  4:24       ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Kees Cook @ 2022-11-16 22:07 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-11-16 22:07     ` Kees Cook
@ 2022-12-06  4:24       ` Masahiro Yamada
  2022-12-06  5:26         ` Kees Cook
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2022-12-06  4:24 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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?






-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar
  2022-12-06  4:24       ` Masahiro Yamada
@ 2022-12-06  5:26         ` Kees Cook
  0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2022-12-06  5:26 UTC (permalink / raw)
  To: Masahiro Yamada, Kees Cook
  Cc: linux-kbuild, Jiri Slaby, Nathan Chancellor, Nick Desaulniers,
	Michal Marek, Nicolas Schier, Tom Rix, linux-kernel, llvm

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


-- 
Kees Cook

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-12-06  5:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-27 16:28 [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar Masahiro Yamada
2022-10-27 16:36 ` Nick Desaulniers
2022-10-27 21:24 ` Nathan Chancellor
2022-11-16 19:00 ` Kees Cook
2022-11-16 20:37   ` Masahiro Yamada
2022-11-16 22:07     ` Kees Cook
2022-12-06  4:24       ` Masahiro Yamada
2022-12-06  5:26         ` Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.