Zsh Mailing List Archive
Messages sorted by:
Reverse Date,
Date,
Thread,
Author
[PATCH] fix several memory leaks
Here is a list of memory leaks I found and their possible
fixes. Please check the patches at the end of the post,
especially those for init.c and termcap.c/terminfo.c.
I used a command line tool named "leaks" on macOS to find
thease leaks, but I believe they can also be found (or at
least confirmed) by valgrind.
[1] found in an interactive zsh (fix: compcore.c)
zsh% zmodload -U compinit; compinit
zsh% setopt list_packed
zsh% (do some completeions, e.g., ls <TAB> etc.)
Leak: 0x7fd2adc11f50 size=320 zone: DefaultMallocZone_0x105777000
0x0000000f 0x0000000e 0x00000017 0x00000012 ................
0x0000000d 0x00000012 0x00000008 0x00000000 ................
(snip)
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
runhookdef module.c:1007 | list_matches compresult.c:2317 |
runhookdef module.c:1007 | complistmatches complist.c:2017 |
calclist compresult.c:1706 | zalloc mem.c:966 | malloc
If list_packed is set, calclist() (compresult.c:1706) allocates
'g->widths' for keeping the widths of the columns in the completion
listing. freematches() (comcpore.c) must free this also.
[2] found in B07emulate.ztst (init.c)
For leaks found during "make check", I will show a simple example
code to reproduce the same leak and the corresponding Call stack.
zsh% emulate zsh -c 'true'
zsh% emulate zsh -c 'true'
Leak: 0x7fea8252ff40 size=16 zone: DefaultMallocZone_0x10635a000 length: 3 "zsh"
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
execbuiltin builtin.c:505 | bin_emulate builtin.c:5960 |
parseopts init.c:462 | ztrdup string.c:83 | zalloc mem.c:966 | malloc
The first 'emulate' allocates "zsh" at init.c:462:
scriptname = scriptfilename = ztrdup("zsh");
The second 'emulate' does not free it and allocate "zsh" again.
I guess we need to allocate 'scriptname' only if 'toplevel' is true.
In the case of 'emulate zsh -c cmd', the 'cmd' will be passed to eval(),
and eval() save/set/restore 'scriptname' by itself, so there is no point
of setting 'scriptname' here, I think.
[3] C01arith.ztst (math.c)
zsh% : $(( 1_0e0 ))
Leak: 0x7fafffc09200 size=16 zone: DefaultMallocZone_0x104c91000 length: 5 "10e0 "
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
matheval math.c:1464 | mathevall math.c:415 | mathparse math.c:1559 |
zzlex math.c:824 | lexconstant math.c:538 |
ztrdup string.c:83 | zalloc mem.c:966 | malloc
The string allocated at math.c:538 is a temporary string (ptr is reset
to nptr at line 556) and can be in heap.
[4] D04parameter.ztst (subst.c)
zsh% a=1; b=2; echo ${a:^b}
Leak: 0x7f995de00da0 size=16 zone: DefaultMallocZone_0x10543f000
0x5de006a0 0x00007f99 0x00000000 0x00000000 ...]............
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
stringsubst subst.c:322 | paramsubst subst.c:3172 |
mkarray utils.c:3945 | zalloc mem.c:966 | malloc
The array allocated at subst.c:3172 can also be in heap.
[5] V07pcre.ztst (pcre.c)
zsh% zmodload zsh/pcre
zsh% pcre_compile 'a'
zsh% pcre_match 'a'
Leak: 0x7fa3f250b7f0 size=16 zone: DefaultMallocZone_0x10f411000 length: 1 "a"
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
execbuiltin builtin.c:505 | bin_pcre_match pcre.c:362 |
ztrdup string.c:83 | zalloc mem.c:966 | malloc
'plaintext' allocated at pcre.c:362 should be freed.
[6] V11db_gdbm.ztst (db_gdbm.c): three types of leaks in this test
(a) The first one is:
zsh% zmodload zsh/db/gdbm
zsh% ztie -d db/gdbm -f test.gdbm dbase
zsh% zuntie dbase
Leak: 0x7fc914f00840 size=16 zone: DefaultMallocZone_0x102eea000 length: 5 "dbase"
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
bin_ztie db_gdbm.c:196 | append_tied_name db_gdbm.c:705 |
ztrdup string.c:83 | zalloc mem.c:966 | malloc
The string allocated at db_gdbm.c:705 is not freed.
The patch adds free() in remove_tied_name().
(b) The other two are:
zsh% zmodload zsh/db/gdbm
zsh% ztie -d db/gdbm -f test.gdbm dbase
zsh% dbase[key]=value
Leak: 0x7f8e83423620 size=16 zone: DefaultMallocZone_0x1047a3000 length: 5 "value"
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
addvars exec.c:2449 | ztrdup string.c:83 | zalloc mem.c: 966 | malloc
and
zsh% zmodload zsh/db/gdbm
zsh% ztie -d db/gdbm -f test.gdbm dbase
zsh% dbase=( a a )
zsh% dbase+=( a b )
Leak: 0x7fe681e34fa0 size=16 zone: DefaultMallocZone_0x10a3c5000 length: 1 "b"
Call stack: [thread 0x7fffdf6f93c0]: | start | main .main.c:93 |
(snip)
addvars exec.c:2487 | ztrdup string.c:83 | zalloc mem.c:966 | malloc
These two have the same origin.
I think gdbmsetfn() can just save the pointer 'val' instead of
creating a copy (db_gdbm.c:362). In params.c, strsetfn() or
strvarsetfn() also saves the pointer 'x' without making a copy.
[7] V01zmodload.ztst (part1: termcap.c/terminfo.c)
There are two types of leak. The first type of them can be
reproduce by several ways, for example
zsh% zmodload zsh/termcap
zsh% zmodload zsh/terminfo
or
zsh% zmodload zsh/termcap
zsh% zmodload -u zsh/termcap
zsh% zmodload zsh/termcap
Then may leaks are reported in the ncurses library functions.
In these modules boot_() calls setupterm(), which then allocates
memory for 'cur_term'. I think this must be freed in cleanup_()
by calling del_curterm(cur_term). Moreover, it seems the two modules
share the same 'cur_term' (a ncurses global pointer), and if
termcap has been already loaded when terminfo is being loaded,
the latter allocates another memory for 'cur_term' without
freeing the previous data. The patch below calls setupterm()
only if 'cur_term" is NULL, but I'm not 100% sure this is the
correct fix.
V01zmodload.ztst has another type of leaks, but I will discuss
it in a separate post because how to fix it (or whether it must
be fixed or not) is not clear.
diff --git a/Src/Modules/db_gdbm.c b/Src/Modules/db_gdbm.c
index 5f776f407..ed702b912 100644
--- a/Src/Modules/db_gdbm.c
+++ b/Src/Modules/db_gdbm.c
@@ -359,7 +359,7 @@ gdbmsetfn(Param pm, char *val)
}
if (val) {
- pm->u.str = ztrdup(val);
+ pm->u.str = val;
pm->node.flags |= PM_UPTODATE;
}
@@ -732,6 +732,9 @@ static int remove_tied_name( const char *name ) {
p++;
}
+ if (*p)
+ zsfree(*p);
+
/* Copy x+1 to x */
while (*p) {
*p=*(p+1);
diff --git a/Src/Modules/pcre.c b/Src/Modules/pcre.c
index 15ee34bc8..6289e003e 100644
--- a/Src/Modules/pcre.c
+++ b/Src/Modules/pcre.c
@@ -380,6 +380,7 @@ bin_pcre_match(char *nam, char **args, Options ops, UNUSED(int func))
if (ovec)
zfree(ovec, ovecsize*sizeof(int));
+ zsfree(plaintext);
return return_value;
}
diff --git a/Src/Modules/termcap.c b/Src/Modules/termcap.c
index 60a6e138a..71257c5e9 100644
--- a/Src/Modules/termcap.c
+++ b/Src/Modules/termcap.c
@@ -353,7 +353,8 @@ boot_(UNUSED(Module m))
* mean the modules hasn't booted---TERM may change,
* and it should be handled dynamically---so ignore errors here.
*/
- (void)setupterm((char *)0, 1, &errret);
+ if (!cur_term) /* if zsh/terminfo is not loaded */
+ (void)setupterm((char *)0, 1, &errret);
# endif
#endif
return 0;
@@ -363,6 +364,7 @@ boot_(UNUSED(Module m))
int
cleanup_(Module m)
{
+ del_curterm(cur_term);
return setfeatureenables(m, &module_features, NULL);
}
diff --git a/Src/Modules/terminfo.c b/Src/Modules/terminfo.c
index bbd325899..7a2ef0a6f 100644
--- a/Src/Modules/terminfo.c
+++ b/Src/Modules/terminfo.c
@@ -346,7 +346,8 @@ boot_(UNUSED(Module m))
* mean the modules hasn't booted---TERM may change,
* and it should be handled dynamically---so ignore errors here.
*/
- (void)setupterm((char *)0, 1, &errret);
+ if (!cur_term) /* if zsh/termcap is not loaded */
+ (void)setupterm((char *)0, 1, &errret);
# endif
#endif
@@ -357,6 +358,7 @@ boot_(UNUSED(Module m))
int
cleanup_(Module m)
{
+ del_curterm(cur_term);
return setfeatureenables(m, &module_features, NULL);
}
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index fd415da89..8eca39447 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -3556,6 +3556,8 @@ freematches(Cmgroup g, int cm)
}
free(g->expls);
}
+ if (g->widths)
+ free(g->widths);
zsfree(g->name);
free(g);
diff --git a/Src/init.c b/Src/init.c
index c5372665a..e9e6be9b4 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -459,7 +459,8 @@ parseopts(char *nam, char ***argvp, char *new_opts, char **cmdp,
/* -c command */
*cmdp = *argv;
new_opts[INTERACTIVE] &= 1;
- scriptname = scriptfilename = ztrdup("zsh");
+ if (toplevel)
+ scriptname = scriptfilename = ztrdup("zsh");
} else if (**argv == 'o') {
if (!*++*argv)
argv++;
diff --git a/Src/math.c b/Src/math.c
index 32bccc6e9..4b7ecf0ab 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -535,7 +535,7 @@ lexconstant(void)
for (ptr2 = ptr; ptr2 < nptr; ptr2++) {
if (*ptr2 == '_') {
int len = nptr - ptr;
- ptr = ztrdup(ptr);
+ ptr = dupstring(ptr);
for (ptr2 = ptr; len; len--) {
if (*ptr2 == '_')
chuck(ptr2);
diff --git a/Src/subst.c b/Src/subst.c
index a265a187e..c1021fbf3 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -3169,7 +3169,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
zip = hmkarray(sval);
}
if (!isarr) {
- aval = mkarray(val);
+ aval = hmkarray(val);
isarr = 1;
}
if (zip) {
Messages sorted by:
Reverse Date,
Date,
Thread,
Author