Re: Curious... on readline history patch -- round up memory allocs?

[Eduardo Bustamante]

The change was in reference to this bug report:
http://lists.gnu.org/archive/html/bug-bash/2016-09/msg00107.html

The problem was that bash tried to allocate memory from the start
based on the value of HISTSIZE, but this proved problematic for users
which used a large HISTSIZE to have unlimited history.

The commit is here
http://git.savannah.gnu.org/cgit/bash.git/commit/?h=devel=61c476d20d32dfd389c79fd4f2161a780685e42e

And the entry in the changelog:

+lib/readine/history.c
+ - add_history: if allocating the history list for the first time,
+ make sure the max history list size isn't so large that it will
+ cause allocation errors. Cap it at MAX_HISTORY_INITIAL_SIZE
+ (8192). Reported by Sean Zha 

Re: Curious... on readline history patch -- round up memory allocs?

[L. A. Walsh]

Eduardo Bustamante wrote:

I think this is unnecessary, malloc (either the bash malloc in
lib/malloc/malloc.c or the libc provided malloc) should already take
care of requesting memory in page sized chunks.

At least that's what I see here (morecore function):
http://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c?h=devel=216e2e9b8ba21fff677cf7794ef3d9af8c91d46d#n622

Unless I'm missing something, there's nothing to gain from optimizing
on top of xmalloc/malloc.
  

---
That's fine -- I was just reading the comment about how 4.4 changed
history allocs to optimize memory reallocs and movement.  So if things
were fine with malloc before, I'm not sure why they were optimized
within bash in 4.4...?

Re: Curious... on readline history patch -- round up memory allocs?

[Eduardo Bustamante]

I think this is unnecessary, malloc (either the bash malloc in
lib/malloc/malloc.c or the libc provided malloc) should already take
care of requesting memory in page sized chunks.

At least that's what I see here (morecore function):
http://git.savannah.gnu.org/cgit/bash.git/tree/lib/malloc/malloc.c?h=devel=216e2e9b8ba21fff677cf7794ef3d9af8c91d46d#n622

Unless I'm missing something, there's nothing to gain from optimizing
on top of xmalloc/malloc.

Curious... on readline history patch -- round up memory allocs?

[L. A. Walsh]

Saw the bit about bash-4.4 changing things to reduce reallocs/copies,
and wondered if you'd thought about rounding up the allocations
to the nearest page size (at least on linux):

Something along the lines of:
Ishtar:tools/bash/readline-7.0> diff -u history.c.orig history.c
--- history.c 2015-12-28 10:50:31.0 -0800
+++ history.c 2016-11-15 12:36:51.452105223 -0800
@@ -57,6 +57,12 @@
/* How big to make the_history when we first allocate it. */
#define DEFAULT_HISTORY_INITIAL_SIZE 502

+#define MAX_HISTORY_INITIAL_SIZE 8192
+
+#define DEFAULT_PAGE_SIZE 8192
+
+#define ROUND_UP_TO_PAGE_SIZE(x) ( x + DEFAULT_PAGE_SIZE-1) & 
~(DEFAULT_PAGE_SIZE-1)

+
/* The number of slots to increase the_history by. */
#define DEFAULT_HISTORY_GROW_SIZE 50

@@ -310,16 +316,19 @@
 history_size = history_max_entries + 2;
   else
 history_size = DEFAULT_HISTORY_INITIAL_SIZE;
-   the_history = (HIST_ENTRY **)xmalloc (history_size * sizeof 
(HIST_ENTRY *));
+   the_history = (HIST_ENTRY **)xmalloc 
(ROUND_UP_TO_PAGE_SIZE(history_size * sizeof (HIST_ENTRY *)));

   history_length = 1;
 }
  else
 {
   if (history_length == (history_size - 1))
 {
-   history_size += DEFAULT_HISTORY_GROW_SIZE;
+   history_size = (history_max_entries > MAX_HISTORY_INITIAL_SIZE)
+ ? MAX_HISTORY_INITIAL_SIZE
+ : history_max_entries + 2;
   the_history = (HIST_ENTRY **)
-   xrealloc (the_history, history_size * sizeof (HIST_ENTRY *));
+   xrealloc (the_history, ROUND_UP_TO_PAGE_SIZE(history_size * sizeof 
(HIST_ENTRY *)));

+
 }
   history_length++;
 }


Note, since I'm having probs building bash right now, the above
isn't execution tested, but it does compile!  ;-)

probs making bash-4.4(+ patches)

[L. A. Walsh]

Still having probs building bash-4.4 -- trying first, the builtin
version of readline, and on running got:

readline: ~/.inputrc: line 6: history-size=-1: unknown variable name
readline: ~/.inputrc: line 35: completion-prefix-display-length=256: 
unknown variable name

readline: ~/.inputrc: line 37: expand-tilda: unknown variable name
readline: ~/.inputrc: line 42: quoted-insert: unknown variable name
readline: ~/.inputrc: line 44: tab-insert: unknown variable name
Ishtar:tools/bash/bash-4.4> echo $BASH_VERSION
4.4.5(1)-release

---
Next I tried using installed readline (libreadline.so.6.2 **1)
and got an error in the final link step:

bashline.o:bashline.c:function bash_execute_unix_command: error: 
undefined reference to 'rl_executing_keyseq'
bashline.o:bashline.c:function bash_execute_unix_command: error: 
undefined reference to 'rl_clear_visible_line'
bashline.o:bashline.c:function bash_execute_unix_command: error: 
undefined reference to 'rl_redraw_prompt_last_line'
bashline.o:bashline.c:function bashline_set_event_hook: error: undefined 
reference to 'rl_signal_event_hook'
bashline.o:bashline.c:function bashline_reset_event_hook: error: 
undefined reference to 'rl_signal_event_hook'
bashline.o:bashline.c:function command_word_completion_function: error: 
undefined reference to 'rl_filename_stat_hook'
bashline.o:bashline.c:function attempt_shell_completion: error: 
undefined reference to 'rl_filename_stat_hook'
bashline.o:bashline.c:function bashline_reset: error: undefined 
reference to 'rl_filename_stat_hook'
bashline.o:bashline.c:function initialize_readline: error: undefined 
reference to 'rl_filename_stat_hook'
./lib/sh/libsh.a(tmpfile.o):tmpfile.c:function sh_mktmpname: warning: 
the use of `mktemp' is dangerous, better use `mkstemp'

collect2: error: ld returned 1 exit status
Makefile:572: recipe for target 'bash' failed
make: *** [bash] Error 1

(see note '**1' below)

---
(3rd try)
Next I tried prepending the dir w/readline-7.0 to the
LDFLAGS but the patch for libreadline/history.c didn't make it
into the readline sources:

 bash

User law's .bashrc called 2nd time
bash: xmalloc: cannot allocate 8589934608 bytes (3088384 bytes allocated)

(4th try)
Next tried patching readline-7.0 along lines of bash-44-001.  Even though
I altered the LDPATH, it seems to be looking in //lib [sic]:

Note -- had problems making bash-4.4 w/o patches, so the patches
didn't break anything -- just didn't help as much as I'd hoped.


Link started off with:
/usr/bin/gcc -L./builtins -L//lib -L//lib -L./lib/glob -L./lib/tilde 
-L./lib/malloc -L./lib/sh -L/home/tools/bash/readline-7.0 -O2  -m64 


Note 1: On cygwin, '//lib' says to go out and try the root-share of machine
"lib"...probably not what is wanted?

Note 2: Shouldn't it be /lib64?  I had my libdir set for that...
From config:
CFLAGS="$CFLAGS" LD_FLAGS="$LDFLAGS" configure "--prefix=/ 
--exec-prefix=/ --bindir=/bin --sbindir=/sbin --datadir=/usr/share 
--datarootdir=/usr/share --includedir=/include --oldincludedir=/include 
--libdir=/lib64 --libexecdir=/lib64/bash [...]





**1 - since it is linking against /lib instead of /lib64, it was really
linking against "libreadline.so.5.2" -- so maybe if it can listen
to "libdir" it might link with a 6.x version of readline -- and that
might work -- but still -- shouldn't my prepending the location
of my libreadline dir, allow it to pick up Version 7?



So can I link this with version 7, or will that not work?  Why would
the builtin readline give me errors reading my current inputrc?  And
3rd, why does the config/make ignore my setting of libdir?


Thanks & sorry for so much at once, but after the patches, thought
it should work.   Maybe if I hacked the makefile or something...?

Re: bash integer overflow (?) with large HISTFILESIZE

[Greg Wooledge]

On Mon, Nov 14, 2016 at 08:39:27PM -0500, Patrick Donnelly wrote:
> Bash Version: 4.4
> Patch Level: 0
> Release Status: release
> 
> Description:
> 
> With the latest version of bash just deployed in the Arch Linux
> repositories, bash fails with HISTSIZE=2147483647:
> 
> bash: xmalloc: cannot allocate 18446744056529682440 bytes

This was addressed with patch bash44-001:
http://ftp.gnu.org/pub/gnu/bash/bash-4.4-patches/bash44-001

bash integer overflow (?) with large HISTFILESIZE

[Patrick Donnelly]

Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS:  -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-unknown-linux-gnu'
-DCONF_VENDOR='unknown' -DLOCALEDIR='/usr/share/locale'
-DPACKAGE='bash' -DSHELL -DHAVE_CONFIG_H   -I.  -I. -I./include
-I./lib  -D_FORTIFY_SOURCE=2 -march=x86-64 -mtune=generic -O2 -pipe
-fstack-protector-strong
-DDEFAULT_PATH_VALUE='/usr/local/sbin:/usr/local/bin:/usr/bin'
-DSTANDARD_UTILS_PATH='/usr/bin' -DSYS_BASHRC='/etc/bash.bashrc'
-DSYS_BASH_LOGOUT='/etc/bash.bash_logout' -Wno-parentheses
-Wno-format-security
uname output: Linux icewind 4.8.7-1-ARCH #1 SMP PREEMPT Thu Nov 10
17:22:48 CET 2016 x86_64 GNU/Linux
Machine Type: x86_64-unknown-linux-gnu

Bash Version: 4.4
Patch Level: 0
Release Status: release

Description:

With the latest version of bash just deployed in the Arch Linux
repositories, bash fails with HISTSIZE=2147483647:

bash: xmalloc: cannot allocate 18446744056529682440 bytes


Repeat-By:

$ env HISTSIZE=2147483647 /bin/bash --norc
bash: xmalloc: cannot allocate 18446744056529682440 bytes

Fix:
[Description of how to fix the problem.  If you don't know a
fix for the problem, don't include this section.]

-- 
Patrick Donnelly