* Crash when exporting scalar without value and getsparam fails @ 2015-01-09 13:26 Mikael Magnusson 2015-01-10 6:52 ` Bart Schaefer 0 siblings, 1 reply; 11+ messages in thread From: Mikael Magnusson @ 2015-01-09 13:26 UTC (permalink / raw) To: zsh workers 2086 if (!(pm->node.flags & PM_UNSET) && !pm->env && !value) 30. returned_null: getsparam returns null (checked 46 out of 57 times). [show details] CID 439066 (#1 of 1): Dereference null return value (NULL_RETURNS)31. dereference: Dereferencing a pointer that might be null getsparam(pname) when calling addenv. [show details] 2087 addenv(pm, getsparam(pname)); The issue has this comment by Oliver on it, but I couldn't find any thread about it with a cursory search, so I'm repeating it here; Comment: The line of code occurs when you export an existing scalar variable without giving a new value. It isn't easy to find ways of making getsparam to fail but I've found one which is enough to declare this a bug: unset IFS export IFS memory allocation failing, such as dupstring would also do the job. Arguably these cases need fixing but the code highlighted here should perhaps default to adding the variable to the environment with an empty value. and sure enough, zsh: segmentation fault (core dumped) zsh -fc 'unset IFS; export IFS' Would the non-gnu equivalent of addenv(pm, getsparam(pname) ?: ""); do the trick here? Or should it just return with an error? I guess if we do the former, then the environment would get out of sync with the internal parameter value. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-09 13:26 Crash when exporting scalar without value and getsparam fails Mikael Magnusson @ 2015-01-10 6:52 ` Bart Schaefer 2015-01-10 7:56 ` Mikael Magnusson 0 siblings, 1 reply; 11+ messages in thread From: Bart Schaefer @ 2015-01-10 6:52 UTC (permalink / raw) To: zsh workers On Jan 9, 2:26pm, Mikael Magnusson wrote: } bug: } unset IFS } export IFS } } Would the non-gnu equivalent of addenv(pm, getsparam(pname) ?: ""); do } the trick here? Or should it just return with an error? I think it should do nothing to the environment and return success. % bash -c 'unset IFS; export IFS; echo $?; printenv | grep IFS' 0 % However, we need an unset parameter object with the export flag set, so that it will become both set and exported when next a value is assigned to it. I think that can be done but don't immediately recall how. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 6:52 ` Bart Schaefer @ 2015-01-10 7:56 ` Mikael Magnusson 2015-01-10 8:09 ` Mikael Magnusson 2015-01-10 16:57 ` Bart Schaefer 0 siblings, 2 replies; 11+ messages in thread From: Mikael Magnusson @ 2015-01-10 7:56 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sat, Jan 10, 2015 at 7:52 AM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 9, 2:26pm, Mikael Magnusson wrote: > } bug: > } unset IFS > } export IFS > } > } Would the non-gnu equivalent of addenv(pm, getsparam(pname) ?: ""); do > } the trick here? Or should it just return with an error? > > I think it should do nothing to the environment and return success. > > % bash -c 'unset IFS; export IFS; echo $?; printenv | grep IFS' > 0 > % > > However, we need an unset parameter object with the export flag set, > so that it will become both set and exported when next a value is > assigned to it. I think that can be done but don't immediately > recall how. Ah yes, this is what happens for other parameters in zsh too. % unset foo % export foo % printenv|grep foo % foo=3 % printenv|grep foo foo=3 -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 7:56 ` Mikael Magnusson @ 2015-01-10 8:09 ` Mikael Magnusson 2015-01-10 17:25 ` Bart Schaefer 2015-01-10 16:57 ` Bart Schaefer 1 sibling, 1 reply; 11+ messages in thread From: Mikael Magnusson @ 2015-01-10 8:09 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sat, Jan 10, 2015 at 8:56 AM, Mikael Magnusson <mikachu@gmail.com> wrote: > On Sat, Jan 10, 2015 at 7:52 AM, Bart Schaefer > <schaefer@brasslantern.com> wrote: >> On Jan 9, 2:26pm, Mikael Magnusson wrote: >> } bug: >> } unset IFS >> } export IFS >> } >> } Would the non-gnu equivalent of addenv(pm, getsparam(pname) ?: ""); do >> } the trick here? Or should it just return with an error? >> >> I think it should do nothing to the environment and return success. >> >> % bash -c 'unset IFS; export IFS; echo $?; printenv | grep IFS' >> 0 >> % >> >> However, we need an unset parameter object with the export flag set, >> so that it will become both set and exported when next a value is >> assigned to it. I think that can be done but don't immediately >> recall how. > > Ah yes, this is what happens for other parameters in zsh too. > % unset foo > % export foo > % printenv|grep foo > % foo=3 > % printenv|grep foo > foo=3 So just to check, I tried both of the things anyway. With setting it to "" on failure, it gets exported as the empty string, and this if (!(pm->node.flags & PM_UNSET) && !pm->env && !value) { void *foo = getsparam(pname); if (foo) addenv(pm, foo); } results in almost the correct behaviour, except that after unsetting the parameter and setting it again, it still gets exported. (This doesn't happen for other parameters). Maybe bin_unset or unsetparam_pm has some special code that needs changing too? -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 8:09 ` Mikael Magnusson @ 2015-01-10 17:25 ` Bart Schaefer 2015-01-10 18:28 ` Bart Schaefer 0 siblings, 1 reply; 11+ messages in thread From: Bart Schaefer @ 2015-01-10 17:25 UTC (permalink / raw) To: zsh workers On Jan 10, 9:09am, Mikael Magnusson wrote: } Subject: Re: Crash when exporting scalar without value and getsparam fails } } So just to check, I tried both of the things anyway. With setting it } to "" on failure, it gets exported as the empty string, and this } if (!(pm->node.flags & PM_UNSET) && !pm->env && !value) { } void *foo = getsparam(pname); } if (foo) } addenv(pm, foo); } } } } results in almost the correct behaviour, except that after unsetting } the parameter and setting it again, it still gets exported. (This } doesn't happen for other parameters). Maybe bin_unset or unsetparam_pm } has some special code that needs changing too? I think the whole problem is that the PM_UNSET flag is for some reason not set, even though the parameter is in fact unset. This is happening at line 2075: pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~(off | PM_UNSET); That line is never executed for e.g. "export FOO", because of this up at line 1937: /* * We need to compare types with an existing pm if special, * even if that's unset */ if (pm && (pm->node.flags & PM_SPECIAL)) usepm = 1; Consequently ... diff --git a/Src/builtin.c b/Src/builtin.c index 8abe728..8dee8f9 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -1935,7 +1935,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), * even if that's unset */ if (pm && (pm->node.flags & PM_SPECIAL)) - usepm = 1; + usepm = 2; /* indicate that we preserve the PM_UNSET flag */ /* * Don't use an existing param if @@ -2072,7 +2072,11 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), arrfixenv(pm->node.nam, x); } } - pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~(off | PM_UNSET); + if (usepm == 2) /* do not change the PM_UNSET flag */ + pm->node.flags = (pm->node.flags | (on & ~PM_READONLY)) & ~off; + else + pm->node.flags = (pm->node.flags | + (on & ~PM_READONLY)) & ~(off | PM_UNSET); if (on & (PM_LEFT | PM_RIGHT_B | PM_RIGHT_Z)) { if (typeset_setwidth(cname, pm, ops, on, 0)) return NULL; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 17:25 ` Bart Schaefer @ 2015-01-10 18:28 ` Bart Schaefer 2015-01-10 19:10 ` Mikael Magnusson 0 siblings, 1 reply; 11+ messages in thread From: Bart Schaefer @ 2015-01-10 18:28 UTC (permalink / raw) To: zsh workers On Jan 10, 9:25am, Bart Schaefer wrote: } } @@ -1935,7 +1935,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), } * even if that's unset } */ } if (pm && (pm->node.flags & PM_SPECIAL)) } - usepm = 1; } + usepm = 2; /* indicate that we preserve the PM_UNSET flag */ } } /* } * Don't use an existing param if Upon further reflection: diff --git a/Src/builtin.c b/Src/builtin.c index 8dee8f9..9258ddb 100644 --- a/Src/builtin.c +++ b/Src/builtin.c @@ -1934,7 +1934,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), * We need to compare types with an existing pm if special, * even if that's unset */ - if (pm && (pm->node.flags & PM_SPECIAL)) + if (!usepm && pm && (pm->node.flags & PM_SPECIAL)) usepm = 2; /* indicate that we preserve the PM_UNSET flag */ /* ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 18:28 ` Bart Schaefer @ 2015-01-10 19:10 ` Mikael Magnusson 2015-01-10 20:04 ` Bart Schaefer 0 siblings, 1 reply; 11+ messages in thread From: Mikael Magnusson @ 2015-01-10 19:10 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sat, Jan 10, 2015 at 7:28 PM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 10, 9:25am, Bart Schaefer wrote: > } > } @@ -1935,7 +1935,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), > } * even if that's unset > } */ > } if (pm && (pm->node.flags & PM_SPECIAL)) > } - usepm = 1; > } + usepm = 2; /* indicate that we preserve the PM_UNSET flag */ > } > } /* > } * Don't use an existing param if > > Upon further reflection: > > diff --git a/Src/builtin.c b/Src/builtin.c > index 8dee8f9..9258ddb 100644 > --- a/Src/builtin.c > +++ b/Src/builtin.c > @@ -1934,7 +1934,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func), > * We need to compare types with an existing pm if special, > * even if that's unset > */ > - if (pm && (pm->node.flags & PM_SPECIAL)) > + if (!usepm && pm && (pm->node.flags & PM_SPECIAL)) > usepm = 2; /* indicate that we preserve the PM_UNSET flag */ > > /* I get this now, which I think is the same as I got for one of my two tests earlier; % unset IFS % export IFS % printenv|grep IFS % IFS=5 % printenv|grep IFS IFS=5 % unset IFS % printenv|grep IFS % IFS=5 % printenv|grep IFS # this would be empty for a normal parameter IFS=5 % unset IFS % printenv|grep IFS % IFS=5 % printenv|grep IFS # and this (same case) IFS=5 typeset +x IFS works to un-export it, even while it's unset. (this is a new instance) % echo ${(t)IFS} scalar-special % unset IFS % echo ${(t)IFS} % export IFS % echo ${(t)IFS} % printenv|grep IFS % IFS=5 % printenv|grep IFS IFS=5 % echo ${(t)IFS} scalar-export-special % typeset +x IFS % echo ${(t)IFS} scalar-special % printenv|grep IFS % export IFS % printenv|grep IFS IFS=5 % unset IFS % printenv|grep IFS % echo ${(t)IFS} % typeset +x IFS % echo ${(t)IFS} % IFS=3 % printenv|grep IFS % echo ${(t)IFS} scalar-special -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 19:10 ` Mikael Magnusson @ 2015-01-10 20:04 ` Bart Schaefer 2015-01-10 20:28 ` Mikael Magnusson 0 siblings, 1 reply; 11+ messages in thread From: Bart Schaefer @ 2015-01-10 20:04 UTC (permalink / raw) To: zsh workers On Jan 10, 8:10pm, Mikael Magnusson wrote: } } % unset IFS } % export IFS } % IFS=5 } % unset IFS } % printenv|grep IFS # this would be empty for a normal parameter } IFS=5 OK, so the remaining issue is that the PM_EXPORT flag is not cleared when a special parameter is unset. I think this is within the definition of how a special paramter behaves; i.e., it retains all its attributes even when unset. In this case we have an attribute that's not normally "part of the specialness" of the parameter, which is also being preserved. The same thing happens with other flags on other specials, e.g.: torch% print $SECONDS 762 torch% typeset -E SECONDS torch% print $SECONDS 7.801753190e+02 torch% unset SECONDS torch% print $SECONDS torch% SECONDS=763 torch% print $SECONDS 7.671235090e+02 So should e.g. stdunsetfn() and tiedarrunsetfn() clear the PM_EXPORTED flag as well as assert the PM_UNSET flag? Or is that going to cause a problem elsewhere because there is a special parameter that has the feature of always being exported? I can't think of any, but maybe we shouldn't rule it out ... in which case this gets really hairy, as we will have to know, for every special, what flags should or should not be retained across unset. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 20:04 ` Bart Schaefer @ 2015-01-10 20:28 ` Mikael Magnusson 0 siblings, 0 replies; 11+ messages in thread From: Mikael Magnusson @ 2015-01-10 20:28 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sat, Jan 10, 2015 at 9:04 PM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 10, 8:10pm, Mikael Magnusson wrote: > } > } % unset IFS > } % export IFS > } % IFS=5 > } % unset IFS > } % printenv|grep IFS # this would be empty for a normal parameter > } IFS=5 > > OK, so the remaining issue is that the PM_EXPORT flag is not cleared > when a special parameter is unset. > > I think this is within the definition of how a special paramter behaves; > i.e., it retains all its attributes even when unset. In this case we > have an attribute that's not normally "part of the specialness" of the > parameter, which is also being preserved. The same thing happens with > other flags on other specials, e.g.: > > torch% print $SECONDS > 762 > torch% typeset -E SECONDS > torch% print $SECONDS > 7.801753190e+02 > torch% unset SECONDS > torch% print $SECONDS > > torch% SECONDS=763 > torch% print $SECONDS > 7.671235090e+02 > > > So should e.g. stdunsetfn() and tiedarrunsetfn() clear the PM_EXPORTED > flag as well as assert the PM_UNSET flag? Or is that going to cause a > problem elsewhere because there is a special parameter that has the > feature of always being exported? I can't think of any, but maybe we > shouldn't rule it out ... in which case this gets really hairy, as we > will have to know, for every special, what flags should or should not > be retained across unset. Ah, these are good points. I don't have any especially strong feelings about making IFS stop being exported after unset, I was just reporting what I observed in case it indicated some further wonkiness. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 7:56 ` Mikael Magnusson 2015-01-10 8:09 ` Mikael Magnusson @ 2015-01-10 16:57 ` Bart Schaefer 2015-01-10 17:12 ` Mikael Magnusson 1 sibling, 1 reply; 11+ messages in thread From: Bart Schaefer @ 2015-01-10 16:57 UTC (permalink / raw) To: zsh workers On Jan 10, 8:56am, Mikael Magnusson wrote: } Subject: Re: Crash when exporting scalar without value and getsparam fails } } On Sat, Jan 10, 2015 at 7:52 AM, Bart Schaefer } <schaefer@brasslantern.com> wrote: } > I think it should do nothing to the environment and return success. } > } > % bash -c 'unset IFS; export IFS; echo $?; printenv | grep IFS' } > 0 } > % } } Ah yes, this is what happens for other parameters in zsh too. So did I somehow miss that this is specific to IFS and doesn't affect exports in general? Or is there some other characteristic of the parameters that tickle this bug? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Crash when exporting scalar without value and getsparam fails 2015-01-10 16:57 ` Bart Schaefer @ 2015-01-10 17:12 ` Mikael Magnusson 0 siblings, 0 replies; 11+ messages in thread From: Mikael Magnusson @ 2015-01-10 17:12 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sat, Jan 10, 2015 at 5:57 PM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 10, 8:56am, Mikael Magnusson wrote: > } Subject: Re: Crash when exporting scalar without value and getsparam fails > } > } On Sat, Jan 10, 2015 at 7:52 AM, Bart Schaefer > } <schaefer@brasslantern.com> wrote: > } > I think it should do nothing to the environment and return success. > } > > } > % bash -c 'unset IFS; export IFS; echo $?; printenv | grep IFS' > } > 0 > } > % > } > } Ah yes, this is what happens for other parameters in zsh too. > > So did I somehow miss that this is specific to IFS and doesn't affect > exports in general? Or is there some other characteristic of the > parameters that tickle this bug? (I see that what I wrote is a bit unclear, but I'm not sure what the 'this' refers to, so I'll be specific about everything). I don't know exactly how specific the crash is, but for most parameters, doing an unset followed by an export does what you suggest we should do for IFS. Eg, it isn't put into the environment, but upon the next assignment it is set and exported into the environment without another explicit 'export'. For IFS doing this just crashes currently. The reason for which is that getsparam fails on IFS when it is unset (I don't know why). According to Oliver's original analysis, there may be other cases such as dupstring failing (that would break everything else anyway), or other unknown causes. Neither of the changes I tested makes IFS behave as the generic case, but both stop the crashing. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-01-10 20:28 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-01-09 13:26 Crash when exporting scalar without value and getsparam fails Mikael Magnusson 2015-01-10 6:52 ` Bart Schaefer 2015-01-10 7:56 ` Mikael Magnusson 2015-01-10 8:09 ` Mikael Magnusson 2015-01-10 17:25 ` Bart Schaefer 2015-01-10 18:28 ` Bart Schaefer 2015-01-10 19:10 ` Mikael Magnusson 2015-01-10 20:04 ` Bart Schaefer 2015-01-10 20:28 ` Mikael Magnusson 2015-01-10 16:57 ` Bart Schaefer 2015-01-10 17:12 ` 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).