bugmake - Bugs: bug #57242, Non-recursive command passes...

 
 

bug #57242: Non-recursive command passes invalid jobserver file descriptors

Submitter:  Stefan Brüns <stefanbruens>
Submitted:  Fri 15 Nov 2019 07:41:39 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.2.1 Operating System:  POSIX-Based
Fixed Release:  4.4 Triage Status:  Medium Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 10 Aug 2022 11:22:30 AM UTC, comment #15: 

Note the FIFO support has been added to the current GCC master.
May I please ask about the next Make release, is it planned any time soon?

Martin Liška <marxin>
Tue 09 Aug 2022 12:03:38 PM UTC, comment #14: 


> One note: an ideal implementation would look for the last instance of --jobserver-auth in MAKEFLAGS, not the first instance.  In the current code in Git, if we are using anonymous pipes and we invoke a sub-process which is not considered to be a recursive make, we will add an extra "--jobserver-auth=-2,-2" to the END of MAKEFLAGS to disable the jobserver (unfortunately it's complicated to try to replace the existing option so instead we just add an overriding option.
>
> This should make the jobserver more reliable than before, even when using anonymous pipes.  It doesn't solve all problems though: named pipes works better in many situations.


Yeah, I've done that in v2:
https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599478.html

Martin Liška <marxin>
Thu 04 Aug 2022 01:58:16 PM UTC, comment #13: 

OK; I guess if it's the old style anonymous pipe you do something different?

One note: an ideal implementation would look for the last instance of --jobserver-auth in MAKEFLAGS, not the first instance.  In the current code in Git, if we are using anonymous pipes and we invoke a sub-process which is not considered to be a recursive make, we will add an extra "--jobserver-auth=-2,-2" to the END of MAKEFLAGS to disable the jobserver (unfortunately it's complicated to try to replace the existing option so instead we just add an overriding option.

This should make the jobserver more reliable than before, even when using anonymous pipes.  It doesn't solve all problems though: named pipes works better in many situations.

Paul D. Smith <psmith>
Group administrator
Thu 04 Aug 2022 01:22:07 PM UTC, comment #12: 


comment #11:

> I don't see how that patch can work.  There has to be some code somewhere to open the named pipe...??


No. It works by preserving the MAKEFLAGS env. variable as we call make subprocess from lto-wrapper (where the sub-make will correctly connect to the jobserver).

Martin Liška <marxin>
Thu 04 Aug 2022 12:57:29 PM UTC, comment #11: 

I don't see how that patch can work.  There has to be some code somewhere to open the named pipe...??

Paul D. Smith <psmith>
Group administrator
Thu 04 Aug 2022 08:58:26 AM UTC, comment #10: 

Btw. I've just sent a patch for GCC that supports it:
https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599313.html

Martin Liška <marxin>
Thu 04 Aug 2022 08:48:43 AM UTC, comment #9: 

comment #8:

> This should be resolved in the next release of GNU make which uses named pipes for the jobserver.


Oh, great, thanks a lot for it. We (GCC) have been waiting for the feature for a long time. When can we expect a new make release?

Martin Liška <marxin>
Wed 03 Aug 2022 03:44:05 AM UTC, comment #8: 

This should be resolved in the next release of GNU make which uses named pipes for the jobserver.

Paul D. Smith <psmith>
Group administrator
Tue 12 Jan 2021 01:16:16 PM UTC, comment #7: 

About this statement below:

"This is necessary, because a command can mess up job server operation or a command may expect a specific fd to be available."

a) I wonder how a command can mess up job server operation:

a1) One way would be that a program is looking at all open file descriptors it inherited, and messing with them. I am not sure whether we want to make the job server robust against such attacks.

a2) Another way is that the program has a bug and is not using the job server properly. But I do not think that this scenario is worth much consideration either.


b) If one really wants to stop inheriting the job server and the associated flag, I wonder whether a makefile can filter out the job server options from MAKEFLAGS.

I am actually doing the opposite, I am fetching the jobserver flags from MAKEFLAGS and letting only those through. Check out variable EXTRACT_SELECTED_FLAGS_FROM_MAKEFLAGS in this makefile:

https://github.com/rdiez/JtagDue/blob/master/Toolchain/Makefile

Would it be possible to use GNU Make function $(filter-out pattern...,text) to remove the job server options?


c) I wonder how "a command may expect a specific fd to be available".

A while ago I wanted a way to always pass down the jobserver file descriptors:

Passing jobserver file descriptors to all children
https://lists.gnu.org/archive/html/help-make/2020-02/msg00000.html

I encountered a similar statement then:

"Originally make did pass these file descriptors to all children, but there were bugs filed because some tools invoked by make expected to have specific file descriptors available; having make pass down open FDs caused them to fail."

Can someone provide more detail about which file descriptors a program may expect to be available? Because that is generally considered bad practice, as far as I know.

If this mainly refers to Bash scripts, I found in its manual page that Bash seems to use descriptors above 10 internally, as only the low 10 seem guarantee stability for scripts:

"Redirections using file descriptors greater than 9 should be used with care, as they may conflict with file descriptors the shell uses internally."

Maybe if GNU Make moved those job server descriptors to higher numbers, like Bash does internally, there would be less problems in this area. I cannot imagine that some program or script may assume that file descriptor number 12,345 must be available right now. But maybe I am missing something, I am no expert in this area.

R. Diez <rdiez>
Mon 30 Nov 2020 09:39:59 PM UTC, comment #6: 

I can't speak about the broader situation but Paul's "much simpler" option would certainly be sufficient for my use case.

Something else that would be useful to us is a way to force make to consider some or all subprograms to be recursive make invocations. The "+" convention does not work well for us because we're working with third party (and usually auto-generated) makefiles and it's no fun to mark these up with "+" characters. I'm currently working around this using a one-line patch to make, but that isn't something that I can generally expect other people to do.

Regarding the named pipe, I assume this would also solve the issue where there's some uncertainty about the inherited jobserver fds being blocking vs. non-blocking, because every client would simply open the pipe in whatever mode suited it? If so, then I'm all for it.

John Regehr <regehr>
Mon 30 Nov 2020 09:19:09 PM UTC, comment #5: 

It is not TOO hard to provide a custom value (that doesn't include the jobserver options) for the MAKEFLAGS environment variable we set when forking/execing a command that is not a recursive make.

It's much trickier to reset the MAKEFLAGS make variable to not include those options when expanding the command lines.  For example:


foo:
        +: $(MAKEFLAGS)
        +: $$MAKEFLAGS
        : $(MAKEFLAGS)
        : $$MAKEFLAGS


Ideally when run with -j you'd see the jobserver options in the first two lines and you would NOT see them in the last two lines but this is hard.

Much simpler would be to show the jobserver options in the first three lines but not show them in the last line.  For most uses this is probably sufficient since many programs will be obtaining this information from their environment anyway, not from the command line.  Still, it's an annoying "gotcha".

Regarding named pipes: yes this would be a good option (although it comes with its own issues such as location and permissions); I didn't use this because I wanted the solution to be maximally portable.  But, perhaps there are no useful systems left that both (a) provide standard pipes sufficiently well to support the jobserver and (b) don't provide named pipes.

Paul D. Smith <psmith>
Group administrator
Mon 30 Nov 2020 04:36:50 PM UTC, comment #4: 

Hello, I wanted to add that I have written a jobserver client and am currently getting burned by the issue described in this bug.

We have an LLVM plugin that is compute-intensive, so it wants to use multiple cores. However, by the time execution reaches our plugin, fds 3 and 4 are in use by LLVM even if they were closed on exec by make, making it hard to tell whether these fds lead to the jobserver or not

Of course the problem only happens if people fail to mark LLVM as a recursive make command, but this is a very easy mistake to make, and the consequences (reading and writing bytes from whatever random files LLVM has open at the moment) seem pretty unpleasant.

John Regehr <regehr>
Wed 20 Nov 2019 08:22:52 AM UTC, comment #3: 

I fully agree with Stafan that MAKEFLAGS should be aligned with jobserver FDs.
My impression is that a much better approach would be the usage of named pipes instead of file descriptors. I would consider the process relying on fixed file descriptor numbers as a bad practice.

Martin Liška <marxin>
Tue 19 Nov 2019 10:24:27 PM UTC, comment #2: 

comment #1:

> Stefan, the behavior you described is intended.
> make closes the pipe unless the command has + or (MAKE) or {MAKE}.
> This is necessary, because a command can mess up job server operation or a command may expect a specific fd to be available.


Then at least the information should be consistent and the MAKEFLAGS should not mention jobserver FDs.

As is, it already messes up make itself - a make process invocated indirectly may get valid filedescriptors not belonging to the jobserver, and start busy looping on the "read(rfd, 1)" call (when the other end is closed and read returns 0).

Stefan Brüns <stefanbruens>
Tue 19 Nov 2019 09:58:28 PM UTC, comment #1: 

Stefan, the behavior you described is intended.
make closes the pipe unless the command has + or (MAKE) or {MAKE}.
This is necessary, because a command can mess up job server operation or a command may expect a specific fd to be available.

Dmitry Goncharov <dgoncharov>
Fri 15 Nov 2019 07:41:39 PM UTC, original submission:  

The problem can be reproduced using the following trivial
Makefile:
---
all:
    env | grep FLAGS
---

Using strace to get some diagnostics:
$> env -i strace -epipe,dup2,execve,fcntl -v -f -o nonrecursive.txt  /usr/bin/make -j2 -f  Makefile

Obivously, the jobserver file descriptors (3,4) are closed on exec by fcntl({3,4}, F_SETFD, FD_CLOEXEC), but are still exported via MFLAGS/MAKEFLAGS=--jobserver-auth=3,4.

When the "env | grep FLAGS" command is invoked via shell, the shell creates a new pipe, reusing the no longer used file descriptors 3 and 4.

I.e. "env" (which serves as a placeholder here for some command implementing the jobserver protocol) receives some file descriptors which are no jobserver fds.

This does not happen when the command is marked as a recursive one (using the '+' marker), but this is non-trivial to achieve in practice:

1. the Makefile may come from some generator
2. the command itself may be some script invoking "gmake" again.

Stefan Brüns <stefanbruens>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by saturn
  • -email is unavailable- added by rdiez (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by regehr (Posted a comment)
  • -email is unavailable- added by marxin (Posted a comment)
  • -email is unavailable- added by dgoncharov (Posted a comment)
  • -email is unavailable- added by stefanbruens
  • -email is unavailable- added by stefanbruens (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-08-03 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4
        Triage StatusNone Medium Effort
    2022-02-18 saturn Carbon-Copy- Added saturn
    2019-11-15 stefanbruens Carbon-Copy- Added marxin

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code