2022-10-27 16:51:22

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

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 <[email protected]>
Reported-by: Nathan Chancellor <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
Tested-by: Nick Desaulniers <[email protected]>
---

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



2022-10-27 16:52:46

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

On Thu, Oct 27, 2022 at 9:28 AM Masahiro Yamada <[email protected]> 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 <[email protected]>
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>

Looks great! Thanks all.
Reviewed-by: Nick Desaulniers <[email protected]>

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

2022-10-27 22:19:56

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

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 <[email protected]>
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>

Tested-by: Nathan Chancellor <[email protected]>

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

2022-11-16 19:33:02

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

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 <[email protected]>
> Reported-by: Nathan Chancellor <[email protected]>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Tested-by: Nick Desaulniers <[email protected]>
> ---
>
> 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

2022-11-16 20:49:39

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <[email protected]> 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

2022-11-16 22:25:15

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

On Thu, Nov 17, 2022 at 05:37:31AM +0900, Masahiro Yamada wrote:
> On Thu, Nov 17, 2022 at 4:01 AM Kees Cook <[email protected]> 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

2022-12-06 05:00:23

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

On Thu, Nov 17, 2022 at 7:07 AM Kees Cook <[email protected]> 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 <[email protected]> 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

2022-12-06 05:58:51

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2] kbuild: fix SIGPIPE error message for AR=gcc-ar and AR=llvm-ar

On December 5, 2022 8:24:41 PM PST, Masahiro Yamada <[email protected]> wrote:
>On Thu, Nov 17, 2022 at 7:07 AM Kees Cook <[email protected]> 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 <[email protected]> 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