[PATCH] Handle jobserver file descriptors in btest.

[PATCH] Handle jobserver file descriptors in btest.

[Martin Liška]

Hi.

The patch is about sensitive handling of file descriptors opened
by make's jobserver.

Ready for master?
Thanks,
Martin

	PR testsuite/102742

libbacktrace/ChangeLog:

	* btest.c (check_open_files): Ignore file descriptors provided
	by jobserver.
---
  libbacktrace/btest.c | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/libbacktrace/btest.c b/libbacktrace/btest.c
index 9f9c03babf3..a1ea93728be 100644
--- a/libbacktrace/btest.c
+++ b/libbacktrace/btest.c
@@ -464,9 +464,25 @@ static void
  check_open_files (void)
  {
    int i;
+  int rfd = -1;
+  int wfd = -1;
+
+#define JS_NEEDLE "--jobserver-auth="
+
+  /* Ignore file descriptors provided by jobserver.  */
+  const char *makeflags = getenv ("MAKEFLAGS");
+  if (makeflags != NULL)
+    {
+      const char *n = strstr (makeflags, JS_NEEDLE);
+      if (n != NULL)
+	sscanf (n + strlen (JS_NEEDLE), "%d,%d", &rfd, &wfd);
+    }
  
    for (i = 3; i < 10; i++)
      {
+      if (i == rfd || i == wfd)
+	continue;
+
        if (close (i) == 0)
  	{
  	  fprintf (stderr,
-- 
2.33.1

Re: [PATCH] Handle jobserver file descriptors in btest.

[Ian Lance Taylor]

On Thu, Oct 21, 2021 at 12:48 AM Martin Liška <mliska@suse.cz> wrote:
>
> The patch is about sensitive handling of file descriptors opened
> by make's jobserver.

Thanks.  I think a better approach would be, at the start of main,
fstat the descriptors up to 10 and record the ones for which fstat
succeeds.  Then at the end of main only check the descriptors for
which fstat failed earlier.

I can work on that at some point if you don't want to tackle it.

Ian

Re: [PATCH] Handle jobserver file descriptors in btest.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

On 10/21/21 20:15, Ian Lance Taylor wrote:
> On Thu, Oct 21, 2021 at 12:48 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> The patch is about sensitive handling of file descriptors opened
>> by make's jobserver.
> 
> Thanks.  I think a better approach would be, at the start of main,
> fstat the descriptors up to 10 and record the ones for which fstat
> succeeds.  Then at the end of main only check the descriptors for
> which fstat failed earlier.

Sure, makes sense.

> 
> I can work on that at some point if you don't want to tackle it.

I've just done that in the attached patch.

Is it fine?
Thanks,
Martin

> 
> Ian
> 

[-- Attachment #2: 0001-Handle-jobserver-file-descriptors-in-btest.patch --]
[-- Type: text/x-patch, Size: 1883 bytes --]

From ad52a33e10f76119867dbf0b6d5875378aa399ab Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Fri, 22 Oct 2021 10:12:56 +0200
Subject: [PATCH] Handle jobserver file descriptors in btest.

	PR testsuite/102742

libbacktrace/ChangeLog:

	* btest.c (MIN_DESCRIPTOR): New.
	(MAX_DESCRIPTOR): Likewise.
	(check_available_files): Likewise.
	(check_open_files): Check only file descriptors that
	were not available at the entry.
	(main): Call check_available_files.
---
 libbacktrace/btest.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/libbacktrace/btest.c b/libbacktrace/btest.c
index 9f9c03babf3..7ef6d320497 100644
--- a/libbacktrace/btest.c
+++ b/libbacktrace/btest.c
@@ -38,6 +38,7 @@ POSSIBILITY OF SUCH DAMAGE.  */
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/stat.h>
 
 #include "filenames.h"
 
@@ -458,16 +459,29 @@ test5 (void)
   return failures;
 }
 
+#define MIN_DESCRIPTOR 3
+#define MAX_DESCRIPTOR 10
+
+static int fstat_status[MAX_DESCRIPTOR];
+
+/* Check files that are available.  */
+
+static void
+check_available_files (void)
+{
+  struct stat s;
+  for (unsigned i = MIN_DESCRIPTOR; i < MAX_DESCRIPTOR; i++)
+    fstat_status[i] = fstat (i, &s);
+}
+
 /* Check that are no files left open.  */
 
 static void
 check_open_files (void)
 {
-  int i;
-
-  for (i = 3; i < 10; i++)
+  for (unsigned i = MIN_DESCRIPTOR; i < MAX_DESCRIPTOR; i++)
     {
-      if (close (i) == 0)
+      if (fstat_status[i] != 0 && close (i) == 0)
 	{
 	  fprintf (stderr,
 		   "ERROR: descriptor %d still open after tests complete\n",
@@ -482,6 +496,8 @@ check_open_files (void)
 int
 main (int argc ATTRIBUTE_UNUSED, char **argv)
 {
+  check_available_files ();
+
   state = backtrace_create_state (argv[0], BACKTRACE_SUPPORTS_THREADS,
 				  error_callback_create, NULL);
 
-- 
2.33.1

Re: [PATCH] Handle jobserver file descriptors in btest.

[Ian Lance Taylor]

On Fri, Oct 22, 2021 at 1:15 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 10/21/21 20:15, Ian Lance Taylor wrote:
> > On Thu, Oct 21, 2021 at 12:48 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> The patch is about sensitive handling of file descriptors opened
> >> by make's jobserver.
> >
> > Thanks.  I think a better approach would be, at the start of main,
> > fstat the descriptors up to 10 and record the ones for which fstat
> > succeeds.  Then at the end of main only check the descriptors for
> > which fstat failed earlier.
>
> Sure, makes sense.
>
> >
> > I can work on that at some point if you don't want to tackle it.
>
> I've just done that in the attached patch.
>
> Is it fine?

This is OK.

Thanks.

Ian