Module Name: src
Committed By: rillig
Date: Mon Dec 13 05:25:04 UTC 2021
Modified Files:
src/usr.bin/make: main.c make.h parse.c str.c str.h
Log Message:
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.
To generate a diff of this commit:
cvs rdiff -u -r1.541 -r1.542 src/usr.bin/make/main.c
cvs rdiff -u -r1.270 -r1.271 src/usr.bin/make/make.h
cvs rdiff -u -r1.575 -r1.576 src/usr.bin/make/parse.c
cvs rdiff -u -r1.86 -r1.87 src/usr.bin/make/str.c
cvs rdiff -u -r1.13 -r1.14 src/usr.bin/make/str.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/make/main.c
diff -u src/usr.bin/make/main.c:1.541 src/usr.bin/make/main.c:1.542
--- src/usr.bin/make/main.c:1.541 Sat Aug 14 13:32:12 2021
+++ src/usr.bin/make/main.c Mon Dec 13 05:25:04 2021
@@ -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 @@ main_Init(int argc, char **argv)
/* default to writing debug to stderr */
opts.debug_file = stderr;
+ Str_Intern_Init();
HashTable_Init(&cached_realpaths);
#ifdef SIGINFO
Index: src/usr.bin/make/make.h
diff -u src/usr.bin/make/make.h:1.270 src/usr.bin/make/make.h:1.271
--- src/usr.bin/make/make.h:1.270 Sun Nov 28 23:12:51 2021
+++ src/usr.bin/make/make.h Mon Dec 13 05:25:04 2021
@@ -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 @@ typedef struct GNode {
* 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;
Index: src/usr.bin/make/parse.c
diff -u src/usr.bin/make/parse.c:1.575 src/usr.bin/make/parse.c:1.576
--- src/usr.bin/make/parse.c:1.575 Mon Dec 13 00:09:07 2021
+++ src/usr.bin/make/parse.c Mon Dec 13 05:25:04 2021
@@ -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 @@ MAKE_RCSID("$NetBSD: parse.c,v 1.575 202
* 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 @@ PrintStackTrace(void)
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 @@ static void
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 @@ Parse_Error(ParseErrorLevel type, const
lineno = 0;
} else {
IFile *curFile = CurFile();
- fname = curFile->fname;
+ fname = curFile->name.str;
lineno = (size_t)curFile->lineno;
}
@@ -2143,7 +2143,7 @@ IncludeFile(const char *file, bool isSys
* 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 @@ GetActuallyIncludingFile(void)
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 @@ Parse_PushInput(const char *name, int li
bool fromForLoop = name == NULL;
if (fromForLoop)
- name = CurFile()->fname;
+ name = CurFile()->name.str;
else
ParseTrackInput(name);
@@ -2426,7 +2426,7 @@ Parse_PushInput(const char *name, int li
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 @@ Parse_PushInput(const char *name, int li
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 @@ ParseEOF(void)
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 @@ ParseEOF(void)
curFile = CurFile();
DEBUG2(PARSE, "ParseEOF: returning to file %s, line %d\n",
- curFile->fname, curFile->lineno);
+ curFile->name.str, curFile->lineno);
- ParseSetParseFile(curFile->fname);
+ ParseSetParseFile(curFile->name.str);
return true;
}
Index: src/usr.bin/make/str.c
diff -u src/usr.bin/make/str.c:1.86 src/usr.bin/make/str.c:1.87
--- src/usr.bin/make/str.c:1.86 Mon Jun 21 16:59:18 2021
+++ src/usr.bin/make/str.c Mon Dec 13 05:25:04 2021
@@ -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_Match(const char *str, const char *p
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;
+}
Index: src/usr.bin/make/str.h
diff -u src/usr.bin/make/str.h:1.13 src/usr.bin/make/str.h:1.14
--- src/usr.bin/make/str.h:1.13 Sun Dec 12 23:39:34 2021
+++ src/usr.bin/make/str.h Mon Dec 13 05:25:04 2021
@@ -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 <ril...@netbsd.org>
@@ -343,3 +343,6 @@ char *str_concat2(const char *, const ch
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 *);
