ksh93 / ksh

ksh 93u+m: KornShell lives! | Latest release: https://github.com/ksh93/ksh/releases
Eclipse Public License 2.0
192 stars 32 forks source link

The word 'select' causes 2d arrays to fail #790

Closed TristanJamesBall closed 1 month ago

TristanJamesBall commented 1 month ago
# typeset -a a1=( (demo)  ("select") (col) (from) (table)  )
# typeset -p a1
typeset -a a1=((demo) (select) (col) (from) (table) )

# eval ${ typeset -p a1; }
/bin/ksh: eval: syntax error: `select' unexpected

# typeset -a a1=( (line0) ( select from ) )
/bin/ksh: syntax error: `select' unexpected

Seems to be specific to the word "select", and it only occurs when it appears as the only element of the array dimenion, where that dimension >0

For example, this works fine:

typeset -a a1=( select col from table )
eval ${ typeset -p a1 ;}

Quoting the "Select" appears to work around it, but typeset -p doesn't quote array elements unless it has too, so you can't eval the output typeset -p

Occurs on both my dev build from here: Version JM 93u+m/1.1.0-alpha+1c4b3ae6/MOD 2023-06-14 And the distro version: Version AJM 93u+m/1.0.8 2024-01-01

McDutchie commented 1 month ago

Thanks for the report. This is also reproducible on 93u+ 2012-08-01 and on ksh2020.

McDutchie commented 1 month ago

The problem is that reserved words are incorrectly recognised when assign() is called recursively to parse multidimensional array assignments. Please try this patch:

diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c
index 1f88e424d..58dbea2f1 100644
--- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -1053,7 +1053,10 @@ static struct argnod *assign(Lex_t *lexp, struct argnod *ap, int type)
            ar = stkfreeze(sh.stk,1);
            ar->argnxt.ap = 0;
            if(!aq)
+           {
+               lexp->noreserv = 1;
                ar = assign(lexp,ar,0);
+           }
            ar->argflag |= ARG_MESSAGE;
            *settail = ar;
            settail = &(ar->argnxt.ap);
McDutchie commented 1 month ago

That patch is not quite right. It fixes the reproducer but not this:

$ arch/*/bin/ksh -c 'typeset -a a1=((demo) (select) (col) (from) (table) ); typeset -p a1'
typeset -a a1=((demo) (select) (col) (from) (table) )
$ arch/*/bin/ksh -c 'typeset -a a1=((demo) select (col) (from) (table) ); typeset -p a1'  
arch/darwin.arm64-64/bin/ksh: syntax error at line 1: `select' unexpected

Patch v2:

diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c
index 1f88e424d..71a364def 100644
--- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -1053,7 +1053,12 @@ static struct argnod *assign(Lex_t *lexp, struct argnod *ap, int type)
            ar = stkfreeze(sh.stk,1);
            ar->argnxt.ap = 0;
            if(!aq)
+           {
                ar = assign(lexp,ar,0);
+               /* since noreserv is reset to 0 below, we have to restore it after a recursive call */
+               if(array)
+                   lexp->noreserv = 1;
+           }
            ar->argflag |= ARG_MESSAGE;
            *settail = ar;
            settail = &(ar->argnxt.ap);
McDutchie commented 1 month ago

Another approach, which may be safer, is to save and restore the value:

diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c
index 1f88e424d..cc30ae7f2 100644
--- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -1053,7 +1053,12 @@ static struct argnod *assign(Lex_t *lexp, struct argnod *ap, int type)
            ar = stkfreeze(sh.stk,1);
            ar->argnxt.ap = 0;
            if(!aq)
+           {
+               /* since noreserv is reset to 0 below, we have to restore it after a recursive call */
+               char n = lexp->noreserv;
                ar = assign(lexp,ar,0);
+               lexp->noreserv = n;
+           }
            ar->argflag |= ARG_MESSAGE;
            *settail = ar;
            settail = &(ar->argnxt.ap);
McDutchie commented 1 month ago

It's still not right. It should also work without the typeset -a, when the array attribute is assigned implicitly, and it doesn't.

$ arch/*/bin/ksh -c 'my_array=((somevalue) select) && typeset -p my_array'
arch/darwin.arm64-64/bin/ksh: syntax error at line 1: `select' unexpected
McDutchie commented 1 month ago

This fixes it with or without the explicit typeset -a.

Patch v4:

diff --git a/src/cmd/ksh93/sh/parse.c b/src/cmd/ksh93/sh/parse.c
index 1f88e424d..5e2f9cc0e 100644
--- a/src/cmd/ksh93/sh/parse.c
+++ b/src/cmd/ksh93/sh/parse.c
@@ -1053,7 +1053,11 @@ static struct argnod *assign(Lex_t *lexp, struct argnod *ap, int type)
            ar = stkfreeze(sh.stk,1);
            ar->argnxt.ap = 0;
            if(!aq)
+           {
                ar = assign(lexp,ar,0);
+               /* since noreserv is reset to 0 below, we have set it again after a recursive call */
+               lexp->noreserv = 1;
+           }
            ar->argflag |= ARG_MESSAGE;
            *settail = ar;
            settail = &(ar->argnxt.ap);
TristanJamesBall commented 4 weeks ago

This is fantastic, thankyou for such a quick turnaround on the fix!

TristanJamesBall commented 4 weeks ago

Unfortunately though, there still seems to be an issue - although triggered by the word 'export' not 'select'

typeset -a arr=( (a b c) (export demo array) )
typeset -p arr
typeset -a arr=((a b c) ())

Do you want a new issue raised?

McDutchie commented 4 weeks ago

Yes please, that's a different issue, not related to reserved words.