[RFA,1/2] Fix regressions for multi breakpoints command line setting/clearing

Message ID 8736vfa3pp.fsf@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Aug. 15, 2018, 6:23 p.m. UTC
  Tom> I will test a new (much smaller) patch a bit later.

This is hilariously simpler and seems to fix the problem.
Let me know what you think.

Tom

commit 06b655186f44cf3cf9f0c9419427a65f6b32cb5c
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Aug 15 08:25:49 2018 -0600

    Do not allow command repetition from read_next_line
    
    Pedro pointed out that the commands being read by "commands" should
    not be repeatable.  This patch fixes that problem, and the
    use-after-free bug, by simply modifying read_next_line to disallow
    repetition.
    
    Tested by the buildbot.
    
    gdb/ChangeLog
    2018-08-15  Tom Tromey  <tom@tromey.com>
    
            * cli/cli-script.c (read_next_line): Pass 0 as repeat argument to
            command_line_input.
  

Comments

Pedro Alves Aug. 16, 2018, 3:34 p.m. UTC | #1
On 08/15/2018 07:23 PM, Tom Tromey wrote:
> Tom> I will test a new (much smaller) patch a bit later.
> 
> This is hilariously simpler and seems to fix the problem.
> Let me know what you think.

I think this is the right fix.  Please push.

>     Pedro pointed out that the commands being read by "commands" should
>     not be repeatable.  This patch fixes that problem, and the
>     use-after-free bug, by simply modifying read_next_line to disallow
>     repetition.

I think it'd be good to expand a bit more about the use-after-free
bug here (someone reading git logs later on won't have the context).
You already had something in the previous version of the patch,
could just copy it in.

AFAICT, the "repeat" parameter of command_line_input is always 0 now,
and so could be eliminated (in a separate patch).

Thanks,
Pedro Alves
  
Tom Tromey Aug. 17, 2018, 11:11 p.m. UTC | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I think it'd be good to expand a bit more about the use-after-free
Pedro> bug here (someone reading git logs later on won't have the context).
Pedro> You already had something in the previous version of the patch,
Pedro> could just copy it in.

Pedro> AFAICT, the "repeat" parameter of command_line_input is always 0 now,
Pedro> and so could be eliminated (in a separate patch).

I'll send both patches momentarily.
I'm going to put them on the 8.2 branch as well.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9fac8ccf5f4..1e5f5a8e30d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-08-15  Tom Tromey  <tom@tromey.com>
+
+	* cli/cli-script.c (read_next_line): Pass 0 as repeat argument to
+	command_line_input.
+
 2018-08-15  Tom Tromey  <tom@tromey.com>
 
 	* aarch64-linux-tdep.c (aarch64_linux_core_read_vq): Use pulongest.
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 6f31a400197..d03b3bcf60b 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -903,7 +903,7 @@  read_next_line (void)
   else
     prompt_ptr = NULL;
 
-  return command_line_input (prompt_ptr, from_tty, "commands");
+  return command_line_input (prompt_ptr, 0, "commands");
 }
 
 /* Return true if CMD's name is NAME.  */