additional changes to avoid array subscript double expansion in arithmetic contexts

CWRU/CWRU.chlog

@@ -10208,3 +10208,42 @@ subst.c
 execute_cmd.c
 	- execute_cond_node: if -v is the operator, and the operand is a valid
 	  array reference, pass TEST_ARRAYEXP flag to unary_test
+
+				    5/7
+				    ---
+builtins/common.[ch]
+	- set_expand_once: set array_expand_once to the value passed as the
+	  first argument, returning the original value
+
+builtins.h
+	- ARRAYREF_BUILTIN: new flag for shell builtins, means that the builtin
+	  can take array references, with subscripts, as arguments
+
+builtins/mkbuiltins.c
+	- set ARRAYREF_BUILTIN flag on builtins given in the arrayvar_builtins
+	  array
+
+execute_cmd.c
+	- execute_cond_node: use set_expand_once to set array_expand_once to 0
+	  before calling unary_test with -v (see change from 5/6)
+
+arrayfunc.c
+	- unbind_array_element: allow the caller to choose whether or not a
+	  subscript of `*' or `@' unsets the entire array by passing
+	  VA_ALLOWALL in FLAGS. Right now, since the unset builtin doesn't
+	  pass VA_ALLOWALL, those subscripts unset individual elements for
+	  associative arrays. We preserve the old behavior of unsetting
+	  indexed arrays for the time being with new indexed-array-specific
+	  code
+
+				    5/9
+				    ---
+
+builtins/shopt.def
+	- expand_once_flag: "assoc_expand_once" option now sets this flag,
+	  calls set_assoc_expand on change
+	- set_assoc_expand: sets assoc_expand_once to mirror expand_once_flag;
+	  placeholder for future changes
+
+builtins/common.h
+	- expand_once_flag: extern declaration

arrayfunc.c

@@ -1025,8 +1025,8 @@ quote_array_assignment_chars (list)
    expands from array VAR.  A subscript of `*' or `@' unsets the array. */
 /* If FLAGS&1 (VA_NOEXPAND) we don't expand the subscript; we just use it
    as-is. If FLAGS&VA_ONEWORD, we don't try to use skipsubscript to parse
-   the subscript, we just assume the subscript ends with a close bracket
-   and use the rest. */
+   the subscript, we just assume the subscript ends with a close bracket,
+   if one is present, and use what's inside the brackets. */
 int
 unbind_array_element (var, sub, flags)
      SHELL_VAR *var;
@@ -1055,9 +1055,7 @@ unbind_array_element (var, sub, flags)
     {
       if (array_p (var) || assoc_p (var))
 	{
-#if 0	/* TAG: bash-5.2 */
 	  if (flags & VA_ALLOWALL)
-#endif
 	    {
 	      unbind_variable (var->name);	/* XXX -- {array,assoc}_flush ? */
 	      return (0);
@@ -1083,6 +1081,27 @@ unbind_array_element (var, sub, flags)
     }
   else if (array_p (var))
     {
+      if (ALL_ELEMENT_SUB (sub[0]) && sub[1] == 0)
+	{
+	  /* We can go several ways here:
+		1) remove the array (backwards compatible)
+		2) empty the array (new behavior)
+		3) do nothing; treat the `@' or `*' as an expression and throw
+		   an error
+	  */
+	  /* Behavior 1 */
+	  if (shell_compatibility_level <= 51)
+	    {
+	      unbind_variable (name_cell (var));
+	      return 0;
+	    }
+	  else /* Behavior 2 */
+	    {
+	      array_flush (array_cell (var));
+	      return 0;
+	    }
+	  /* Fall through for behavior 3 */
+	}
       ind = array_expand_index (var, sub, len+1, 0);
       /* negative subscripts to indexed arrays count back from end */
       if (ind < 0)

builtins.h

@@ -45,6 +45,7 @@
 #define ASSIGNMENT_BUILTIN 0x10	/* This builtin takes assignment statements. */
 #define POSIX_BUILTIN	0x20	/* This builtins is special in the Posix command search order. */
 #define LOCALVAR_BUILTIN   0x40	/* This builtin creates local variables */
+#define ARRAYREF_BUILTIN 0x80	/* This builtin takes array references as arguments */
 
 #define BASE_INDENT	4
 

builtins/common.c

@@ -1026,3 +1026,29 @@ builtin_unbind_variable (vname)
     }
   return (unbind_variable (vname));
 }
+
+/* **************************************************************** */
+/*								    */
+/*	    External interface to manipulate shell options	    */
+/*								    */
+/* **************************************************************** */
+
+#if defined (ARRAY_VARS)
+int
+set_expand_once (nval, uwp)
+     int nval, uwp;
+{
+  int oa;
+
+  oa = assoc_expand_once;
+#if 0	/* TAG:bash-5.2 */
+  if (shell_compatibility_level > 51)
+#endif
+    {
+      if (uwp)
+	unwind_protect_int (assoc_expand_once);
+      assoc_expand_once = nval;
+    }
+  return oa;
+}
+#endif

builtins/common.h

@@ -138,6 +138,10 @@ extern sh_builtin_func_t *builtin_address PARAMS((char *));
 extern sh_builtin_func_t *find_special_builtin PARAMS((char *));
 extern void initialize_shell_builtins PARAMS((void));
 
+#if defined (ARRAY_VARS)
+extern int set_expand_once PARAMS((int, int));
+#endif
+
 /* Functions from exit.def */
 extern void bash_logout PARAMS((void));
 
@@ -243,6 +247,11 @@ extern int loop_level;
 /* variables from shift.def */
 extern int print_shift_error;
 
+/* variables from shopt.def */
+#if defined (ARRAY_VARS)
+extern int expand_once_flag;
+#endif
+
 /* variables from source.def */
 extern int source_searches_cwd;
 extern int source_uses_path;

builtins/declare.def

@@ -408,6 +408,12 @@ declare_internal (list, local_var)
 #else
       assoc_noexpand = 0;
 #endif
+      /* XXX - we allow unbalanced brackets if assoc_noexpand is set, we count
+	 brackets and make sure they match if assoc_noexpand is not set. So we
+	 need to make sure we're checking assoc_noexpand and expand_once_flag
+	 for backwards compatibility. We also use assoc_noexpand below when
+	 we call assign_array_element, so we need to make sure they're
+	 consistent in how they count brackets. */
       offset = assignment (name, assoc_noexpand ? 2 : 0);
       aflags = 0;
       created_var = 0;

builtins/mkbuiltins.c

@@ -72,7 +72,8 @@ extern char *strcpy ();
 #define BUILTIN_FLAG_SPECIAL	0x01
 #define BUILTIN_FLAG_ASSIGNMENT 0x02
 #define BUILTIN_FLAG_LOCALVAR	0x04
-#define BUILTIN_FLAG_POSIX_BUILTIN 0x08
+#define BUILTIN_FLAG_POSIX_BUILTIN	0x08
+#define BUILTIN_FLAG_ARRAYREF_ARG	0x10
 
 #define BASE_INDENT	4
 
@@ -173,11 +174,22 @@ char *posix_builtins[] =
   (char *)NULL
 };
 
+/* The builtin commands that can take array references as arguments and pay
+   attention to `assoc_expand_once'. These are the ones that don't assign
+   values, but need to avoid double expansions. */
+char *arrayvar_builtins[] =
+{
+  "declare", "let", "local", "printf", "read", "test", "[",
+  "typeset", "unset", "wait",		/*]*/
+  (char *)NULL
+};
+	
 /* Forward declarations. */
 static int is_special_builtin ();
 static int is_assignment_builtin ();
 static int is_localvar_builtin ();
 static int is_posix_builtin ();
+static int is_arrayvar_builtin ();
 
 #if !defined (HAVE_RENAME)
 static int rename ();
@@ -831,6 +843,8 @@ builtin_handler (self, defs, arg)
     new->flags |= BUILTIN_FLAG_LOCALVAR;
   if (is_posix_builtin (name))
     new->flags |= BUILTIN_FLAG_POSIX_BUILTIN;
+  if (is_arrayvar_builtin (name))
+    new->flags |= BUILTIN_FLAG_ARRAYREF_ARG;
 
   array_add ((char *)new, defs->builtins);
   building_builtin = 1;
@@ -1111,7 +1125,7 @@ char *structfile_header[] = {
   "/* This file is manufactured by ./mkbuiltins, and should not be",
   "   edited by hand.  See the source to mkbuiltins for details. */",
   "",
-  "/* Copyright (C) 1987-2015 Free Software Foundation, Inc.",
+  "/* Copyright (C) 1987-2021 Free Software Foundation, Inc.",
   "",
   "   This file is part of GNU Bash, the Bourne Again SHell.",
   "",
@@ -1250,12 +1264,13 @@ write_builtins (defs, structfile, externfile)
 		  else
 		    fprintf (structfile, "(sh_builtin_func_t *)0x0, ");
 
-		  fprintf (structfile, "%s%s%s%s%s, %s_doc,\n",
+		  fprintf (structfile, "%s%s%s%s%s%s, %s_doc,\n",
 		    "BUILTIN_ENABLED | STATIC_BUILTIN",
 		    (builtin->flags & BUILTIN_FLAG_SPECIAL) ? " | SPECIAL_BUILTIN" : "",
 		    (builtin->flags & BUILTIN_FLAG_ASSIGNMENT) ? " | ASSIGNMENT_BUILTIN" : "",
 		    (builtin->flags & BUILTIN_FLAG_LOCALVAR) ? " | LOCALVAR_BUILTIN" : "",
 		    (builtin->flags & BUILTIN_FLAG_POSIX_BUILTIN) ? " | POSIX_BUILTIN" : "",
+		    (builtin->flags & BUILTIN_FLAG_ARRAYREF_ARG) ? " | ARRAYREF_BUILTIN" : "",
 		    document_name (builtin));
 
 		  /* Don't translate short document summaries that are identical
@@ -1645,6 +1660,13 @@ is_posix_builtin (name)
   return (_find_in_table (name, posix_builtins));
 }
 
+static int
+is_arrayvar_builtin (name)
+     char *name;
+{
+  return (_find_in_table (name, arrayvar_builtins));
+}
+
 #if !defined (HAVE_RENAME)
 static int
 rename (from, to)

builtins/set.def

@@ -959,8 +959,15 @@ unset_builtin (list)
 #if defined (ARRAY_VARS)
       if (var && unset_array)
 	{
+	  int tvflags;
+
+	  tvflags = vflags;
+#if 0	/* TAG:bash-5.2 */
+	  if (shell_compatibility_level <= 51)
+	    tvflags |= VA_ALLOWALL;
+#endif
 	  /* Let unbind_array_element decide what to do with non-array vars */
-	  tem = unbind_array_element (var, t, vflags);	/* XXX new third arg */
+	  tem = unbind_array_element (var, t, tvflags);	/* XXX new third arg */
 	  if (tem == -2 && array_p (var) == 0 && assoc_p (var) == 0)
 	    {
 	      builtin_error (_("%s: not an array variable"), var->name);

builtins/shopt.def

@@ -1,7 +1,7 @@
 This file is shopt.def, from which is created shopt.c.
 It implements the Bash `shopt' builtin.
 
-Copyright (C) 1994-2020 Free Software Foundation, Inc.
+Copyright (C) 1994-2021 Free Software Foundation, Inc.
 
 This file is part of GNU Bash, the Bourne Again SHell.
 
@@ -120,6 +120,7 @@ extern int debugging_mode;
 #if defined (ARRAY_VARS)
 extern int assoc_expand_once;
 extern int array_expand_once;
+int expand_once_flag;
 #endif
 
 #if defined (SYSLOG_HISTORY)
@@ -140,6 +141,10 @@ static int shopt_enable_hostname_completion PARAMS((char *, int));
 static int shopt_set_complete_direxpand PARAMS((char *, int));
 #endif
 
+#if defined (ARRAY_VARS)
+static int set_assoc_expand PARAMS((char *, int));
+#endif
+
 static int shopt_set_debug_mode PARAMS((char *, int));
 
 static int shopt_login_shell;
@@ -163,7 +168,7 @@ static struct {
 } shopt_vars[] = {
   { "autocd", &autocd, (shopt_set_func_t *)NULL },
 #if defined (ARRAY_VARS)
-  { "assoc_expand_once", &assoc_expand_once, (shopt_set_func_t *)NULL },
+  { "assoc_expand_once", &expand_once_flag, set_assoc_expand },
 #endif
   { "cdable_vars", &cdable_vars, (shopt_set_func_t *)NULL },
   { "cdspell", &cdspelling, (shopt_set_func_t *)NULL },
@@ -362,7 +367,7 @@ reset_shopt_options ()
 #endif
 
 #if defined (ARRAY_VARS)
-  assoc_expand_once = 0;
+  expand_once_flag = assoc_expand_once = 0;
 #endif
 
 #if defined (HISTORY)
@@ -897,3 +902,17 @@ initialize_bashopts (no_bashopts)
   /* Set up the $BASHOPTS variable. */
   set_bashopts ();
 }
+
+#if defined (ARRAY_VARS)
+static int
+set_assoc_expand (option_name, mode)
+     char *option_name;
+     int mode;
+{
+#if 0 /* TAG: bash-5.2, fix shell_compatibility_level test */
+  if (shell_compatibility_level <= 51)
+#endif
+    assoc_expand_once = expand_once_flag;
+  return 0;
+}
+#endif

examples/loadables/cat.c

@@ -36,13 +36,24 @@ extern char *strerror ();
 extern char **make_builtin_argv ();
 
 static int
-fcopy(fd)
+fcopy(fd, fn)
 int	fd;
+char	*fn;
 {
 	char	buf[1024], *s;
 	int	n, w, e;
 
 	while (n = read(fd, buf, sizeof (buf))) {
+		if (n < 0) {
+			e = errno;
+			write(2, "cat: read error: ", 18);
+			write(2, fn, strlen(fn));
+			write(2, ": ", 2);
+			s = strerror(e);
+			write(2, s, strlen(s));
+			write(2, "\n", 1);
+			return 1;
+		}
 		w = write(1, buf, n);
 		if (w != n) {
 			e = errno;
@@ -65,7 +76,7 @@ char	**argv;
 	char	*s;
 
 	if (argc == 1)
-		return (fcopy(0));
+		return (fcopy(0, "standard input"));
 
 	for (i = r = 1; i < argc; i++) {
 		if (argv[i][0] == '-' && argv[i][1] == '\0')
@@ -82,7 +93,7 @@ char	**argv;
 				continue;
 			}
 		}
-		r = fcopy(fd);
+		r = fcopy(fd, argv[i]);
 		if (fd != 0)
 			close(fd);
 	}

execute_cmd.c

@@ -165,6 +165,7 @@ static int execute_while_or_until PARAMS((WHILE_COM *, int));
 static int execute_if_command PARAMS((IF_COM *));
 static int execute_null_command PARAMS((REDIRECT *, int, int, int));
 static void fix_assignment_words PARAMS((WORD_LIST *));
+static void fix_arrayref_words PARAMS((WORD_LIST *));
 static int execute_simple_command PARAMS((SIMPLE_COM *, int, int, int, struct fd_bitmap *));
 static int execute_builtin PARAMS((sh_builtin_func_t *, WORD_LIST *, int, int));
 static int execute_function PARAMS((SHELL_VAR *, WORD_LIST *, int, struct fd_bitmap *, int, int));
@@ -3891,16 +3892,20 @@ execute_cond_node (cond)
 	arg1 = nullstr;
       if (echo_command_at_execute)
 	xtrace_print_cond_term (cond->type, invert, cond->op, arg1, (char *)NULL);
-      /* TAG:bash-5.2 fix up later with set_expand_once() */
-      oa = assoc_expand_once;
+      /* TAG:bash-5.2 */
 #if 0
-      if (varop && shell_compatibility_level > 51)
+      if (varop && shell_compatibility_level > 51)		/* -v */
 #else
       if (varop)
 #endif
-	assoc_expand_once = 0;
+	oa = set_expand_once (0, 0);
       result = unary_test (cond->op->word, arg1, varflag) ? EXECUTION_SUCCESS : EXECUTION_FAILURE;
-      assoc_expand_once = oa;
+#if 0
+      if (varop && shell_compatibility_level > 51)		/* -v */
+#else
+      if (varop)
+#endif
+	assoc_expand_once = oa;
       if (arg1 != nullstr)
 	free (arg1);
     }
@@ -4218,6 +4223,48 @@ fix_assignment_words (words)
       }
 }
 
+#if defined (ARRAY_VARS)
+/* Set W_ARRAYREF on words that are valid array references to a builtin that
+   accepts them. This is intended to eventually comnletely take the place of
+   assoc_expand_once. */
+static void
+fix_arrayref_words (words)
+     WORD_LIST *words;
+{
+  WORD_LIST *w, *wcmd;
+  struct builtin *b;
+
+  if (words == 0)
+    return;
+
+  b = 0;
+
+  /* Skip over assignment statements preceding a command name */
+  wcmd = words;
+  for (wcmd = words; wcmd; wcmd = wcmd->next)
+    if ((wcmd->word->flags & W_ASSIGNMENT) == 0)
+      break;
+
+  /* Skip over `command' */
+  while (wcmd && wcmd->word && wcmd->word->word && STREQ (wcmd->word->word, "command"))
+    wcmd = wcmd->next;
+
+  if (wcmd == 0)
+    return;
+
+  /* If it's not an array reference builtin, we have nothing to do. */
+  b = builtin_address_internal (wcmd->word->word, 0);
+  if (b == 0 || (b->flags & ARRAYREF_BUILTIN) == 0)
+    return;
+
+  for (w = wcmd->next; w; w = w->next)
+    {
+      if (w->word && w->word->word && valid_array_reference (w->word->word, VA_NOEXPAND))
+	w->word->flags |= W_ARRAYREF;
+    }
+}
+#endif
+
 #ifndef ISOPTION
 #  define ISOPTION(s, c)  (s[0] == '-' && s[1] == c && s[2] == 0)
 #endif
@@ -4428,6 +4475,9 @@ execute_simple_command (simple_command, pipe_in, pipe_out, async, fds_to_close)
     {
       current_fds_to_close = fds_to_close;
       fix_assignment_words (simple_command->words);
+#if defined (ARRAY_VARS)
+      fix_arrayref_words (simple_command->words);
+#endif
       /* Pass the ignore return flag down to command substitutions */
       if (cmdflags & CMD_IGNORE_RETURN)	/* XXX */
 	comsub_ignore_return++;

subst.c

@@ -10442,9 +10442,9 @@ add_string:
 #if defined (ARRAY_VARS)
 	case '[':		/*]*/
 #if 0	/* TAG:bash-5.2 */
-	  if (((quoted & Q_ARITH) == 0 && (word->flags & W_ARRAYREF) == 0) || (shell_compatibility_level <= 51))
+	  if ((quoted & Q_ARITH) == 0 || shell_compatibility_level <= 51)
 #else
-	  if (((quoted & Q_ARITH) == 0 && (word->flags & W_ARRAYREF) == 0))
+	  if ((quoted & Q_ARITH) == 0)
 #endif
 	    {
 	      if (isexp == 0 && (word->flags & (W_NOSPLIT|W_NOSPLIT2)) == 0 && isifs (c) && (quoted & (Q_DOUBLE_QUOTES|Q_HERE_DOCUMENT)) == 0)

tests/array.right

@@ -493,7 +493,7 @@ declare -a a=([1]="2" [2]="3" [3]="4")
 abcd
 unset
 ./array21.sub: line 30: typeset: a: not found
-./array21.sub: line 33: typeset: A: not found
+declare -A A=([four]="4" [two]="2" [three]="3" [one]="1" )
 declare -a a=()
 declare -A A=()
 declare -a foo=([0]="1" [1]="(4 5 6)" [2]="3")

tests/quotearray.right

@@ -79,7 +79,6 @@ declare -A a=()
 declare -A a=(["\$(echo foo)"]="1" )
 declare -A a=()
 declare -A a=(["\$(echo foo)"]="1" )
-./quotearray3.sub: line 66: typeset: assoc: not found
-1
-./quotearray3.sub: line 71: \@: syntax error: operand expected (error token is "\@")
+declare -A assoc=(["!"]="bang" )
+0
 1

tests/quotearray3.sub

@@ -62,9 +62,11 @@ key=@
 assoc[@]=at
 assoc[!]=bang
 
+# this should only unset the element with key `@'
 unset -v assoc[$key]
 typeset -p assoc
 
+# this doesn't work yet
 test -v assoc[$key]
 echo $?