Fix segfault in unsetref+=(foo); add warning

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

NEWS

@@ -4,6 +4,10 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
 
 2023-09-13:
 
+- Fixed a crash on trying to append an indexed array value to an unset name
+  reference, e.g.: nameref unsetref; unsetref+=(foo bar). This now produces
+  a "removing nameref attribute" warning before performing the assignment.
+
 - Fixed: assignments like name=(...) to arrays did not preserve the array and
   variable types. Fix backported from ksh 93v- beta (2013-07-19, 2013-07-27).
 

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";

src/cmd/ksh93/include/name.h

@@ -247,6 +247,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[];

src/cmd/ksh93/sh/name.c

@@ -946,10 +946,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)
 				{

src/cmd/ksh93/tests/nameref.sh

@@ -592,7 +592,9 @@ function read_c
 }
 print "( typeset -i x=36 ) " | read_c ar[5][9][2]
 exp=$'(\n\ttypeset -a [5]=(\n\t\ttypeset -a [9]=(\n\t\t\t[2]=(\n\t\t\t\ttypeset -i x=36\n\t\t\t)\n\t\t)\n\t)\n)'
-[[ $(print -v ar) == "$exp" ]] || err_exit 'read into nameref of global array instance from within a function fails'
+got=$(print -v ar)
+[[ $got == "$exp" ]] || err_exit 'read into nameref of global array instance from within a function fails' \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
 function read_c
 {
@@ -605,7 +607,9 @@ function main
 	nameref nar=ar
 	print "( typeset -i x=36 ) " | read_c nar[5][9][2]
 	exp=$'(\n\ttypeset -a [5]=(\n\t\ttypeset -a [9]=(\n\t\t\t[2]=(\n\t\t\t\ttypeset -i x=36\n\t\t\t)\n\t\t)\n\t)\n)'
-	[[ $(print -v nar) == "$exp" ]] || err_exit 'read from a nameref variable from calling scope fails'
+	got=$(print -v nar)
+	[[ $got == "$exp" ]] || err_exit 'read from a nameref variable from calling scope fails' \
+		"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 }
 main
 
@@ -626,7 +630,9 @@ function main
 	nameref l4=c.l[4]
 	printf "( typeset -a ar=( 1\n2\n3\n) b=1 )\n" | rf l4
 	exp=$'(\n\ttypeset -C -A l=(\n\t\t[4]=(\n\t\t\ttypeset -a ar=(\n\t\t\t\t1\n\t\t\t\t2\n\t\t\t\t3\n\t\t\t)\n\t\t\tb=1\n\t\t)\n\t)\n)'
-	[[ $(print -v c) == "$exp" ]] || err_exit 'read -C with nameref to array element fails'
+	got=$(print -v c)
+	[[ $got == "$exp" ]] || err_exit 'read -C with nameref to array element fails' \
+		"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 }
 main
 
@@ -683,7 +689,12 @@ typeset -n ref='arr[2]'
 
 $SHELL  2> /dev/null -c 'function x { nameref lv=gg ; compound -A lv.c=( [4]=( x=1 )) ; } ; compound gg ; x' || err_exit 'compound array assignment with nameref in a function failed'
 
-$SHELL -c 'unset -n KSH_VERSION' 2> /dev/null || err_exit 'Unable to unset nameref KSH_VERSION.'
+# ======
+exp=''
+got=$("$SHELL" -c 'unset -n KSH_VERSION; typeset -p KSH_VERSION' 2>&1)
+[[ e=$? -eq 0 && $got == "$exp" ]] || err_exit "unable to unset nameref KSH_VERSION" \
+	"(expected status 0 and $(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
 
 # ======
 # bug introduced in ksh 93u 2010-11-22
@@ -693,5 +704,55 @@ got=$(nameref a=b b=c c=d d=a 2>&1)
 [[ e=$? -eq 1 && $got == *"$exp" ]] || err_exit 'self-reference loop detection' \
 	"(expected status 1, *$(printf %q "$exp"); got status $e, $(printf %q "$got"))"
 
+# ======
+# test assigning array values to an unset nameref; this now produces a warning and should no longer crash
+# https://github.com/ksh93/ksh/issues/678
+
+exp=$': warning: unsetref: removing nameref attribute\ntypeset -C unsetref=()'
+got=$("$SHELL" -c 'nameref unsetref; unsetref+=(); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref+=() misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+got=$("$SHELL" -c 'nameref unsetref; unsetref=(); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref=() misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+
+exp=$': warning: unsetref: removing nameref attribute\ntypeset -a unsetref=(foo bar)'
+got=$("$SHELL" -c 'nameref unsetref; unsetref+=(foo bar); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref+=(foo bar) misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+got=$("$SHELL" -c 'nameref unsetref; unsetref=(foo bar); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref=(foo bar) misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+
+exp=$': warning: unsetref: removing nameref attribute\ntypeset -A unsetref=([x]=foo [y]=bar)'
+got=$("$SHELL" -c 'nameref unsetref; unsetref+=([x]=foo [y]=bar); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref+=([x]=foo [y]=bar) misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+got=$("$SHELL" -c 'nameref unsetref; unsetref=([x]=foo [y]=bar); typeset -p unsetref' 2>&1)
+[[ e=$? -eq 0 && $got == *"$exp" ]] || err_exit "unsetref=([x]=foo [y]=bar) misbehaves" \
+	"(expected status 0 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+
+# the following cases should remain errors
+
+exp=': unsetref: no reference name'
+got=$("$SHELL" -c 'nameref unsetref; echo foo $unsetref bar' 2>&1)
+[[ e=$? -eq 1 && $got == *"$exp" ]] || err_exit "\$unsetref misbehaves" \
+	"(expected status 1 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+got=$("$SHELL" -c 'nameref unsetref; unsetref.foo=bar' 2>&1)
+[[ e=$? -eq 1 && $got == *"$exp" ]] || err_exit "unsetref.foo=bar misbehaves" \
+	"(expected status 1 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+got=$("$SHELL" -c 'nameref unsetref; unsetref[1]=bar' 2>&1)
+[[ e=$? -eq 1 && $got == *"$exp" ]] || err_exit "unsetref[1]=bar misbehaves" \
+	"(expected status 1 and match of *$(printf %q "$exp")," \
+	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
+
 # ======
 exit $((Errors<125?Errors:125))