Michal Hlavinka
2015-04-20 15:31:48 UTC
Hi,
some time ago, I've reported memory leak that occurs with aliases. This
leak was fixed, but it still occured when there was no space after alias.
For example:
alias lsl='ls -l'
a=`lsl `
b=`lsl`
where "a" did not leak, but "b" did. This was fixed by simple patch,
that added ' ' to the end of command substitution:
--- sh/macro.c.heresub 2014-05-21 16:48:42.650700910 +0200
+++ sh/macro.c 2014-05-21 16:48:42.678700772 +0200
@@ -2085,6 +2085,7 @@ static void comsubst(Mac_t *mp,register
}
sfputc(stkp,c);
}
+ sfputc(stkp,' ');
c = stktell(stkp);
str=stkfreeze(stkp,1);
/* disable verbose and don't save in history file */
but this fix caused a problem later with here documents, so it was
changed to sfputc(stkp,'\n'); but another problem occurred - with
missing ending quote. The parser tries to fix missing quote by adding
one at the end of the input. Which means that originally it would change
"abc to "abc" but with the \n it changes "abc to "abc\n" This causes
obfuscated error message. While syntax error message would be ok, the
error message now is file not found.
Reproducer:
$ touch abc
$ echo `ls "abc`
ls: abc
: not found
This leads to user confusion.
I was looking at the original memory leak and found that it is caused by
setupalias:
valgrind: 528 bytes in 3 blocks are still reachable in loss record 106
of 125
malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
sfnew (sfnew.c:97)
_sfopen (_sfopen.c:94)
setupalias (lex.c:2518)
sh_lex (lex.c:1541)
skipnl (parse.c:1654)
term (parse.c:564)
UnknownInlinedFun (parse.c:547)
sh_cmd (parse.c:497)
sh_parse (parse.c:386)
comsubst (macro.c:2099)
copyto (macro.c:600)
sh_mactrim (macro.c:183)
nv_setlist (name.c:339)
When input is read whole, fcfile set to zero and later in setupalias it
gets reinitialized with new stream:
# if(!(base=fcfile()))
# base = sfopen(NIL(Sfio_t*),fcseek(0),"s");
# fcclose();
# sfstack(base,iop);
# fcfopen(base);
but this new stream is never freed.
It seems to me that in sh_lex, the old stream is always available, it
just can be "forgotten"
setupalias happens either:
a) when stream is still linked in fcfile and in that case the setupalias
won't open new one
or
b) input was read all and sh_lex S_EOF removed the link, but it also
saved it in 'sp' variable.
I've tried to use this pointer later, if fcfile is zero and it fixed the
leak and did not cause any regression.
--- sh/lex.c.patched 2015-04-17 16:44:46.591326703 +0200
+++ sh/lex.c 2015-04-17 17:06:40.421379298 +0200
@@ -327,7 +327,7 @@ int sh_lex(Lex_t* lp)
Stk_t *stkp = shp->stk;
int inlevel=lp->lexd.level, assignment=0, ingrave=0;
int epatchar=0;
- Sfio_t *sp;
+ Sfio_t *sp=NULL;
#if SHOPT_MULTIBYTE
LEN=1;
#endif /* SHOPT_MULTIBYTE */
@@ -1538,6 +1538,8 @@ breakloop:
#endif /* KSHELL */
&& (state=nv_getval(np)))
{
+ if (!fcfile())
+ _Fcin._fcfile = sp;
setupalias(lp,state,np);
nv_onattr(np,NV_NOEXPAND);
lp->lex.reservok = 1;
Michal
some time ago, I've reported memory leak that occurs with aliases. This
leak was fixed, but it still occured when there was no space after alias.
For example:
alias lsl='ls -l'
a=`lsl `
b=`lsl`
where "a" did not leak, but "b" did. This was fixed by simple patch,
that added ' ' to the end of command substitution:
--- sh/macro.c.heresub 2014-05-21 16:48:42.650700910 +0200
+++ sh/macro.c 2014-05-21 16:48:42.678700772 +0200
@@ -2085,6 +2085,7 @@ static void comsubst(Mac_t *mp,register
}
sfputc(stkp,c);
}
+ sfputc(stkp,' ');
c = stktell(stkp);
str=stkfreeze(stkp,1);
/* disable verbose and don't save in history file */
but this fix caused a problem later with here documents, so it was
changed to sfputc(stkp,'\n'); but another problem occurred - with
missing ending quote. The parser tries to fix missing quote by adding
one at the end of the input. Which means that originally it would change
"abc to "abc" but with the \n it changes "abc to "abc\n" This causes
obfuscated error message. While syntax error message would be ok, the
error message now is file not found.
Reproducer:
$ touch abc
$ echo `ls "abc`
ls: abc
: not found
This leads to user confusion.
I was looking at the original memory leak and found that it is caused by
setupalias:
valgrind: 528 bytes in 3 blocks are still reachable in loss record 106
of 125
malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
sfnew (sfnew.c:97)
_sfopen (_sfopen.c:94)
setupalias (lex.c:2518)
sh_lex (lex.c:1541)
skipnl (parse.c:1654)
term (parse.c:564)
UnknownInlinedFun (parse.c:547)
sh_cmd (parse.c:497)
sh_parse (parse.c:386)
comsubst (macro.c:2099)
copyto (macro.c:600)
sh_mactrim (macro.c:183)
nv_setlist (name.c:339)
When input is read whole, fcfile set to zero and later in setupalias it
gets reinitialized with new stream:
# if(!(base=fcfile()))
# base = sfopen(NIL(Sfio_t*),fcseek(0),"s");
# fcclose();
# sfstack(base,iop);
# fcfopen(base);
but this new stream is never freed.
It seems to me that in sh_lex, the old stream is always available, it
just can be "forgotten"
setupalias happens either:
a) when stream is still linked in fcfile and in that case the setupalias
won't open new one
or
b) input was read all and sh_lex S_EOF removed the link, but it also
saved it in 'sp' variable.
I've tried to use this pointer later, if fcfile is zero and it fixed the
leak and did not cause any regression.
--- sh/lex.c.patched 2015-04-17 16:44:46.591326703 +0200
+++ sh/lex.c 2015-04-17 17:06:40.421379298 +0200
@@ -327,7 +327,7 @@ int sh_lex(Lex_t* lp)
Stk_t *stkp = shp->stk;
int inlevel=lp->lexd.level, assignment=0, ingrave=0;
int epatchar=0;
- Sfio_t *sp;
+ Sfio_t *sp=NULL;
#if SHOPT_MULTIBYTE
LEN=1;
#endif /* SHOPT_MULTIBYTE */
@@ -1538,6 +1538,8 @@ breakloop:
#endif /* KSHELL */
&& (state=nv_getval(np)))
{
+ if (!fcfile())
+ _Fcin._fcfile = sp;
setupalias(lp,state,np);
nv_onattr(np,NV_NOEXPAND);
lp->lex.reservok = 1;
Michal