Re: [PATCH 1 of 2] readline rl_prompt corruption fix

Re: [PATCH 1 of 2] readline rl_prompt corruption fix

[Elena Zannoni]

In general patches to readline should be sent to the official
readline maintainer, Chet Ramey. ftp://ftp.cwru.edu/pub/bash/ We don't
want to diverge from the official readline release unless absolutely
necessary. 
In any event, I am sligltly confused about the 2 patches. 
What was the original problem? The positioning of the cursor in the TUI?
Can you explain a little more?

Thanks
Elena


Jimmy Guo writes:
 > This is part 1 of 2 patches to fix readline rl_prompt string corruption
 > problem.  This patch is for the readline library that readline will
 > store a copy of the passed in prompt string.  The next patch is for
 > gdb/event-top.c to plug a mem leak due to this change.
 > 
 > - Jimmy
 > 
 > Fri Jul 28 09:58:55	Jimmy Guo	<guo@cup.hp.com>
 > 
 > 	* readline.c: Initalize globals rl_initialized rl_prompt.
 > 	(readline): Use a copy of prompt instead of itself when
 > 	assigning to rl_prompt in readline ().
 > 
 > 	* callback.c (rl_callback_handler_install): Use a copy of
 > 	prompt instead of itself when assigning to rl_prompt.
 > 	
 > 	* display.c: Initialize local_prompt and local_prompt_prefix.
 > 
 > Index: readline.c
 > /usr/local/bin/diff -c -w -L readline.c readline.c@@/main/cygnus/6 readline.c
 > *** readline.c
 > --- readline.c	Fri Jul 28 09:57:29 2000
 > ***************
 > *** 119,125 ****
 >   int rl_arg_sign = 1;
 >   
 >   /* Non-zero means we have been called at least once before. */
 > ! static int rl_initialized;
 >   
 >   /* If non-zero, this program is running in an EMACS buffer. */
 >   static int running_in_emacs;
 > --- 119,125 ----
 >   int rl_arg_sign = 1;
 >   
 >   /* Non-zero means we have been called at least once before. */
 > ! static int rl_initialized = 0;
 >   
 >   /* If non-zero, this program is running in an EMACS buffer. */
 >   static int running_in_emacs;
 > ***************
 > *** 153,159 ****
 >   int readline_echoing_p = 1;
 >   
 >   /* Current prompt. */
 > ! char *rl_prompt;
 >   int rl_visible_prompt_length = 0;
 >   
 >   /* Set to non-zero by calling application if it has already printed rl_prompt
 > --- 153,159 ----
 >   int readline_echoing_p = 1;
 >   
 >   /* Current prompt. */
 > ! char *rl_prompt = NULL;
 >   int rl_visible_prompt_length = 0;
 >   
 >   /* Set to non-zero by calling application if it has already printed rl_prompt
 > ***************
 > *** 251,257 ****
 >   {
 >     char *value;
 >   
 > !   rl_prompt = prompt;
 >   
 >     /* If we are at EOF return a NULL string. */
 >     if (rl_pending_input == EOF)
 > --- 251,259 ----
 >   {
 >     char *value;
 >   
 > !   if (rl_prompt)
 > !     free (rl_prompt);
 > !   rl_prompt = prompt ? strdup (prompt) : 0;
 >   
 >     /* If we are at EOF return a NULL string. */
 >     if (rl_pending_input == EOF)
 > ***************
 > *** 260,266 ****
 >         return ((char *)NULL);
 >       }
 >   
 > !   rl_visible_prompt_length = rl_expand_prompt (rl_prompt);
 >   
 >     rl_initialize ();
 >     (*rl_prep_term_function) (_rl_meta_flag);
 > --- 262,269 ----
 >         return ((char *)NULL);
 >       }
 >   
 > !   rl_visible_prompt_length = (rl_prompt && *rl_prompt) ?
 > !                              rl_expand_prompt (rl_prompt) : 0;
 >   
 >     rl_initialize ();
 >     (*rl_prep_term_function) (_rl_meta_flag);
 > Index: callback.c
 > /usr/local/bin/diff -c -w -L callback.c callback.c@@/main/cygnus/2 callback.c
 > *** callback.c
 > --- callback.c	Fri Jul 28 09:55:11 2000
 > ***************
 > *** 81,88 ****
 >        char *prompt;
 >        VFunction *linefunc;
 >   {
 > !   rl_prompt = prompt;
 > !   rl_visible_prompt_length = rl_prompt ? rl_expand_prompt (rl_prompt) : 0;
 >     rl_linefunc = linefunc;
 >     _rl_callback_newline ();
 >   }
 > --- 81,91 ----
 >        char *prompt;
 >        VFunction *linefunc;
 >   {
 > !   if (rl_prompt)
 > !     free (rl_prompt);
 > !   rl_prompt = prompt ? strdup (prompt) : 0;
 > !   rl_visible_prompt_length = (rl_prompt && *rl_prompt) ?
 > !                              rl_expand_prompt (rl_prompt) : 0;
 >     rl_linefunc = linefunc;
 >     _rl_callback_newline ();
 >   }
 > Index: display.c
 > /usr/local/bin/diff -c -w -L display.c display.c@@/main/cygnus/7 display.c
 > *** display.c
 > --- display.c	Fri Jul 28 09:56:05 2000
 > ***************
 > *** 146,152 ****
 >   /* Default and initial buffer size.  Can grow. */
 >   static int line_size = 1024;
 >   
 > ! static char *local_prompt, *local_prompt_prefix;
 >   static int visible_length, prefix_length;
 >   
 >   /* The number of invisible characters in the line currently being
 > --- 146,152 ----
 >   /* Default and initial buffer size.  Can grow. */
 >   static int line_size = 1024;
 >   
 > ! static char *local_prompt = NULL, *local_prompt_prefix = NULL;
 >   static int visible_length, prefix_length;
 >   
 >   /* The number of invisible characters in the line currently being
 > 
From guo@cup.hp.com Tue Aug 01 10:18:00 2000
From: Jimmy Guo <guo@cup.hp.com>
To: gdb-patches@sourceware.cygnus.com
Subject: [PATCH] dejagnu lib/target.exp HP warnings
Date: Tue, 01 Aug 2000 10:18:00 -0000
Message-id: <Pine.LNX.4.10.10008011016581.1500-100000@hpcll168.cup.hp.com>
X-SW-Source: 2000-08/msg00016.html
Content-length: 2191

Couple of changes specific to HP compilation message filtering.

- Jimmy

Tue Aug  1 10:13:22	Jimmy Guo	<guo@cup.hp.com>

	* lib/target.exp (prune_warnings): Remove duplicate PA2.0 pattern;
	revise HP Fortran (f77 & f90) pattern.

Index: lib/target.exp
/usr/local/bin/diff -c -L lib/target.exp lib/target.exp@@/main/cygnus/12 lib/target.exp
*** lib/target.exp
--- lib/target.exp	Tue Aug  1 10:12:06 2000
***************
*** 146,153 ****
         # Ignore the compiler's warnings about PA incompatibility.
         regsub -all "(^|\n)\[^\n\]*PA 2.0 object file \[^\n\]* was detected. The linked output may not run on a PA 1.x system." $text "" text 
   
-        regsub -all "(^|\n)\[^\n\]*PA 2.0 object file \[^\n\]* was detected. The linked output may not run on a PA 1.x system." $text "" text 
- 
         # And the linker's +vcompatwarnings verbage.
         regsub -all "(^|\n)\[^\n\]*Linker features were used that may not be supported\[^\n\]*.\[^\n\]*." $text "" text
  
--- 146,151 ----
***************
*** 188,198 ****
         #
         regsub -all "aCC .assigner.: Warning .*Could not satisfy instantiation request for \[^\n\]* contained in\[^\n\]*\n\t/lib/pa20_64/lib\[a-zA-Z0-9\]*.sl" $text "" text
         
!       # Remove the lines that are output by the HP F77 compiler to
        # indicate the functions that are being compiled.
        upvar compiler_type compiler_type
        if { [info exists compiler_type] && $compiler_type == "f77" } {
! 	  regsub -all "\[ \ta-zA-Z_0-9\./\]*:\[\r\n\]+" $text "" text
        }
  
        # Ignore the warnings about unknown options
--- 186,196 ----
         #
         regsub -all "aCC .assigner.: Warning .*Could not satisfy instantiation request for \[^\n\]* contained in\[^\n\]*\n\t/lib/pa20_64/lib\[a-zA-Z0-9\]*.sl" $text "" text
         
!       # Remove the lines that are output by the HP F77 / F90 compiler to
        # indicate the functions that are being compiled.
        upvar compiler_type compiler_type
        if { [info exists compiler_type] && $compiler_type == "f77" } {
! 	  regsub -all "\[ \ta-zA-Z_0-9\./\-\]*(:|)\[\r\n\]+.*" $text "" text
        }
  
        # Ignore the warnings about unknown options

Re: [PATCH 1 of 2] readline rl_prompt corruption fix

[Jimmy Guo]

>In general patches to readline should be sent to the official
>readline maintainer, Chet Ramey. ftp://ftp.cwru.edu/pub/bash/ We don't
>want to diverge from the official readline release unless absolutely
>necessary. 

Thanks for the info.  I will work with Chet Ramey once we agree if this
is something that we need to patch readline up.

>In any event, I am sligltly confused about the 2 patches. 
>What was the original problem? The positioning of the cursor in the TUI?
>Can you explain a little more?

GDB (event-top.c, couple of places) calls rl_callback_install to set the
prompt and the callback function.  In one place the prompt string is
allocated via malloc, in another it's allocated via alloca.  Today,
readline use the passed in prompt parameter directly and assign the
pointer value to rl_prompt, a global -- no matter what this
implementation is questionable, since it should save a copy itself.

In the TUI mode, TUI code does strlen (rl_prompt) when handling arrow
key input events.  Unfortunately rl_prompt can be corrupted if it was
originally from one of the event-top.c callback install which uses
alloca(), and the behavior would be when a user presses arrow keys in
TUI mode to browse source listings, cursor placement will become random.

There are several ways to fix it, some with side effects like memory
leaks (which exist in today's gdb BTW wrt the malloc'd prompt), others
with changes to probably the tui code to use the prompt length
calculated by readline, etc.  However, I think the way readline handles
passed in prompt is inviting trouble, and fixing that is probably the
most appropriate approach.

- Jimmy