readline fix for rl_undo_list pointer aliasing; arith command sets word_top

CWRU/CWRU.chlog

@@ -5216,7 +5216,7 @@ builtins/printf.def
 builtins/set.def
 	- unset_builtin: since tokenize_array_reference modifies its NAME
 	  argument, we need to restore the original word if there is no
-	  array variable found with that name, so we can do ahead and try to
+	  array variable found with that name, so we can go ahead and try to
 	  unset a function with that wonky name. Inspired by a report from
 	  Emanuele Torre <torreemanuele6@gmail.com>
 
@@ -5295,7 +5295,7 @@ doc/{bash.1,bashref.texi}
 				    2/8
 				    ---
 bashline.c
-	- set_up_new__line: if the new line doesn't change rl_line_buffer,
+	- set_up_new_line: if the new line doesn't change rl_line_buffer,
 	  just return right away without changing rl_point. Affects
 	  alias-expand-line, history-expand-line, and history-and-alias-expand-line.
 	  Inspired by a report from
@@ -5496,3 +5496,41 @@ lib/readline/search.c
 	- rl_history_search_reinit: change check against history_string_size
 	  to account for history_string_size now being a size_t. Report and
 	  fix from Grisha Levit <grishalevit@gmail.com>
+
+				    3/2
+				    ---
+lib/readline/history.c
+	- _hs_search_history_data: search the history list for an entry with
+	  `data' matching the argument; return the index
+	- _hs_replace_history_data: search backwards through the history list
+	  from the end
+
+lib/readline/histlib.h
+	- _hs_search_history_data: extern declaration
+
+lib/readline/search.c
+	- make_history_line_current: make sure rl_undo_list doesn't appear in
+	  any history entries (it should be a private search string undo list)
+	  before freeing it
+
+lib/readline/misc.c
+	- _rl_free_saved_history_line: make sure the undo list in
+	  _rl_saved_line_for_history isn't rl_undo_list and doesn't appear in
+	  any history list entries as `data' before freeing it. Fixes core
+	  dump after SIGINT reported by Grisha Levit <grishalevit@gmail.com>
+
+parse.y,print_cmd.c
+	- changes for size_t when computing new amount for xrealloc
+
+				    3/3
+				    ---
+parse.y
+	- set_word_top: new inline function to check and set word_top; called
+	  from the appropriate places
+	- parse_dparen: call set_word_top before parsing an arith command or
+	  nested subshell. Fixes underflow issue reported by
+	  Grisha Levit <grishalevit@gmail.com>
+
+examples/loadables/kv.c
+	- kv: new loadable builtin, reads key-value pairs from stdin and assigns
+	  them to an associative array

MANIFEST

@@ -737,6 +737,7 @@ examples/loadables/finfo.c	f
 examples/loadables/cat.c	f
 examples/loadables/csv.c	f
 examples/loadables/dsv.c	f
+examples/loadables/kv.c		f
 examples/loadables/cut.c	f
 examples/loadables/logname.c	f
 examples/loadables/basename.c	f

examples/loadables/Makefile.in

@@ -104,7 +104,7 @@ INC = -I. -I.. -I$(topdir) -I$(topdir)/lib -I$(topdir)/builtins -I${srcdir} \
 ALLPROG = print truefalse sleep finfo logname basename dirname fdflags \
 	  tty pathchk tee head mkdir rmdir mkfifo mktemp printenv id whoami \
 	  uname sync push ln unlink realpath strftime mypid setpgid seq rm \
-	  accept csv dsv cut stat getconf
+	  accept csv dsv cut stat getconf kv
 OTHERPROG = necho hello cat pushd asort
 
 all:	$(SHOBJ_STATUS)
@@ -225,6 +225,9 @@ csv:	csv.o
 dsv:	dsv.o
 	$(SHOBJ_LD) $(SHOBJ_LDFLAGS) $(SHOBJ_XLDFLAGS) -o $@ dsv.o $(SHOBJ_LIBS)
 
+kv:	kv.o
+	$(SHOBJ_LD) $(SHOBJ_LDFLAGS) $(SHOBJ_XLDFLAGS) -o $@ kv.o $(SHOBJ_LIBS)
+
 cut:	cut.o
 	$(SHOBJ_LD) $(SHOBJ_LDFLAGS) $(SHOBJ_XLDFLAGS) -o $@ cut.o $(SHOBJ_LIBS)
 
@@ -317,6 +320,7 @@ hello.o: hello.c
 cat.o: cat.c
 csv.o: csv.c
 dsv.o: dsv.c
+kv.o: kv.c
 cut.o: cut.c
 printenv.o: printenv.c
 id.o: id.c

examples/loadables/kv.c

@@ -0,0 +1,230 @@
+/* kv - process a series of lines containing key-value pairs and assign them
+        to an associative array. */
+
+/*
+   Copyright (C) 2023 Free Software Foundation, Inc.
+
+   Bash is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   Bash is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with Bash.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+/* See Makefile for compilation details. */
+
+#include <config.h>
+
+#include <sys/types.h>
+#if defined (HAVE_UNISTD_H)
+#  include <unistd.h>
+#endif
+#include "bashansi.h"
+#include "posixstat.h"
+#include <stdio.h>
+#include <errno.h>
+
+#include "loadables.h"
+
+#ifndef errno
+extern int errno;
+#endif
+
+#define KV_ARRAY_DEFAULT	"KV"
+
+/* Split LINE into a key and value, with the delimiter between the key and
+   value being a member of DSTRING. A sequence of one or more delimiters
+   separates the key and value. Assign to associative array KV as if
+   KV[key]=value without expansion. This does not treat single or double
+   quotes specially, nor does it remove whitespace at the beginning or end
+   of LINE. */
+static int
+kvsplit (SHELL_VAR *v, char *line, char *dstring)
+{
+  char *key, *value;
+  size_t ind;
+
+  key = line;
+  value = 0;
+
+  ind = (line && *line) ? strcspn (key, dstring) : 0;
+
+  /* blank line or line starting with delimiter */
+  if (ind == 0 || *key == 0)
+    return 0;
+
+  if (key[ind])
+    {
+      key[ind++] = '\0';
+      value = key + ind;
+      /* skip until non-delim; this allows things like key1 = value1 where delims = " =" */
+      ind = strspn (value, dstring);
+      value += ind;
+    }
+  else
+    value = "";
+
+  return (bind_assoc_variable (v, name_cell (v), savestring (key), savestring (value), 0) != 0);
+}
+
+int
+kvfile (SHELL_VAR *v, int fd, char *delims, char *rs)
+{
+  ssize_t n;
+  char *line;
+  size_t llen;
+  int unbuffered_read, nr;
+  struct stat sb;
+
+  nr = 0;
+#ifndef __CYGWIN__
+  /* We probably don't need to worry about setting this at all; we're not
+     seeking back and forth yet. */
+  if (*rs == '\n')
+    unbuffered_read = (lseek (fd, 0L, SEEK_CUR) < 0) && (errno == ESPIPE);
+  else
+    unbuffered_read = (fstat (fd, &sb) != 0) || (S_ISREG (sb.st_mode) == 0);
+#else
+  unbuffered_read = 1;
+#endif
+
+  line = 0;
+  llen = 0;
+
+  zreset ();
+  while ((n = zgetline (fd, &line, &llen, *rs, unbuffered_read)) != -1)
+    {
+      QUIT;
+      if (line[n] == *rs)
+	line[n] = '\0';		/* value doesn't include the record separator */
+      nr += kvsplit (v, line, delims);
+      free (line);
+      line = 0;
+      llen = 0;
+    }
+
+  QUIT;
+  return nr;  
+}
+
+int
+kv_builtin (WORD_LIST *list)
+{
+  int opt, rval;
+  char *array_name, *delims, *rs;
+  SHELL_VAR *v;
+
+  array_name = delims = rs = 0;
+  rval = EXECUTION_SUCCESS;
+
+  reset_internal_getopt ();
+  while ((opt = internal_getopt (list, "A:s:d:")) != -1)
+    {
+      switch (opt)
+	{
+	case 'A':
+	  array_name = list_optarg;
+	  break;
+	case 's':
+	  delims = list_optarg;
+	  break;
+	case 'd':
+	  rs = list_optarg;
+	  break;
+	CASE_HELPOPT;
+	default:
+	  builtin_usage ();
+	  return (EX_USAGE);
+	}
+    }
+  list = loptend;
+
+  if (list)
+    {
+      builtin_error ("too many arguments");
+      builtin_usage ();
+      return (EX_USAGE);
+    }
+
+  if (array_name == 0)
+    array_name = KV_ARRAY_DEFAULT;
+
+  if (legal_identifier (array_name) == 0)
+    {
+      sh_invalidid (array_name);
+      return (EXECUTION_FAILURE);
+    }
+
+  if (delims == 0)
+    delims = getifs ();
+  if (rs == 0)
+    rs = "\n";
+
+  v = find_or_make_array_variable (array_name, 3);
+  if (v == 0 || readonly_p (v) || noassign_p (v))
+    {
+      if (v && readonly_p (v))
+	err_readonly (array_name);
+      return (EXECUTION_FAILURE);
+    }
+  else if (assoc_p (v) == 0)
+    {
+      builtin_error ("%s: not an associative array", array_name);
+      return (EXECUTION_FAILURE);
+    }
+  if (invisible_p (v))
+    VUNSETATTR (v, att_invisible);
+  assoc_flush (assoc_cell (v));
+
+  rval = kvfile (v, 0, delims, rs);
+
+  return (rval > 0 ? EXECUTION_SUCCESS : EXECUTION_FAILURE);
+}
+
+/* Called when builtin is enabled and loaded from the shared object.  If this
+   function returns 0, the load fails. */
+int
+kv_builtin_load (char *name)
+{
+  return (1);
+}
+
+/* Called when builtin is disabled. */
+void
+kv_builtin_unload (char *name)
+{
+}
+
+char *kv_doc[] = {
+	"Read key-value pairs into an associative array.",
+	"",
+	"Read delimiter-terminated records composed of a single key-value pair",
+	"from the standard input and add the key and corresponding value",
+	"to the associative array ARRAYNAME. The key and value are separated",
+	"by a sequence of one or more characters in SEPARATORS. Records are",
+	"terminated by the first character of RS, similar to the read and",
+	"mapfile builtins.",
+	"",
+	"If SEPARATORS is not supplied, $IFS is used to separate the keys",
+	"and values. If RS is not supplied, newlines terminate records.",
+	"If ARRAYNAME is not supplied, \"KV\" is the default array name.",
+	"",
+	"Returns success if at least one key-value pair is stored in ARRAYNAME.",
+	(char *)NULL
+};
+
+struct builtin kv_struct = {
+	"kv",			/* builtin name */
+	kv_builtin,		/* function implementing the builtin */
+	BUILTIN_ENABLED,	/* initial flags for builtin */
+	kv_doc,		/* array of long documentation strings. */
+	"kv [-A ARRAYNAME] [-s SEPARATORS] [-d RS]",	/* usage synopsis; becomes short_doc */
+	0			/* reserved for internal use */
+};

lib/readline/histlib.h

@@ -1,6 +1,6 @@
 /* histlib.h -- internal definitions for the history library. */
 
-/* Copyright (C) 1989-2009,2021-2022 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2009,2021-2023 Free Software Foundation, Inc.
 
    This file contains the GNU History Library (History), a set of
    routines for managing the text of previously typed lines.
@@ -91,6 +91,7 @@ extern int _hs_history_search (const char *, int, int);
 
 /* history.c */
 extern void _hs_replace_history_data (int, histdata_t *, histdata_t *);
+extern int _hs_search_history_data (histdata_t *);
 extern int _hs_at_end_of_history (void);
 
 /* histfile.c */

lib/readline/history.c

@@ -1,6 +1,6 @@
 /* history.c -- standalone history library */
 
-/* Copyright (C) 1989-2021 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2023 Free Software Foundation, Inc.
 
    This file contains the GNU History Library (History), a set of
    routines for managing the text of previously typed lines.
@@ -458,7 +458,7 @@ _hs_replace_history_data (int which, histdata_t *old, histdata_t *new)
     }
 
   last = -1;
-  for (i = 0; i < history_length; i++)
+  for (i = history_length - 1; i >= 0; i--)
     {
       entry = the_history[i];
       if (entry == 0)
@@ -475,7 +475,27 @@ _hs_replace_history_data (int which, histdata_t *old, histdata_t *new)
       entry = the_history[last];
       entry->data = new;	/* XXX - we don't check entry->old */
     }
-}      
+}
+
+int
+_hs_search_history_data (histdata_t *needle)
+{
+  register int i;
+  HIST_ENTRY *entry;
+
+  if (history_length == 0 || the_history == 0)
+    return -1;
+
+  for (i = history_length - 1; i >= 0; i--)
+    {
+      entry = the_history[i];
+      if (entry == 0)
+	continue;
+      if (entry->data == needle)
+	return i;
+    }
+  return -1;
+}
   
 /* Remove history element WHICH from the history.  The removed
    element is returned to you so you can free the line, data,

lib/readline/misc.c

@@ -340,6 +340,9 @@ rl_maybe_replace_line (void)
       xfree (temp->line);
       FREE (temp->timestamp);
       xfree (temp);
+      /* XXX - what about _rl_saved_line_for_history? if the saved undo list
+	 is rl_undo_list, and we just put that into a history entry, should
+	 we set the saved undo list to NULL? */
     }
   return 0;
 }
@@ -385,14 +388,16 @@ _rl_free_saved_history_line (void)
 {
   if (_rl_saved_line_for_history)
     {
-      if (rl_undo_list && rl_undo_list == (UNDO_LIST *)_rl_saved_line_for_history->data)
-	rl_undo_list = 0;
-      /* Have to free this separately because _rl_free_history entry can't:
-	 it doesn't know whether or not this has application data. Only the
-	 callers that know this is _rl_saved_line_for_history can know that
-	 it's an undo list. */
-      if (_rl_saved_line_for_history->data)
-	_rl_free_undo_list ((UNDO_LIST *)_rl_saved_line_for_history->data);
+      UNDO_LIST *sentinel;
+
+      sentinel = (UNDO_LIST *)_rl_saved_line_for_history->data;
+
+      /* We should only free `data' if it's not the current rl_undo_list and
+	 it's not the `data' member in a history entry somewhere. We have to
+	 free it separately because only the callers know it's an undo list. */
+      if (sentinel && sentinel != rl_undo_list && _hs_search_history_data ((histdata_t *)sentinel) < 0)
+	_rl_free_undo_list (sentinel);
+
       _rl_free_history_entry (_rl_saved_line_for_history);
       _rl_saved_line_for_history = (HIST_ENTRY *)NULL;
     }

lib/readline/search.c

@@ -88,8 +88,10 @@ make_history_line_current (HIST_ENTRY *entry)
 
   xlist = _rl_saved_line_for_history ? (UNDO_LIST *)_rl_saved_line_for_history->data : 0;
   /* At this point, rl_undo_list points to a private search string list. */
-  if (rl_undo_list && rl_undo_list != (UNDO_LIST *)entry->data && rl_undo_list != xlist)
+  if (rl_undo_list && rl_undo_list != (UNDO_LIST *)entry->data && rl_undo_list != xlist &&
+	_hs_search_history_data ((histdata_t *)rl_undo_list) < 0)
     rl_free_undo_list ();
+  rl_undo_list = 0;	/* XXX */
 
   /* Now we create a new undo list with a single insert for this text.
      WE DON'T CHANGE THE ORIGINAL HISTORY ENTRY UNDO LIST */

parse.y

@@ -184,6 +184,7 @@ static void free_string_list (void);
 
 static char *read_a_line (int);
 
+static int set_word_top (int);
 static int reserved_word_acceptable (int);
 static int yylex (void);
 
@@ -1097,7 +1098,6 @@ if_command:	IF compound_list THEN compound_list FI
 			  $$ = make_if_command ($2, $4, (COMMAND *)NULL);
   			  if (word_top >= 0) word_top--;
 			}
-			
 	|	IF compound_list THEN compound_list ELSE compound_list FI
 			{
 			  $$ = make_if_command ($2, $4, $6);
@@ -2575,7 +2575,7 @@ shell_getc (int remove_quoted_newline)
 	 not already end in an EOF character.  */
       if (shell_input_line_terminator != EOF && shell_input_line_terminator != READERR)
 	{
-	  if (shell_input_line_size < SIZE_MAX-3 && (shell_input_line_len+3 > shell_input_line_size))
+	  if (shell_input_line_size + 3 < SIZE_MAX && (shell_input_line_len+3 > shell_input_line_size))
 	    shell_input_line = (char *)xrealloc (shell_input_line,
 					1 + (shell_input_line_size += 2));
 
@@ -3039,11 +3039,7 @@ static int open_brace_count;
 		open_brace_count--; \
 \
 	      if (last_read_token == IF || last_read_token == WHILE || last_read_token == UNTIL) \
-		{ \
-		  if (word_top < MAX_COMPOUND_NEST) \
-		    word_top++; \
-		  word_lineno[word_top] = line_number; \
-		} \
+		set_word_top (last_read_token); \
 \
 	      if (posixly_correct) \
 		parser_state &= ~PST_ALEXPNEXT; \
@@ -4614,9 +4610,8 @@ parse_dparen (int c)
 #if defined (ARITH_FOR_COMMAND)
   if (last_read_token == FOR)
     {
-      if (word_top < MAX_COMPOUND_NEST)
-	word_top++;
-      arith_for_lineno = word_lineno[word_top] = line_number;
+      set_word_top (last_read_token);
+      arith_for_lineno = line_number;
       cmdtyp = parse_arith_cmd (&wval, 0);
       if (cmdtyp == 1)
 	{
@@ -4635,6 +4630,8 @@ parse_dparen (int c)
     {
       sline = line_number;
 
+      if (last_read_token == IF || last_read_token == WHILE || last_read_token == UNTIL)
+	set_word_top (last_read_token);
       cmdtyp = parse_arith_cmd (&wval, 0);
       if (cmdtyp == 1)	/* arithmetic command */
 	{
@@ -5506,11 +5503,22 @@ got_token:
     case CASE:
     case SELECT:
     case FOR:
-      if (word_top < MAX_COMPOUND_NEST)
-	word_top++;
-      word_lineno[word_top] = line_number;
       expecting_in_token++;
       break;
+    }
+  set_word_top (last_read_token);
+
+  return (result);
+}
+
+static inline int
+set_word_top (int t)
+{
+  switch (t)
+    {
+    case CASE:
+    case SELECT:
+    case FOR:
     case IF:
     case WHILE:
     case UNTIL:
@@ -5518,9 +5526,10 @@ got_token:
 	word_top++;
       word_lineno[word_top] = line_number;
       break;
+    default:
+      break;
     }
-
-  return (result);
+  return word_top;
 }
 
 /* Return 1 if TOKSYM is a token that after being read would allow

print_cmd.c

@@ -477,7 +477,7 @@ indirection_level_string (void)
   /* Dynamically resize indirection_string so we have room for everything
      and we don't have to truncate ps4 */
   ineed = (ps4_firstc_len * indirection_level) + strlen (ps4);
-  if (ineed > indirection_stringsiz - 1)
+  if (ineed + 1 > indirection_stringsiz)
     {
       indirection_stringsiz = ineed + 1;
       indirection_string = xrealloc (indirection_string, indirection_stringsiz);

tests/func.right

@@ -166,4 +166,27 @@ after FUNCNEST unset: f = 201
 ./func4.sub: line 23: foo: maximum function nesting level exceeded (20)
 1
 after FUNCNEST assign: f = 38
+11111 () 
+{ 
+    printf "FUNCNAME: %s\n" $FUNCNAME
+}
+function a=2 () 
+{ 
+    printf "FUNCNAME: %s\n" $FUNCNAME
+}
+function 11111 () 
+{ 
+    printf "FUNCNAME: %s\n" $FUNCNAME
+}
+function a=2 () 
+{ 
+    printf "FUNCNAME: %s\n" $FUNCNAME
+}
+FUNCNAME: a=2
+break is a function
+break () 
+{ 
+    echo FUNCNAME: $FUNCNAME
+}
+FUNCNAME: break
 5

tests/func.tests

@@ -166,6 +166,7 @@ export -f zf
 
 ${THIS_SH} -c 'type -t zf'
 ${THIS_SH} -c 'type zf'
+unset -f zf
 
 ${THIS_SH} ./func1.sub
 
@@ -179,6 +180,9 @@ ${THIS_SH} ./func3.sub
 # FUNCNEST testing
 ${THIS_SH} ./func4.sub
 
+# function naming restrictions
+${THIS_SH} ./func5.sub
+
 unset -f myfunction
 myfunction() {
     echo "bad shell function redirection"

tests/func5.sub

@@ -0,0 +1,38 @@
+#   This program is free software: you can redistribute it and/or modify
+#   it under the terms of the GNU General Public License as published by
+#   the Free Software Foundation, either version 3 of the License, or
+#   (at your option) any later version.
+#
+#   This program is distributed in the hope that it will be useful,
+#   but WITHOUT ANY WARRANTY; without even the implied warranty of
+#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#   GNU General Public License for more details.
+#
+#   You should have received a copy of the GNU General Public License
+#   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+function a=2
+{
+	printf "FUNCNAME: %s\n" $FUNCNAME
+}
+
+function 11111
+{
+	printf "FUNCNAME: %s\n" $FUNCNAME
+}
+
+declare -f
+set -o posix
+declare -f
+set +o posix
+
+a\=2
+
+break()
+{
+	echo FUNCNAME: $FUNCNAME
+}
+
+type break
+\break

tests/parser.tests

@@ -1,5 +1,12 @@
 # catch-all for parsing problems that don't fit anywhere else
 
+# word_top issues in bash-5.2
+case x in x) if ((1)); then :; fi ;; esac
+case x in x) if ((1)); then :; fi esac
+
+case x in x) if ((true ) ); then :; fi ;; esac
+case x in x) if ((true ) ); then :; fi esac
+
 # this has to be in a separate file to get desired EOF behavior
 ${THIS_SH} ./parser1.sub