two bugs in construct_command_argv_internal() (job.c, latest CVS)

[Toomas Rosin]

Hello,

I had two problems with job.c, and consider them bugs, at least the
second one.  (I admit I don't quite understand what was going on in
the first case.)

The problems were with construct_command_argv_internal() when supplied
a large LINE argument consisting of many lines.  My system:

   [20:04:32] address@hidden:/usr/src/tmp/make.build $ uname -a
   Linux ns.tklabor.ee 2.4.20 #2 Mon Mar 17 15:40:04 EET 2003 i686 GNU/Linux
   [20:04:37] address@hidden:/usr/src/tmp/make.build $ gcc --version | head -1
   gcc (GCC) 3.3.2 20030831 (Debian prerelease)
   [20:04:40] address@hidden:/usr/src/tmp/make.build $ ulimit -s -d -v
   stack size            (kbytes, -s) unlimited
   data seg size         (kbytes, -d) unlimited
   virtual memory        (kbytes, -v) unlimited
   [20:15:57] address@hidden:/usr/src/tmp/make.build $ head -3 /proc/meminfo
           total:    used:    free:  shared: buffers:  cached:
   Mem:  64688128 55259136  9428992        0  6201344 31764480
   Swap: 575696896 54808576 520888320

The first problem:

If the length of LINE was more than about 1042500 bytes, `make' got
SIGSEGV.  (alloca(), on line 3082, allocates twice this much for
new_line.)  This went away after substituting xmalloc() for alloca():

--- job.c.orig  2003-09-10 18:44:51.000000000 +0300
+++ job.c       2003-09-10 19:05:06.000000000 +0300
@@ -3079,7 +3079,7 @@
 #endif
     unsigned int line_len = strlen (line);
 
-    char *new_line = (char *) alloca (shell_len + (sizeof (minus_c) - 1)
+    char *new_line = (char *) xmalloc (shell_len + (sizeof (minus_c) - 1)
                                      + (line_len * 2) + 1);
     char *command_ptr = NULL; /* used for batch_mode_shell mode */
 
@@ -3141,8 +3141,11 @@
        *ap++ = *p;
       }
     if (ap == new_line + shell_len + sizeof (minus_c) - 1)
-      /* Line was empty.  */
-      return 0;
+      {
+        /* Line was empty.  */
+        free (new_line);
+        return 0;
+      }
     *ap = '\0';
 
 #ifdef WINDOWS32
@@ -3317,6 +3320,7 @@
       fatal (NILF, _("%s (line %d) Bad shell context (!unixy && 
!batch_mode_shell)\n"),
             __FILE__, __LINE__);
 #endif
+    free (new_line);
   }
 #endif /* ! AMIGA */
 
That fixed, another problem immediately stroke me: when invoked on the
same makefile, `make' ran out of memory.  It turned out that the same
construct_command_argv_internal() function had a memory leak, which is
probably small in most cases, but which grew huge in this specific
case.  The significant lines are:

  2794:  new_argv = (char **) xmalloc (i * sizeof (char *));
  ...
  3003:  if (new_argv[0] == 0)
  3004:    /* Line was empty.  */
  3005:    return 0;
  3006:  else
  3007:    return new_argv;

The `new_argv' allocated on line 2794 is always freed by the caller,
except when construct_command_argv_internal() returns zero, but this
happens as soon as LINE begins with '\n' (preceded by any number of
blanks).  I discovered that the LINE I had trouble with, which
contained nearly 3000 lines, had (accidentally) a blank line after
every 3-4 non-blank lines.  On encountering one of these blank lines,
several MB of memory was thrown away each time, until there was
nothing left.

This could be fixed by freeing new_argv before returning 0, but there
is another and, I think, better solution -- to skip blank lines even
before allocating new_argv:

--- job.c.1     2003-09-10 19:05:06.000000000 +0300
+++ job.c       2003-09-10 19:42:53.000000000 +0300
@@ -2724,7 +2724,7 @@
     *restp = NULL;
 
   /* Make sure not to bother processing an empty line.  */
-  while (isblank ((unsigned char)*line))
+  while (isblank ((unsigned char)*line) || *line == '\n')
     ++line;
   if (*line == '\0')
     return 0;

After this change, I can't see how `make' could arrive at line 3005,
unless the LINE argument consists only of blank lines.  At least I
have not experienced any more problems.

Have a nice day,
Toomas.