unsetref+=(foo) segfaults
#678
Labels
bug
Something is not working
Comments
|
Please test this patch: diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c
index 29a32bc91..5f3d8ea7f 100644
--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -914,7 +914,7 @@ Namval_t *nv_create(const char *name, Dt_t *root, int flags, Namfun_t *dp)
noscope = 1;
}
sh.first_root = root;
- if(nv_isref(np) && (c=='[' || c=='.' || !(flags&NV_ASSIGN)))
+ if(nv_isref(np) && (c=='[' || c=='.' || c==0 || !(flags&NV_ASSIGN)))
{
errormsg(SH_DICT,ERROR_exit(1),e_noref,nv_name(np));
UNREACHABLE(); |
|
That removes the crash for $ arch/linux.i386-64/bin/ksh -c 'typeset -n ref; ref+=(); typeset -p ref'
arch/linux.i386-64/bin/ksh: ref: no reference name
$ arch/linux.i386-64/bin/ksh -c 'typeset -n ref; ref+=(foo); typeset -p ref'
arch/linux.i386-64/bin/ksh: ref: no reference name
$ arch/linux.i386-64/bin/ksh -c 'typeset -n ref; ref+=($1); typeset -p ref' _ hi
_: ref: no reference name
$ arch/linux.i386-64/bin/ksh -c 'typeset -n ref; ref+=($1); typeset -p ref' _
_: ref: no reference name |
Yes. If the nameref is unset, there is no variable to add the empty array to, so IMO the error is appropriate. |
|
Fair enough. |
|
On second thought, I'm uncomfortable making it an error to assign an array value to an unset nameref, because this used to work in some cases. We could break an existing script somewhere. Yet, if this happens it's an indication of a probable bug in a script, so I don't think we should silently ignore it either. Here is patch version two. It makes ksh act like bash, and more or less like mksh, when assigning array values to an unset nameref. In that case, the nameref attribute is unset and a "removing nameref attribute" warning is printed as on bash. Simply expanding an unset nameref (as in Patch v2diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c
index fb08ed6ac..967ee7b4d 100644
--- a/src/cmd/ksh93/data/msg.c
+++ b/src/cmd/ksh93/data/msg.c
@@ -104,6 +104,7 @@ const char e_badref[] = "%s: reference variable cannot be an array";
const char e_badsubscript[] = "%c: invalid subscript in assignment";
const char e_noarray[] = "%s: cannot be an array";
const char e_badappend[] = "%s: invalid append to associative array";
+const char e_rmref[] = "%s: removing nameref attribute";
const char e_noref[] = "%s: no reference name";
const char e_nounattr[] = "cannot unset attribute C or A or a";
const char e_selfref[] = "%s: invalid self reference";
diff --git a/src/cmd/ksh93/include/name.h b/src/cmd/ksh93/include/name.h
index 61a8194f1..5bd915201 100644
--- a/src/cmd/ksh93/include/name.h
+++ b/src/cmd/ksh93/include/name.h
@@ -245,6 +245,7 @@ extern const char e_aliname[];
extern const char e_badexport[];
extern const char e_badref[];
extern const char e_badsubscript[];
+extern const char e_rmref[];
extern const char e_noref[];
extern const char e_selfref[];
extern const char e_staticfun[];
diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c
index 29a32bc91..25b463f51 100644
--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -914,10 +914,18 @@ Namval_t *nv_create(const char *name, Dt_t *root, int flags, Namfun_t *dp)
noscope = 1;
}
sh.first_root = root;
- if(nv_isref(np) && (c=='[' || c=='.' || !(flags&NV_ASSIGN)))
+ if(nv_isref(np))
{
- errormsg(SH_DICT,ERROR_exit(1),e_noref,nv_name(np));
- UNREACHABLE();
+ if(!(flags&NV_ASSIGN))
+ {
+ errormsg(SH_DICT,ERROR_exit(1),e_noref,nv_name(np));
+ UNREACHABLE();
+ }
+ else if(c=='[' || c=='.' || c==0)
+ {
+ errormsg(SH_DICT,ERROR_warn(0),e_rmref,nv_name(np));
+ nv_offattr(np,NV_REF);
+ }
}
if(sub && c==0)
{ |
|
Patch version three, minimising unrelated changes ( Patch v3diff --git a/src/cmd/ksh93/data/msg.c b/src/cmd/ksh93/data/msg.c
index fb08ed6ac..967ee7b4d 100644
--- a/src/cmd/ksh93/data/msg.c
+++ b/src/cmd/ksh93/data/msg.c
@@ -104,6 +104,7 @@ const char e_badref[] = "%s: reference variable cannot be an array";
const char e_badsubscript[] = "%c: invalid subscript in assignment";
const char e_noarray[] = "%s: cannot be an array";
const char e_badappend[] = "%s: invalid append to associative array";
+const char e_rmref[] = "%s: removing nameref attribute";
const char e_noref[] = "%s: no reference name";
const char e_nounattr[] = "cannot unset attribute C or A or a";
const char e_selfref[] = "%s: invalid self reference";
diff --git a/src/cmd/ksh93/include/name.h b/src/cmd/ksh93/include/name.h
index 61a8194f1..5bd915201 100644
--- a/src/cmd/ksh93/include/name.h
+++ b/src/cmd/ksh93/include/name.h
@@ -245,6 +245,7 @@ extern const char e_aliname[];
extern const char e_badexport[];
extern const char e_badref[];
extern const char e_badsubscript[];
+extern const char e_rmref[];
extern const char e_noref[];
extern const char e_selfref[];
extern const char e_staticfun[];
diff --git a/src/cmd/ksh93/sh/name.c b/src/cmd/ksh93/sh/name.c
index c094eb97f..60f72ae9b 100644
--- a/src/cmd/ksh93/sh/name.c
+++ b/src/cmd/ksh93/sh/name.c
@@ -942,10 +942,21 @@ Namval_t *nv_create(const char *name, Dt_t *root, int flags, Namfun_t *dp)
noscope = 1;
}
sh.first_root = root;
- if(nv_isref(np) && (c=='[' || c=='.' || !(flags&NV_ASSIGN)))
+ if(nv_isref(np))
{
- errormsg(SH_DICT,ERROR_exit(1),e_noref,nv_name(np));
- UNREACHABLE();
+ /* At this point, it is known that np is an unset nameref */
+ if(c=='[' || c=='.' || !(flags&NV_ASSIGN))
+ {
+ /* unsetref[foo]=bar or unsetref.foo=bar or $unsetref: error */
+ errormsg(SH_DICT,ERROR_exit(1),e_noref,nv_name(np));
+ UNREACHABLE();
+ }
+ else if(c==0)
+ {
+ /* unsetref=(...) or unsetref+=(...): remove -n attribute */
+ errormsg(SH_DICT,ERROR_warn(0),e_rmref,nv_name(np));
+ nv_offattr(np,NV_REF);
+ }
}
if(sub && c==0)
{ |
McDutchie
added a commit
that referenced
this issue
Sep 13, 2023
Emanuele Torre (@emanuele6) writes: > Appending a non-empty indexed array to an unset nameref crashes > with a segmentation fault: > $ arch/*/bin/ksh -c 'typeset -n ref; ref+=(); typeset -p ref' > typeset -n ref > $ arch/*/bin/ksh -c 'typeset -n ref; ref+=(foo); typeset -p ref' > Segmentation fault (core dumped) [...] > Appending an associative array errors correctly: > $ arch/*/bin/ksh -c 'typeset -n ref; ref+=([foo]=bar); typeset -p ref' > arch/linux.i386-64/bin/ksh: ref: no reference name For consistency, this commit makes all array assignments to an unset nameref (using either = or +=) succeed, but print a warning that the nameref attribute is removed -- a bash-like bahaviour. I think that this will cause fewer backward compatibility problems than making such assignments error out, since they used to succeed in some cases (e.g. 'typeset -n ref; ref=(foo bar)' always worked). It does mean the associative array reproducer above now generates a warning instead of an error. src/cmd/ksh93/sh/name.c: nv_create(): - After dereferencing a nameref, if nv_isref(np) is still true, we know it must be an unset nameref. In that case, add a check for c==0 (the value occurring upon var=(foo) and var+=(foo)), and produce a warning instead of an error, as argued above. Resolves: #678
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Appending a non-empty indexed array to an unset nameref crashes with a segmentation fault:
Appending an associative array errors correctly:
$ arch/linux.i386-64/bin/ksh -c 'typeset -n ref; ref+=([foo]=bar); typeset -p ref' arch/linux.i386-64/bin/ksh: ref: no reference nameThe text was updated successfully, but these errors were encountered: