Thanks for the fix. This problem was reported as Bug 24980 - Valgrind reports a 'jump on uninit' in readline https://sourceware.org/bugzilla/show_bug.cgi?id=24980
Philippe On Wed, 2019-09-18 at 16:11 -0400, Andrew Burgess wrote: > This commit in binutils-gdb: > > commit 830b67068cebe7db0eb0db3fa19244e03859fae0 > Date: Fri Jul 12 09:53:02 2019 +0200 > > [readline] Fix heap-buffer-overflow in update_line > > Which corresponds to this commit in upstream readline: > > commit 31547b4ea4a1a904e1b08e2bc4b4ebd5042aedaa > Date: Mon Aug 5 10:24:27 2019 -0400 > > commit readline-20190805 snapshot > > Introduced a use of an undefined variable, which can be seen using > valgrind: > > $ valgrind --tool=memcheck gdb > GNU gdb (GDB) 8.3.50.20190918-git > Copyright (C) 2019 Free Software Foundation, Inc. > License GPLv3+: GNU GPL version 3 or later > <http://gnu.org/licenses/gpl.html> > This is free software: you are free to change and redistribute it. > There is NO WARRANTY, to the extent permitted by law. > Type "show copying" and "show warranty" for details. > This GDB was configured as "x86_64-pc-linux-gnu". > Type "show configuration" for configuration details. > For bug reporting instructions, please see: > <http://www.gnu.org/software/gdb/bugs/>;. > Find the GDB manual and other documentation resources online at: > <http://www.gnu.org/software/gdb/documentation/>;. > > For help, type "help". > Type "apropos word" to search for commands related to "word". > ==24924== Conditional jump or move depends on uninitialised value(s) > ==24924== at 0x9986C3: rl_redisplay (display.c:710) > ==24924== by 0x9839CE: readline_internal_setup (readline.c:447) > ==24924== by 0x9A1C2B: _rl_callback_newline (callback.c:100) > ==24924== by 0x9A1C85: rl_callback_handler_install (callback.c:111) > ==24924== by 0x6195EB: gdb_rl_callback_handler_install(char const*) > (event-top.c:319) > ==24924== by 0x61975E: display_gdb_prompt(char const*) (event-top.c:409) > ==24924== by 0x4FBFE3: cli_interp_base::pre_command_loop() > (cli-interp.c:286) > ==24924== by 0x6E53DA: interp_pre_command_loop(interp*) (interps.c:321) > ==24924== by 0x731F30: captured_command_loop() (main.c:334) > ==24924== by 0x733568: captured_main(void*) (main.c:1182) > ==24924== by 0x7335CE: gdb_main(captured_main_args*) (main.c:1197) > ==24924== by 0x41325D: main (gdb.c:32) > ==24924== > (gdb) > > The problem can be traced back to init_line_structures. The very > first time this function is ever called its MINSIZE parameter is > always 0 and the global LINE_SIZE is 1024. Prior to the above > mentioned commits we spot that the line_state variables have not yet > been initialised, and allocate them some new buffer, then we enter > this loop: > > for (n = minsize; n < line_size; n++) > { > visible_line[n] = 0; > invisible_line[n] = 1; > } > > which would initialise everything from the incoming minimum up to the > potentially extended upper line size. > > The problem is that the above patches added a new condition that would > bump up the minsize like this: > > if (minsize <= _rl_screenwidth) /* XXX - for gdb */ > minsize = _rl_screenwidth + 1; > > So, the first time this function is called the incoming MINSIZE is 0, > the LINE_SIZE global is 1024, and if the _rl_screenwidth is 80, we see > that MINSIZE will be pushed up to 80. We still notice that the line > state is uninitialised and allocate some buffers, then we enter the > initialisation loop: > > for (n = minsize; n < line_size; n++) > { > visible_line[n] = 0; > invisible_line[n] = 1; > } > > And initialise from 80 to 1023 i the newly allocated buffers, leaving > 0 to 79 uninitialised. > > To confirm this is an issue, if we then look at rl_redisplay we see > that a call to init_line_structures is followed first by a call to > rl_on_new_line, which does initialise visible_line[0], but not > invisible_line[0]. Later in rl_redisplay we have this logic: > > if (visible_line[0] != invisible_line[0]) > rl_display_fixed = 0; > > The use of invisible_line[0] here will be undefined. > > Considering how this variable was originally initialised before the > above patches, this patch modifies the initialisation loop in > init_line_structures, to use the original value of MINSIZE. With this > change the valgrind warning goes away. > > readline/ChangeLog: > > * display.c (init_line_structures): Initialise line_state using > original minsize value. > --- > readline/ChangeLog.gdb | 5 +++++ > readline/display.c | 3 ++- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/readline/display.c b/readline/display.c > index b39f28291be..89193b572bc 100644 > --- a/readline/display.c > +++ b/readline/display.c > @@ -602,6 +602,7 @@ static void > init_line_structures (int minsize) > { > register int n; > + int original_minsize = minsize; > > if (minsize <= _rl_screenwidth) /* XXX - for gdb */ > minsize = _rl_screenwidth + 1; > @@ -622,7 +623,7 @@ init_line_structures (int minsize) > invisible_line = (char *)xrealloc (invisible_line, line_size); > } > > - for (n = minsize; n < line_size; n++) > + for (n = original_minsize; n < line_size; n++) > { > visible_line[n] = 0; > invisible_line[n] = 1; _______________________________________________ Bug-readline mailing list [email protected] https://lists.gnu.org/mailman/listinfo/bug-readline
