* Segfault on =() expansion @ 2008-08-31 8:39 Mikael Magnusson 2008-09-01 20:13 ` Peter Stephenson 0 siblings, 1 reply; 3+ messages in thread From: Mikael Magnusson @ 2008-08-31 8:39 UTC (permalink / raw) To: zsh-workers I could reproduce this on all my remote ssh shells, ranging from 4.2.3 to my local current cvs. % a==(echo foo); : & cat $a [1] 14645 foo % [1] + done : % a==(echo foo); : & cat $a zsh: segmentation fault zsh -f Let's see if I can get a backtrace... *waits for recompile*. Program received signal SIGSEGV, Segmentation fault. 0x0806d364 in untokenize (s=0x0) at exec.c:1639 1639 if (*s) { (gdb) bt full #0 0x0806d364 in untokenize (s=0x0) at exec.c:1639 No locals. #1 0x0806e224 in addvars (state=0xafc54c5c, pc=0xa7bd35e0, addflags=0) at exec.c:2068 pm = (Param) 0x80ac572 val = 0xc <Address 0xc out of bounds> allexp = 0 myflags = 0 vl = (LinkList) 0xafc54b34 xtr = 0 isstr = 1 htok = 1 arr = (char **) 0xafc54bb0 ptr = (char **) 0x0 name = 0xa7bd3660 "a" flags = 0 opc = (Wordcode) 0xa7bd35e4 ac = 5 svl = {list = {first = 0xafc54b28, last = 0xafc54b28, flags = 0}, node = { next = 0xafc54b28, prev = 0xafc54b28, dat = 0x0}} __n0 = {next = 0x0, prev = 0xafc54b34, dat = 0x0} #2 0x0806b7c1 in execsimple (state=0xafc54c5c) at exec.c:992 code = 5 lv = 1 #3 0x0806ba68 in execlist (state=0xafc54c5c, dont_change_job=0, exiting=0) at exec.c:1094 donedebug = 0 next = (Wordcode) 0xa7bd35f0 code = 11329 ret = 1103269876 cj = -1 ---Type <return> to continue, or q <return> to quit--- csp = 0 ltype = 354 old_pline_level = 0 old_list_pipe = 0 oldlineno = 4 oldnoerrexit = 0 donetrap = 0 #4 0x0806b6f5 in execode (p=0xa7bd35b0, dont_change_job=0, exiting=0) at exec.c:965 s = {prog = 0xa7bd35b0, pc = 0xa7bd35ec, strs = 0xa7bd3620 "\213\210echo foo\211"} #5 0x08084a7c in loop (toplevel=1, justonce=0) at init.c:181 toksav = 1 prog = (Eprog) 0xa7bd35b0 #6 0x080874e2 in zsh_main (argc=2, argv=0xafc54d94) at init.c:1407 t = (char **) 0xafc54d9c t0 = 157 #7 0x080551f6 in main (argc=Cannot access memory at address 0x48 ) at ./main.c:93 No locals. Changing (*s) to (s && *s) fixes the crash, but maybe something else is fishy. With that change, $a appears to be empty instead of the crash. % a==(echo foo); : & echo $a [1] 18053 /tmp/zsh5NP27e % [1] + done : % a==(echo foo); : & echo $a [1] 18054 % [1] + done : -- Mikael Magnusson ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Segfault on =() expansion 2008-08-31 8:39 Segfault on =() expansion Mikael Magnusson @ 2008-09-01 20:13 ` Peter Stephenson 2008-09-01 20:24 ` Mikael Magnusson 0 siblings, 1 reply; 3+ messages in thread From: Peter Stephenson @ 2008-09-01 20:13 UTC (permalink / raw) To: zsh-workers On Sun, 31 Aug 2008 10:39:35 +0200 "Mikael Magnusson" <mikachu@gmail.com> wrote: > I could reproduce this on all my remote ssh shells, ranging from 4.2.3 > to my local current cvs. > > % a==(echo foo); : & cat $a > [1] 14645 > foo > % > [1] + done : > % a==(echo foo); : & cat $a > zsh: segmentation fault zsh -f This was quite involved (yes, I know, just for once...) Working backwards, 1. In the second assignment, the list node's data was NULL. This shouldn't happen, so caused the crash when processing it as a string. 2. The data was NULL because =(...) expansion failed. 3. That failed because "thisjob" was -1. 4. thisjob was -1 for two reasons. a. It didn't get set for the current job because the current command was parsed as "simple", so it went through execsimple(), missing out a lot of steps including all job handling. b. In this case, the execution of the background job had reset thisjob to -1. That's why it didn't happen the first time. 5. The assignment was parsed as simple because all assignments are. "Simple" means everything happens entirely in the shell and won't use job handling: this isn't true here because process substitutions need to have a job to attach the temporary file to. The fixes are: to 5: assignments with process substitutions can't be simple. to 2: on failure of process substitutions (and others) set the data to an empty string. (I wonder, actually, if 4b. needs unfixing, i.e. thisjob should be made to default to -1.) This will cause the "cat" in the test to give an error. This is the correct behaviour: the file generated by the =(...) substition is a temporary file (this is clearly documented) that only lasts for the length of the current command, which is just the assignment. So actually process substitutions aren't useful for assignments, which is probably why this hasn't shown up before. I thought about being safe and detecting $(...)'s, too, but that caused this: myfd=99 (echo >&$myfd) 2>msg bad_fd_msg="${$(<msg)##*:}" to return a non-zero status. That's because when assignments are handled by execcmd() rather than execsimple() we don't set the status for the command, so it retains the previous status. I've fixed that, but in the end I decided just to restrict the fix to 5. to process substitution since we'd know by now if it was necessary for command substitution. Index: Src/exec.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/exec.c,v retrieving revision 1.144 diff -u -r1.144 exec.c --- Src/exec.c 31 Aug 2008 16:01:11 -0000 1.144 +++ Src/exec.c 1 Sep 2008 19:58:47 -0000 @@ -2220,6 +2220,7 @@ doneps4 = 0; redir = (wc_code(*state->pc) == WC_REDIR ? ecgetredirs(state) : NULL); if (wc_code(*state->pc) == WC_ASSIGN) { + cmdoutval = 0; varspc = state->pc; while (wc_code((code = *state->pc)) == WC_ASSIGN) state->pc += (WC_ASSIGN_TYPE(code) == WC_ASSIGN_SCALAR ? @@ -2236,6 +2237,12 @@ * they don't modify their argument strings. */ args = (type == WC_SIMPLE ? ecgetlist(state, WC_SIMPLE_ARGC(code), EC_DUP, &htok) : NULL); + /* + * If assignment but no command get the status from variable + * assignment. + */ + if (!args && varspc) + lastval = errflag ? errflag : cmdoutval; for (i = 0; i < 10; i++) { save[i] = -2; Index: Src/parse.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/parse.c,v retrieving revision 1.72 diff -u -r1.72 parse.c --- Src/parse.c 31 Aug 2008 19:50:49 -0000 1.72 +++ Src/parse.c 1 Sep 2008 19:58:48 -0000 @@ -1567,6 +1567,18 @@ str = p + 1; } else equalsplit(tokstr, &str); + for (p = str; *p; p++) { + /* + * We can't treat this as "simple" if it contains + * expansions that require process subsitution, since then + * we need process handling. + */ + if (p[1] == Inpar && + (*p == Equals || *p == Inang || *p == Outang)) { + *complex = 1; + break; + } + } ecstr(name); ecstr(str); isnull = 0; Index: Src/subst.c =================================================================== RCS file: /cvsroot/zsh/zsh/Src/subst.c,v retrieving revision 1.85 diff -u -r1.85 subst.c --- Src/subst.c 12 May 2008 13:50:42 -0000 1.85 +++ Src/subst.c 1 Sep 2008 19:58:49 -0000 @@ -66,6 +66,7 @@ else setdata(node, (void *) getoutputfile(str)); /* =(...) */ if (!getdata(node)) { + setdata(node, dupstring("")); unqueue_signals(); return; } -- Peter Stephenson <p.w.stephenson@ntlworld.com> Web page now at http://homepage.ntlworld.com/p.w.stephenson/ ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Segfault on =() expansion 2008-09-01 20:13 ` Peter Stephenson @ 2008-09-01 20:24 ` Mikael Magnusson 0 siblings, 0 replies; 3+ messages in thread From: Mikael Magnusson @ 2008-09-01 20:24 UTC (permalink / raw) To: zsh-workers 2008/9/1 Peter Stephenson <p.w.stephenson@ntlworld.com>: > On Sun, 31 Aug 2008 10:39:35 +0200 > "Mikael Magnusson" <mikachu@gmail.com> wrote: >> I could reproduce this on all my remote ssh shells, ranging from 4.2.3 >> to my local current cvs. >> >> % a==(echo foo); : & cat $a >> [1] 14645 >> foo >> % >> [1] + done : >> % a==(echo foo); : & cat $a >> zsh: segmentation fault zsh -f > > This was quite involved (yes, I know, just for once...) Working backwards, I am somehow not overly surprised by this :). > 1. In the second assignment, the list node's data was NULL. This shouldn't > happen, so caused the crash when processing it as a string. > 2. The data was NULL because =(...) expansion failed. > 3. That failed because "thisjob" was -1. > 4. thisjob was -1 for two reasons. > a. It didn't get set for the current job because the current command was > parsed as "simple", so it went through execsimple(), missing out a > lot of steps including all job handling. > b. In this case, the execution of the background job had reset thisjob > to -1. That's why it didn't happen the first time. > 5. The assignment was parsed as simple because all assignments are. > "Simple" means everything happens entirely in the shell and won't > use job handling: this isn't true here because process substitutions > need to have a job to attach the temporary file to. > > The fixes are: > to 5: assignments with process substitutions can't be simple. > to 2: on failure of process substitutions (and others) set the data to > an empty string. > > (I wonder, actually, if 4b. needs unfixing, i.e. thisjob should be made to > default to -1.) > > This will cause the "cat" in the test to give an error. This is the > correct behaviour: the file generated by the =(...) substition is a > temporary file (this is clearly documented) that only lasts for the > length of the current command, which is just the assignment. So > actually process substitutions aren't useful for assignments, which is > probably why this hasn't shown up before. Right, that's what i wanted to confirm just seconds before finding the bug, I was quite surprised to find the temp file hadn't been deleted, though in my case it was sort of what i wanted :). (what i want can also be achieved with a shell function, i'm just very lazy). > I thought about being safe and detecting $(...)'s, too, but that caused this: > > myfd=99 > (echo >&$myfd) 2>msg > bad_fd_msg="${$(<msg)##*:}" > > to return a non-zero status. That's because when assignments are > handled by execcmd() rather than execsimple() we don't set the status > for the command, so it retains the previous status. I've fixed that, > but in the end I decided just to restrict the fix to 5. to process > substitution since we'd know by now if it was necessary for command > substitution. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-09-01 20:25 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-08-31 8:39 Segfault on =() expansion Mikael Magnusson 2008-09-01 20:13 ` Peter Stephenson 2008-09-01 20:24 ` Mikael Magnusson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).