[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Paul D. Smith]

Follow-up Comment #21, bug #63307 (project make):

This should be fixed in the for the 4.4.1 release; you can download and try a
pre-release of it here:

https://alpha.gnu.org/gnu/make/make-4.4.0.90.tar.gz


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Eric Blake]

Follow-up Comment #20, bug #63307 (project make):

For reference, another failure caused by make's change in behavior:
https://listman.redhat.com/archives/libguestfs/2023-January/030474.html


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Paul D. Smith]

Update of bug #63307 (project make):

  Status:None => Fixed  
 Assigned to:None => psmith 
 Open/Closed:Open => Closed 
   Fixed Release:None => SCM
   Triage Status:None => Small Effort   


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Alexander Kanavin]

Follow-up Comment #19, bug #63307 (project make):

In addition to the diffutils failure (which I am also observing), this change
broke m4 tests:

https://savannah.gnu.org/support/index.php?110767

I'd err on the side of previous behavior, since this is not the last such
failure for sure. 


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: [bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Alejandro Colomar]

On 11/9/22 21:36, Paul D. Smith wrote:

Follow-up Comment #18, bug #63307 (project make):

I can see an argument for both sides ("give up immediately" and "run to
completion").  Most likely it's one of those things where different people
legitimately want different behaviors.

But I think that since all previous versions of GNU Make used the "give up
immediately" model, and it wasn't our intention to specifically change that,
we should preserve that behavior until/unless someone wants the opposite
behavior.

If we have "give up immediately" and you want to guard against that, you can
do so (say, redirecting output to /dev/null or whatever).  If we have "run to
completion" and that's not what you want, you don't have any way to get "give
up immediately" (unless we introduce an option which I don't want to do).

Not to mention the advantage that it's a much simpler (trivial) change to go
back to "give up immediately" :).


Not that I care about the behavior in this corner case, but was reading the list 
and thought of this:


You can actually get a rough guess of the intention of the user:

If -i or -k was used, "run to completion" is probably wanted; otherwise, "give 
up immediately" makes sense as a default.


No need for new flags.  Yes, this is reinterpreting the options for more than 
they were originally purposed, but in my head it makes sense. :)


Cheers,

Alex
--



OpenPGP_signature
Description: OpenPGP digital signature

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Paul D. Smith]

Follow-up Comment #18, bug #63307 (project make):

I can see an argument for both sides ("give up immediately" and "run to
completion").  Most likely it's one of those things where different people
legitimately want different behaviors.

But I think that since all previous versions of GNU Make used the "give up
immediately" model, and it wasn't our intention to specifically change that,
we should preserve that behavior until/unless someone wants the opposite
behavior.

If we have "give up immediately" and you want to guard against that, you can
do so (say, redirecting output to /dev/null or whatever).  If we have "run to
completion" and that's not what you want, you don't have any way to get "give
up immediately" (unless we introduce an option which I don't want to do).

Not to mention the advantage that it's a much simpler (trivial) change to go
back to "give up immediately" :).


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #17, bug #63307 (project make):

> Speaking only for myself, the most realistic scenario I can think of where I
would encounter this myself is if I run 'make 2>&1 | less' and then quit less.
In that case I would not be interested in having any build continue, I would
just want to get back to my prompt

In this scenario the user quit less.
i was thinking about automated builds (jenkins, build farms, etc). Those build
systems get really messy with miriads of pieces of shell code. Should make
abort a build if one of those pipelines fail? My thinking was, if the failed
pipeline was supposed to produce something, then its failure will fail the
build anyway.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #16, bug #63307 (project make):

> It appears that make still performs the same build.  But maybe I just got
something wrong in my test.

For example something like this


$ cat makefile
all: one
one: two
@echo cp $< $@ >&2
@cp $< $@
two:
@echo touch $@ >&2
@touch $@
$ rm one two
$ # this is make-4.3
$ (sleep 2 ; ~/src/make/4.3/make --debug=b)|:
$ ls one two
ls: cannot access 'one': No such file or directory
ls: cannot access 'two': No such file or directory
$ # this is make which handles sigpipe and completes the build
$ (sleep 2 ; ~/src/make/l64/make --debug=b)|:
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
received sigpipe
touch two
received sigpipe
received sigpipe
cp two one
received sigpipe
received sigpipe
received sigpipe
make: write error: stdout
$ ls one two
one  two


~/src/make/l64/make is a modified make which has the following sigpipe handler
installed

static void
sigpipe_handler (int sig)
{
static const char msg[] = "received sigpipe\n";
write (STDERR_FILENO, msg, sizeof msg);
}


i remember you reported that on your system you had to modify make to set
sigpipe to default, otherwise it was ignored.
What about something like

$ cat test.pl
#!/usr/bin/perl

$SIG{'PIPE'} = 'DEFAULT';
system('bash -c "(sleep 4 ; ~/src/make/l64/make --debug=b)|:"');



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #15, bug #63307 (project make):

Whether that's "equally likely" is something we are probably going to disagree
over, especially in the case where output (or potentially error) is piped to
another process, but not worth getting into a discussion about. Suffice to
say, it is indeed possible that the spawned processes would produce no output,
and letting make continue would potentially allow a build to complete
successfully. Whether that's desirable is going to depend on the user.
Speaking only for myself, the most realistic scenario I can think of where I
would encounter this myself is if I run 'make 2>&1 | less' and then quit less.
In that case I would not be interested in having any build continue, I would
just want to get back to my prompt, but I'm happy to accept that other
scenarios may exist where other users may want something different.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Andreas Schwab]

Follow-up Comment #14, bug #63307 (project make):

The SIGPIPE failure only happens when the process actually produces output to
the broken pipe.  But it is equally likely that a recipe doesn't actually
produce any output on stdout.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #13, bug #63307 (project make):

When SIGPIPE is set to the default action, then yes, writing to a closed pipe
is expected to get make to die abnormally. That's how SIGPIPE is designed to
work. :) This is different from a regular failure to write to stdout.

There is little benefit to continuing the build in that case: if the build
continues, spawned processes will see the same closed pipe that make saw, and
those spawned processes will likely fail anyway because they too would be
terminated by SIGPIPE if they attempt to write to stdout.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Paul D. Smith]

Follow-up Comment #12, bug #63307 (project make):

> Should inability to write to stdout cause make to die abnormally?

I had the same thought but oddly, I can't seem to generate a use-case where
make behaves differently with this FATAL_SIG set for SIGPIPE.

It appears that make still performs the same build.  But maybe I just got
something wrong in my test.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #11, bug #63307 (project make):

[comment #10 comment #10:]
> I'm not sure how using a semaphore helps here.  We would still have to clean
up the semaphore with sem_unlink().

indeed.

> The main goal of that change was to allow our cleanup operations to proceed
even when SIGPIPE was found, and semaphores still need to be cleaned up.  To
that end it seems like the idea to use FATAL_SIG() (which already handles
SIG_IGN correctly) is the right one.

Should inability to write to stdout cause make to die abnormally?
Make can leave an error message and complete the build and exit normally.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Paul D. Smith]

Follow-up Comment #10, bug #63307 (project make):

I'm not sure how using a semaphore helps here.  We would still have to clean
up the semaphore with sem_unlink().

The main goal of that change was to allow our cleanup operations to proceed
even when SIGPIPE was found, and semaphores still need to be cleaned up.  To
that end it seems like the idea to use FATAL_SIG() (which already handles
SIG_IGN correctly) is the right one.

Sorry for not seeing that when the change was originally proposed.

Let me know if there is disagreement.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #9, bug #63307 (project make):

On Sat, Nov 5, 2022 at 6:54 PM Philip Guenther wrote:
> This is a change in behavior:
The root of this change is not sigpipe itself, it is the new synchronisation
mechanism which uses temporary files.
Paul, i remember we discussed using a semaphore for output sync. Do you see
reasons to prefer files over a semaphore for that?

>  Can you just skip that if the disposition is SIG_IGN at start?
That is easy with sigaction. However, make also runs on systems where
sigaction is not available. sv63307_fix2.diff does the same as proposed in
update 7.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Additional Item Attachment, bug #63307 (project make):

File name: sv63307_fix2.diff  Size:1 KB


File name: sv63307_test2.diff Size:2 KB




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #8, bug #63307 (project make):

There may be an even simpler solution than that. There was never any need to
ignore SIGPIPE, we just need to make sure temporary files are cleaned up if we
get SIGPIPE, it's okay and expected if make dies after that cleanup. There is
already a mechanism in place for that for other signals. Re-using that for
SIGPIPE works:

--- a/src/main.c
+++ b/src/main.c
@@ -1182,11 +1182,6 @@ main (int argc, char **argv, char **envp)
   /* Useful for attaching debuggers, etc.  */
   SPIN ("main-entry");
 
-  /* Don't die if our stdout sends us SIGPIPE.  */
-#ifdef SIGPIPE
-  bsd_signal (SIGPIPE, SIG_IGN);
-#endif
-
 #ifdef HAVE_ATEXIT
   if (ANY_SET (check_io_state (), IO_STDOUT_OK))
 atexit (close_stdout);
@@ -1285,6 +1280,8 @@ main (int argc, char **argv, char **envp)
   FATAL_SIG (SIGXFSZ);
 #endif
 
+  FATAL_SIG (SIGPIPE);
+
 #undef  FATAL_SIG
 
   /* Do not ignore the child-death signal.  This must be done before


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #7, bug #63307 (project make):

> Other reasons are the desire to avoid complexity in make.

I agree with the desire to avoid complexity, but preserving an ignored SIGPIPE
on top of your patch needs no more than:

  if (bsd_signal (SIGPIPE, handle_sigpipe) == SIG_IGN)
bsd_signal (SIGPIPE, SIG_IGN);


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

Re: [bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Philip Guenther]

On Sat, Nov 5, 2022 at 1:34 PM Dmitry Goncharov 
wrote:

> Follow-up Comment #6, bug #63307 (project make):
>
> > If SIGIGN was ignored before make was started though, it should remain
> ignored, even for make's children, see also
>  in
> reply
> to Andreas Schwab's first patch. Correct me if I am wrong, but I think your
> patch re-enables it in that case?
>
> My patch causes the disposition of sigpipe to be set to SIG_DFL in the new
> process. See https://pubs.opengroup.org/onlinepubs/9699919799/,
> specifically
> "Signals set to be caught by the calling process image shall be set to the
> default action in the new process image (see )."
>
> This desire to have the default disposition is intentional. Some of the
> reasons are explained in update 2. Other reasons are the desire to avoid
> complexity in make. Especially when there is no clear need for that
> complexity.
>

This is a change in behavior: previously, make didn't alter the disposition
of SIGPIPE and a build system could set it to SIG_IGN and have all the
invoked recipes inherit that.  As part of saying that make will ignore
SIGPIPE, you've decided to impose a rule that the rest of the build system
has to deal with it in the recipes instead of permitting a pass-through.
That's an unnecessary and user-unfriendly imposition.


In regards to the specific impl of Andreas's 2nd patch there are 2 points.
>
> 1. That patch fails to restore the disposition in the case of make
> re-executing itself (in order to read an updated makefile).
> This could be fixed by calling sigaction before execve, but that'd not be
> atomic.


> 2. That patch uses posix_spawnattr_setsigdefault to set the disposition
> for a
> child process (when make uses posix_spawn). This fails the purpose of the
> patch, if make was invoked with sigpipe disposition of SIG_HOLD.
>

(SIG_HOLD?  That's not a real disposition (in posix) but rather part of an
obsolete version of a mechanism standardized as sigprocmask() and
orthogonal to the disposition (ignore/default/catch).)



> Fixing those deficiencies is not that cheap. In the end, i didn't see any
> benefit for make to go through that trouble of restoring sigpipe
> disposition
> for its children and i proposed this simple fix.
>
> Unlike attempts to restore, one nice property of the sighandler patch is
> that,
> if more calls to exec are introduced to make, the patch will still work for
> all of them.
>

Setting the disposition of SIGPIPE to be caught with a do-nothing routine
is a good idea.  Can you just skip that if the disposition is SIG_IGN at
start?  Then a SIG_IGN will be inherited but the disposition will otherwise
be reset by execve.

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #6, bug #63307 (project make):

> If SIGIGN was ignored before make was started though, it should remain
ignored, even for make's children, see also
 in reply
to Andreas Schwab's first patch. Correct me if I am wrong, but I think your
patch re-enables it in that case?

My patch causes the disposition of sigpipe to be set to SIG_DFL in the new
process. See https://pubs.opengroup.org/onlinepubs/9699919799/, specifically
"Signals set to be caught by the calling process image shall be set to the
default action in the new process image (see )."

This desire to have the default disposition is intentional. Some of the
reasons are explained in update 2. Other reasons are the desire to avoid
complexity in make. Especially when there is no clear need for that
complexity.


In regards to the specific impl of Andreas's 2nd patch there are 2 points.

1. That patch fails to restore the disposition in the case of make
re-executing itself (in order to read an updated makefile).
This could be fixed by calling sigaction before execve, but that'd not be
atomic.

2. That patch uses posix_spawnattr_setsigdefault to set the disposition for a
child process (when make uses posix_spawn). This fails the purpose of the
patch, if make was invoked with sigpipe disposition of SIG_HOLD.

Fixing those deficiencies is not that cheap. In the end, i didn't see any
benefit for make to go through that trouble of restoring sigpipe disposition
for its children and i proposed this simple fix.

Unlike attempts to restore, one nice property of the sighandler patch is that,
if more calls to exec are introduced to make, the patch will still work for
all of them.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #4, bug #63307 (project make):

Nice idea to just have a signal handler that does nothing. If SIGIGN was
ignored before make was started though, it should remain ignored, even for
make's children, see also
 in reply
to Andreas Schwab's first patch. Correct me if I am wrong, but I think your
patch re-enables it in that case?


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #5, bug #63307 (project make):

That previous comment was meant to read SIGPIPE, not SIGIGN, of course :)


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #2, bug #63307 (project make):

Is it useful to restore sigpipe before spawning children? Is that not good
enough to have sigpipe disposition set to default for children?
There are signals for which it is important to preseve the disposition,
notably sighup for nohup, but not sigpipe.
Unix programs assume that sigpipe is set to the default disposition. Is there
a program which misbehaves if make sets sigpipe to the default?
With these considerations in mind i suggest the patch in the attachment.



___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Follow-up Comment #3, bug #63307 (project make):

Tested on linux, sun and aix.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[Dmitry Goncharov]

Additional Item Attachment, bug #63307 (project make):

File name: sv63307_fix.diff   Size:1 KB


File name: sv63307_test.diff  Size:2 KB




___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

Follow-up Comment #1, bug #63307 (project make):

Can confirm the patch posted to the mailing list,
, works for
me to fix this. Thanks for the prompt response.


___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/

[bug #63307] make 4.4 passes ignored SIGPIPE on to children

[anonymous]

URL:
  

 Summary: make 4.4 passes ignored SIGPIPE on to children
 Project: make
   Submitter: None
   Submitted: Wed 02 Nov 2022 03:12:02 PM UTC
Severity: 3 - Normal
  Item Group: Bug
  Status: None
 Privacy: Public
 Assigned to: None
 Open/Closed: Open
 Discussion Lock: Any
   Component Version: 4.4
Operating System: POSIX-Based
   Fixed Release: None
   Triage Status: None


___

Follow-up Comments:


---
Date: Wed 02 Nov 2022 03:12:02 PM UTC By: Anonymous
Test case:

$ cat Makefile
all:
(sleep 1; echo foo; echo bar >&2) | :
$ make-4.3
(sleep 1; echo foo; echo bar >&2) | :
$ make-4.4
(sleep 1; echo foo; echo bar >&2) | :
/bin/sh: 1: echo: Broken pipe
bar
$ 

I asked in #63248 whether this was intentional. It was not, so here is a bug
for it. What happens in make 4.3 is the sleep makes it so that the "echo foo"
is extremely unlikely to start until the : has already terminated. Then, when
"echo foo" attempts to write, the whole shell process receives SIGPIPE. What
happens in make 4.4 is that the shell process does not receive SIGPIPE and
continues processing.

This causes SuSE bug 33947
(https://lists.gnu.org/archive/html/bug-diffutils/2019-01/msg0.html,
diffutils testsuite failure when SIGPIPE is ignored) to appear in environments
where it previously did not, after upgrading to make 4.4.







___

Reply to this item at:

  

___
Message sent via Savannah
https://savannah.gnu.org/