[[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Anssi Palin]

Hello,

In workers/41719 I wrote about a memory leak with the (( ${+a[key]} )) syntax. 
While the issue has since been patched I've now noticed that the alternative 
[[ -v a[key] ]] syntax still suffers from the same problem in 5.5.1.

It also seems that post-patch Zsh is incorrectly detecting undefined keys as 
set in empty associative arrays when KSH_ARRAYS is in effect:

$ set -o KSH_ARRAYS
$ typeset -A a
$ if (( ${+a[undefined]} )); then echo 'Should not echo'; fi
Should not echo

Once the array is populated both set and undefined keys are detected correctly. 
The [[ -v a[key] ]] syntax does not appear to be affected in any case 
regardless of KSH_ARRAYS.

Thank you in advance.

Re: [[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Oliver Kiddle]

Anssi Palin wrote:
>
> In workers/41719 I wrote about a memory leak with the (( ${+a[key]} )) syntax. 
> While the issue has since been patched I've now noticed that the alternative 
> [[ -v a[key] ]] syntax still suffers from the same problem in 5.5.1.

After looking at the patch for the first issue, I tried the following
and it does fix the issue for [[ -v a[key] ]].

+++ b/Src/params.c
@@ -691,7 +691,7 @@ issetvar(char *name)
-    if (!(v = getvalue(&vbuf, &name, 1)) || *name)
+    if (!(v = fetchvalue(&vbuf, &name, 1, SCANPM_CHECKING)) || *name)

However, the problem seems to even occur for something like
  : ${a[$i]}

We need to differentiate anything that does assignment and should create
the parameter if it isn't already there.

Oliver

Re: [[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Anssi Palin]

Hello everyone,

My original message in this thread about problems introduced around Zsh 5.5.1 
with the ${+a[$key]} syntax went somewhat unnoticed. Has the issue been 
investigated since? I feel the problem is somewhat urgent as in the meantime 
I've modified some of my scripts to use a workaround with dummy keys to get 
them to execute similarly as before.

Thank you again.

Re: [[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Joey Pabalinas]

[-- Attachment #1: Type: text/plain, Size: 1101 bytes --]

On Wed, Aug 08, 2018 at 09:31:05PM +0000, Anssi Palin wrote:
> My original message in this thread about problems introduced around Zsh 5.5.1 
> with the ${+a[$key]} syntax went somewhat unnoticed. Has the issue been 
> investigated since? I feel the problem is somewhat urgent as in the meantime 
> I've modified some of my scripts to use a workaround with dummy keys to get 
> them to execute similarly as before.

It doesn't work correctly for me on 5.5.1 either; I can confirm the
broken behavior:

	hobbes% zsh -f
	hobbes% setopt ksharrays
	hobbes% typeset -A a
	hobbes% ((${+a[key]})) && print should not echo
	should not echo
	hobbes%

Although interestingly enough:

	hobbes% zsh -f
	hobbes% setopt ksharrays
	hobbes% typeset -A a
	hobbes% [[ -v a[key] ]]
	hobbes% ((${+a[key]})) && print should not echo
	hobbes%

This is the with the zsh currently packaged on arch:

	bobbes% zsh --version
	zsh 5.5.1 (x86_64-unknown-linux-gnu)
	hobbes%

When I get home I'll try compiling the current HEAD to see if the
behavior has changed.

-- 
Cheers,
Joey Pabalinas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Peter Stephenson]

On Wed, 8 Aug 2018 21:31:05 +0000
Anssi Palin <Anssi.Palin@edu.omnia.fi> wrote:
> My original message in this thread about problems introduced around
> Zsh 5.5.1 with the ${+a[$key]} syntax went somewhat unnoticed. Has
> the issue been investigated since? I feel the problem is somewhat
> urgent as in the meantime I've modified some of my scripts to use a
> workaround with dummy keys to get them to execute similarly as before.

It looks like the simple change does the right thing, though this isn't
code I tend to look at.

By the way, although this is certainly a real bug, you'll tend to find
you hit fewer if you don't mix native zsh features (such as ${+...})
with features designed to support [k]sh --- the overlap between the two
worlds is very complicated and probably the most poorly tested part of
the system.

diff --git a/Src/params.c b/Src/params.c
index f130934..a1c299f 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -1427,7 +1427,7 @@ getarg(char **str, int *inv, Value v, int a2, zlong *w,
 	    HashTable ht = v->pm->gsu.h->getfn(v->pm);
 	    if (!ht) {
 		if (flags & SCANPM_CHECKING)
-		    return isset(KSHARRAYS) ? 1 : 0;
+		    return 0;
 		ht = newparamtable(17, v->pm->node.nam);
 		v->pm->gsu.h->setfn(v->pm, ht);
 	    }
diff --git a/Test/D04parameter.ztst b/Test/D04parameter.ztst
index 3b187f4..e327a78 100644
--- a/Test/D04parameter.ztst
+++ b/Test/D04parameter.ztst
@@ -2422,3 +2422,11 @@ F:behavior, see http://austingroupbugs.net/view.php?id=888
 >: #
 >: ` backtick
 >: word
+
+  (
+    setopt KSH_ARRAYS
+    typeset -A ksh_assoc
+    print ${+assoc[unset]}
+  )
+0:Use of parameter subst + to test element of hash with KSH_ARRAYS.
+>0

Re: [[ -v a[key] ]] syntax memory leak & undefined associative array keys detected as set

[Sebastian Gniazdowski]

On Sat, 19 May 2018 at 00:06, Anssi Palin <Anssi.Palin@edu.omnia.fi> wrote:
>
> Hello,
>
> In workers/41719 I wrote about a memory leak with the (( ${+a[key]} )) syntax.
> While the issue has since been patched I've now noticed that the alternative
> [[ -v a[key] ]] syntax still suffers from the same problem in 5.5.1.

On that occasion I would like to mention, that if VATS automatic
Valgrind tests would have been integrated into upstream, we could
already have some group of people looking at it and reporting if in
doubt or after spotting something.

-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin