[src/trunk]: src/usr.bin/make make: fix memory leak for filenames in .for loo...

[rillig <rillig%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/dabe985ab2e8
branches:  trunk
changeset: 1027619:dabe985ab2e8
user:      rillig <rillig%NetBSD.org@localhost>
date:      Mon Dec 13 05:25:04 2021 +0000

description:
make: fix memory leak for filenames in .for loops (since 2013-06-18)

Previously, each time a .for directive pushed its buffer on the input
file stack, the current filename was duplicated.  This was a waste of
memory.

The name of a file is typically only used while it is read in.  There is
one situation when the filename is needed for longer, which is when a
target is defined.

Since .for loops are implemented as a special form of included files,
each .for loop duplicated the current filename as well.

$ cat << EOF > for.mk
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.for i in 1 2 3 4 5 6 7 8 9 0
.endfor
.endfor
.endfor
.endfor
.endfor
.endfor
.endfor

all:
        @ps -o rsz -p ${.MAKE.PID}
EOF

$ make-2021.12.13.03.55.16 -r -f for.mk
  RSZ
10720

$ ./make -r -f for.mk
 RSZ
1716

The difference is 8 MB, which amounts to 1 million .for loops.

diffstat:

 usr.bin/make/main.c  |   5 +++--
 usr.bin/make/make.h  |   5 ++---
 usr.bin/make/parse.c |  32 +++++++++++++++-----------------
 usr.bin/make/str.c   |  21 +++++++++++++++++++--
 usr.bin/make/str.h   |   5 ++++-
 5 files changed, 43 insertions(+), 25 deletions(-)

diffs (223 lines):

diff -r 591604dd12c7 -r dabe985ab2e8 usr.bin/make/main.c
--- a/usr.bin/make/main.c       Mon Dec 13 04:18:01 2021 +0000
+++ b/usr.bin/make/main.c       Mon Dec 13 05:25:04 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: main.c,v 1.541 2021/08/14 13:32:12 rillig Exp $        */
+/*     $NetBSD: main.c,v 1.542 2021/12/13 05:25:04 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -111,7 +111,7 @@
 #include "trace.h"
 
 /*     "@(#)main.c     8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: main.c,v 1.541 2021/08/14 13:32:12 rillig Exp $");
+MAKE_RCSID("$NetBSD: main.c,v 1.542 2021/12/13 05:25:04 rillig Exp $");
 #if defined(MAKE_NATIVE) && !defined(lint)
 __COPYRIGHT("@(#) Copyright (c) 1988, 1989, 1990, 1993 "
            "The Regents of the University of California.  "
@@ -1346,6 +1346,7 @@
        /* default to writing debug to stderr */
        opts.debug_file = stderr;
 
+       Str_Intern_Init();
        HashTable_Init(&cached_realpaths);
 
 #ifdef SIGINFO
diff -r 591604dd12c7 -r dabe985ab2e8 usr.bin/make/make.h
--- a/usr.bin/make/make.h       Mon Dec 13 04:18:01 2021 +0000
+++ b/usr.bin/make/make.h       Mon Dec 13 05:25:04 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: make.h,v 1.270 2021/11/28 23:12:51 rillig Exp $        */
+/*     $NetBSD: make.h,v 1.271 2021/12/13 05:25:04 rillig Exp $        */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -435,8 +435,7 @@
         * everyone but the Suff module) */
        struct Suffix *suffix;
 
-       /* Filename where the GNode got defined */
-       /* XXX: What is the lifetime of this string? */
+       /* Filename where the GNode got defined, unlimited lifetime */
        const char *fname;
        /* Line number where the GNode got defined */
        int lineno;
diff -r 591604dd12c7 -r dabe985ab2e8 usr.bin/make/parse.c
--- a/usr.bin/make/parse.c      Mon Dec 13 04:18:01 2021 +0000
+++ b/usr.bin/make/parse.c      Mon Dec 13 05:25:04 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: parse.c,v 1.575 2021/12/13 00:09:07 rillig Exp $       */
+/*     $NetBSD: parse.c,v 1.576 2021/12/13 05:25:04 rillig Exp $       */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -109,7 +109,7 @@
 #include "pathnames.h"
 
 /*     "@(#)parse.c    8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.575 2021/12/13 00:09:07 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.576 2021/12/13 05:25:04 rillig Exp $");
 
 /* types and constants */
 
@@ -117,7 +117,7 @@
  * Structure for a file being read ("included file")
  */
 typedef struct IFile {
-       char *fname;            /* name of file (relative? absolute?) */
+       FStr name;              /* absolute or relative to the cwd */
        bool fromForLoop;       /* simulated .include by the .for loop */
        int lineno;             /* current line number in file */
        int first_lineno;       /* line number of start of text */
@@ -491,7 +491,7 @@
 
        for (i = n; i-- > 0;) {
                const IFile *entry = entries + i;
-               const char *fname = entry->fname;
+               const char *fname = entry->name.str;
                bool printLineno;
                char dirbuf[MAXPATHLEN + 1];
 
@@ -533,7 +533,7 @@
 RememberLocation(GNode *gn)
 {
        IFile *curFile = CurFile();
-       gn->fname = curFile->fname;
+       gn->fname = Str_Intern(curFile->name.str);
        gn->lineno = curFile->lineno;
 }
 
@@ -662,7 +662,7 @@
                lineno = 0;
        } else {
                IFile *curFile = CurFile();
-               fname = curFile->fname;
+               fname = curFile->name.str;
                lineno = (size_t)curFile->lineno;
        }
 
@@ -2143,7 +2143,7 @@
                 * we can locate the file.
                 */
 
-               incdir = bmake_strdup(CurFile()->fname);
+               incdir = bmake_strdup(CurFile()->name.str);
                slash = strrchr(incdir, '/');
                if (slash != NULL) {
                        *slash = '\0';
@@ -2322,7 +2322,7 @@
 
        for (i = includes.len; i >= 2; i--)
                if (!incs[i - 1].fromForLoop)
-                       return incs[i - 2].fname;
+                       return incs[i - 2].name.str;
        return NULL;
 }
 
@@ -2413,7 +2413,7 @@
        bool fromForLoop = name == NULL;
 
        if (fromForLoop)
-               name = CurFile()->fname;
+               name = CurFile()->name.str;
        else
                ParseTrackInput(name);
 
@@ -2426,7 +2426,7 @@
                return;
 
        curFile = Vector_Push(&includes);
-       curFile->fname = bmake_strdup(name);
+       curFile->name = FStr_InitOwn(bmake_strdup(name));
        curFile->fromForLoop = fromForLoop;
        curFile->lineno = lineno;
        curFile->first_lineno = lineno;
@@ -2441,8 +2441,7 @@
        buf = curFile->readMore(curFile->readMoreArg, &len);
        if (buf == NULL) {
                /* Was all a waste of time ... */
-               if (curFile->fname != NULL)
-                       free(curFile->fname);
+               FStr_Done(&curFile->name);
                free(curFile);
                return;
        }
@@ -2605,8 +2604,7 @@
                curFile->lf = NULL;
        }
 
-       /* Dispose of curFile info */
-       /* Leak curFile->fname because all the GNodes have pointers to it. */
+       FStr_Done(&curFile->name);
        free(curFile->buf_freeIt);
        Vector_Pop(&includes);
 
@@ -2621,9 +2619,9 @@
 
        curFile = CurFile();
        DEBUG2(PARSE, "ParseEOF: returning to file %s, line %d\n",
-           curFile->fname, curFile->lineno);
-
-       ParseSetParseFile(curFile->fname);
+           curFile->name.str, curFile->lineno);
+
+       ParseSetParseFile(curFile->name.str);
        return true;
 }
 
diff -r 591604dd12c7 -r dabe985ab2e8 usr.bin/make/str.c
--- a/usr.bin/make/str.c        Mon Dec 13 04:18:01 2021 +0000
+++ b/usr.bin/make/str.c        Mon Dec 13 05:25:04 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: str.c,v 1.86 2021/06/21 16:59:18 rillig Exp $  */
+/*     $NetBSD: str.c,v 1.87 2021/12/13 05:25:04 rillig Exp $  */
 
 /*
  * Copyright (c) 1988, 1989, 1990, 1993
@@ -71,7 +71,11 @@
 #include "make.h"
 
 /*     "@(#)str.c      5.8 (Berkeley) 6/1/90"  */
-MAKE_RCSID("$NetBSD: str.c,v 1.86 2021/06/21 16:59:18 rillig Exp $");
+MAKE_RCSID("$NetBSD: str.c,v 1.87 2021/12/13 05:25:04 rillig Exp $");
+
+
+static HashTable interned_strings;
+
 
 /* Return the concatenation of s1 and s2, freshly allocated. */
 char *
@@ -395,3 +399,16 @@
                str++;
        }
 }
+
+void
+Str_Intern_Init(void)
+{
+       HashTable_Init(&interned_strings);
+}
+
+/* Return a canonical instance of str, with unlimited lifetime. */
+const char *
+Str_Intern(const char *str)
+{
+       return HashTable_CreateEntry(&interned_strings, str, NULL)->key;
+}
diff -r 591604dd12c7 -r dabe985ab2e8 usr.bin/make/str.h
--- a/usr.bin/make/str.h        Mon Dec 13 04:18:01 2021 +0000
+++ b/usr.bin/make/str.h        Mon Dec 13 05:25:04 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: str.h,v 1.13 2021/12/12 23:39:34 rillig Exp $  */
+/*     $NetBSD: str.h,v 1.14 2021/12/13 05:25:04 rillig Exp $  */
 
 /*
  Copyright (c) 2021 Roland Illig <rillig%NetBSD.org@localhost>
@@ -343,3 +343,6 @@
 char *str_concat3(const char *, const char *, const char *);
 
 bool Str_Match(const char *, const char *);
+
+void Str_Intern_Init(void);
+const char *Str_Intern(const char *);