[PATCHes] Fix issues in setscope

[Philippe Altherr <philippe.altherr@xxxxxxxxx>]

The attached 3 patches fix a number of issues in setscope. The second patch mainly reorders the computations in setscope. The two others should be relatively self-explanatory.

Philippe

diff --git a/Src/params.c b/Src/params.c
index 7b515515e..471849e01 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -525,7 +525,7 @@ newparamtable(int size, char const *name)
 
 /**/
 static HashNode
-getparamnode(HashTable ht, const char *nam)
+getparamnode_nofollow(HashTable ht, const char *nam)
 {
     HashNode hn = gethashnode2(ht, nam);
     Param pm = (Param) hn;
@@ -547,6 +547,14 @@ getparamnode(HashTable ht, const char *nam)
 	}
     }
 
+    return hn;
+}
+
+/**/
+static HashNode
+getparamnode(HashTable ht, const char *nam)
+{
+    HashNode hn = getparamnode_nofollow(ht, nam);
     if (hn && ht == realparamtab && !(hn->flags & PM_UNSET))
 	hn = resolve_nameref((Param)hn, NULL);
     return hn;
@@ -6419,7 +6427,8 @@ setscope(Param pm)
 		    pm->base = basepm->level;
 	    }
 	} else if (pm->base < locallevel && refname &&
-		   (basepm = (Param)getparamnode(realparamtab, refname))) {
+		   (basepm = (Param)getparamnode_nofollow(realparamtab, refname)) &&
+		   (!(basepm->node.flags & PM_NEWREF) || (basepm = basepm->old))) {
 	    pm->base = basepm->level;
 	    if ((pm->node.flags & PM_UPPER) &&
 		(basepm = upscope(basepm, -(pm->level))))
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index 54f0aaf68..73126ae90 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -1179,4 +1179,21 @@ F:previously this could create an infinite recursion and crash
 >typeset PS1=zz
 *?*
 
+ () {
+   typeset var1=var1
+   typeset var2=var2
+   typeset -n ref1=var1
+   echo "ref1=$ref1";
+   () {
+     typeset -n ref1=var2
+     typeset -n ref2=ref1
+     echo "ref1=$ref1";
+     echo "ref2=$ref2";
+   }
+ }
+0:regression: don't follow references when computing base scope - part 1
+>ref1=var1
+>ref1=var2
+>ref2=var2
+
 %clean

diff --git a/Src/params.c b/Src/params.c
index 471849e01..de8dd7879 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -3324,7 +3324,11 @@ assignsparam(char *s, char *val, int flags)
 	}
     }
 
+    if (v->pm->node.flags & PM_NAMEREF)
+	v->pm->node.flags |= PM_NEWREF;
     assignstrvalue(v, val, flags);
+    if (v->pm->node.flags & PM_NAMEREF)
+	v->pm->node.flags &= ~PM_NEWREF;
     unqueue_signals();
     return v->pm;
 }
@@ -6375,12 +6379,39 @@ setscope(Param pm)
 	char *t = refname ? itype_end(refname, INAMESPC, 0) : NULL;
 	int q = queue_signal_level();
 
+	/* Compute pm->width */
 	/* Temporarily change nameref to array parameter itself */
 	if (t && *t == '[')
 	    *t = 0;
 	else
 	    t = 0;
-	stop.name = "";
+	if (t) {
+	    pm->width = t - refname;
+	    *t = '[';
+	    refname = dupstrpfx(refname, pm->width);
+	}
+
+	/* Compute pm->base */
+	if (pm->base < locallevel && refname &&
+	    (basepm = (Param)getparamnode_nofollow(realparamtab, refname)) &&
+	    (!(basepm->node.flags & PM_NEWREF) || (basepm = basepm->old))) {
+	    pm->base = basepm->level;
+	    if ((pm->node.flags & PM_UPPER) &&
+		(basepm = upscope(basepm, -(pm->level))))
+		pm->base = basepm->level;
+	}
+	if (pm->base > pm->level) {
+	    if (EMULATION(EMULATE_KSH)) {
+		zerr("%s: global reference cannot refer to local variable",
+		      pm->node.nam);
+		unsetparam_pm(pm, 0, 1);
+	    } else if (isset(WARNNESTEDVAR))
+		zwarn("reference %s in enclosing scope set to local variable %s",
+		      pm->node.nam, refname);
+	}
+
+	/* Check for self references */
+	stop.name = pm->node.nam;
 	stop.value.scalar = NULL;
 	stop.flags = PM_NAMEREF;
 	if (locallevel && !(pm->node.flags & PM_UPPER))
@@ -6388,11 +6419,6 @@ setscope(Param pm)
 	dont_queue_signals();	/* Prevent unkillable loops */
 	basepm = (Param)resolve_nameref(pm, &stop);
 	restore_queue_signals(q);
-	if (t) {
-	    pm->width = t - refname;
-	    *t = '[';
-	    refname = dupstrpfx(refname, pm->width);
-	}
 	if (basepm) {
 	    if (basepm->node.flags & PM_NAMEREF) {
 		if (pm == basepm) {
@@ -6420,28 +6446,7 @@ setscope(Param pm)
 			break;
 		    }
 		}
-	    } else if (!pm->base) {
-		pm->base = basepm->level;
-		if ((pm->node.flags & PM_UPPER) &&
-		    (basepm = upscope(basepm, -(pm->level))))
-		    pm->base = basepm->level;
 	    }
-	} else if (pm->base < locallevel && refname &&
-		   (basepm = (Param)getparamnode_nofollow(realparamtab, refname)) &&
-		   (!(basepm->node.flags & PM_NEWREF) || (basepm = basepm->old))) {
-	    pm->base = basepm->level;
-	    if ((pm->node.flags & PM_UPPER) &&
-		(basepm = upscope(basepm, -(pm->level))))
-		pm->base = basepm->level;
-	}
-	if (pm->base > pm->level) {
-	    if (EMULATION(EMULATE_KSH)) {
-		zerr("%s: global reference cannot refer to local variable",
-		      pm->node.nam);
-		unsetparam_pm(pm, 0, 1);
-	    } else if (isset(WARNNESTEDVAR))
-		zwarn("reference %s in enclosing scope set to local variable %s",
-		      pm->node.nam, refname);
 	}
 	if (refname && upscope(pm, pm->base) == pm &&
 	    strcmp(pm->node.nam, refname) == 0) {
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index 73126ae90..f84d4ed05 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -1194,6 +1194,24 @@ F:previously this could create an infinite recursion and crash
 0:regression: don't follow references when computing base scope - part 1
 >ref1=var1
 >ref1=var2
+>ref2=var2
+
+ () {
+   typeset var1=var1
+   typeset -n ref1=var1
+   echo ref1=$ref1;
+   () {
+     typeset var2=var2
+     typeset -n ref1
+     typeset -n ref2=ref1
+     ref1=var2
+     echo ref1=$ref1;
+     echo ref2=$ref2;
+   }
+ }
+0:regression: don't follow references when computing base scope - part 2
+>ref1=var1
+>ref1=var2
 >ref2=var2
 
 %clean

diff --git a/Src/params.c b/Src/params.c
index de8dd7879..7bbc69d48 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2240,10 +2240,12 @@ fetchvalue(Value v, char **pptr, int bracks, int flags)
 		}
 		Param p1 = (Param)gethashnode2(paramtab, ref);
 		if (p1) {
-		    int scope = ((pm->node.flags & PM_NAMEREF) ?
-				 ((pm->node.flags & PM_UPPER) ? -(pm->base) :
-				  pm->base) : locallevel);
-		    pm = upscope(p1, scope);
+		    if ((pm->node.flags & PM_NAMEREF) &&
+			(pm->node.flags & PM_UPPER))
+			pm = upscope_upper(p1, pm->level - 1);
+		    else
+			pm = upscope(p1, (pm->node.flags & PM_NAMEREF) ?
+				     pm->base : locallevel);
 		}
 		if (!(p1 && pm) ||
 		    ((pm->node.flags & PM_UNSET) &&
@@ -6318,12 +6320,13 @@ resolve_nameref(Param pm, const Asgment stop)
 	} else if ((hn = gethashnode2(realparamtab, seek))) {
 	    if (pm) {
 		if (!(stop && (stop->flags & (PM_LOCAL)))) {
-		    int scope = ((pm->node.flags & PM_NAMEREF) ?
-				 ((pm->node.flags & PM_UPPER) ?
-				  /* pm->base == 0 means not set yet */
-				  -(pm->base ? pm->base : pm->level) :
-				  pm->base) : ((Param)hn)->level);
-		    hn = (HashNode)upscope((Param)hn, scope);
+		    if ((pm->node.flags & PM_NAMEREF) &&
+			(pm->node.flags & PM_UPPER))
+			hn = (HashNode)upscope_upper((Param)hn, pm->level - 1);
+		    else
+			hn = (HashNode)upscope((Param)hn,
+					       (pm->node.flags & PM_NAMEREF) ?
+					       (pm->base) : ((Param)hn)->level);
 		}
 		/* user can't tag a nameref, safe for loop detection */
 		pm->node.flags |= PM_TAGGED;
@@ -6392,13 +6395,10 @@ setscope(Param pm)
 	}
 
 	/* Compute pm->base */
-	if (pm->base < locallevel && refname &&
+	if (!(pm->node.flags & PM_UPPER) && refname &&
 	    (basepm = (Param)getparamnode_nofollow(realparamtab, refname)) &&
 	    (!(basepm->node.flags & PM_NEWREF) || (basepm = basepm->old))) {
 	    pm->base = basepm->level;
-	    if ((pm->node.flags & PM_UPPER) &&
-		(basepm = upscope(basepm, -(pm->level))))
-		pm->base = basepm->level;
 	}
 	if (pm->base > pm->level) {
 	    if (EMULATION(EMULATE_KSH)) {
@@ -6463,13 +6463,18 @@ upscope(Param pm, int reflevel)
 {
     Param up = pm->old;
     while (up && up->level >= reflevel) {
-	if (reflevel < 0 && up->level < -(reflevel))
-	    break;
 	pm = up;
 	up = up->old;
     }
-    if (reflevel < 0 && locallevel > 0)
-	return pm->level == locallevel ? up : pm;
+    return pm;
+}
+
+/**/
+mod_export Param
+upscope_upper(Param pm, int reflevel)
+{
+    while (pm && pm->level > reflevel)
+	pm = pm->old;
     return pm;
 }
 
diff --git a/Test/K01nameref.ztst b/Test/K01nameref.ztst
index f84d4ed05..27efb9515 100644
--- a/Test/K01nameref.ztst
+++ b/Test/K01nameref.ztst
@@ -1100,15 +1100,15 @@ F:previously this could create an infinite recursion and crash
 >h:1: rs= - ra= - rs1= - ra1=
 >h:2: rs= - ra= - rs1= - ra1=
 >i:1: rs= - ra= - rs1= - ra1=
->i:2: rs=g - ra=g - rs1=g - ra1=g
->j:1: rs=g - ra=g - rs1=g - ra1=g
->j:2: rs=g - ra=g - rs1=g - ra1=g
->i:3: rs=g - ra=g - rs1=g - ra1=g
->k:1: rs=g - ra=g - rs1=g - ra1=g
->k:2: rs=g - ra=g - rs1=g - ra1=g
->h:3: rs=g - ra=g - rs1=g - ra1=g
->k:1: rs=g - ra=g - rs1=g - ra1=g
->k:2: rs=g - ra=g - rs1=g - ra1=g
+>i:2: rs=f - ra=f - rs1=f - ra1=f
+>j:1: rs=f - ra=f - rs1=f - ra1=f
+>j:2: rs=f - ra=f - rs1=f - ra1=f
+>i:3: rs=f - ra=f - rs1=f - ra1=f
+>k:1: rs=f - ra=f - rs1=f - ra1=f
+>k:2: rs=f - ra=f - rs1=f - ra1=f
+>h:3: rs=f - ra=f - rs1=f - ra1=f
+>k:1: rs=f - ra=f - rs1=f - ra1=f
+>k:2: rs=f - ra=f - rs1=f - ra1=f
 >g:3: rs=f - ra=f - rs1=f - ra1=f
 
  e '' 6