Bug 19187 - running with process record over a fork causes GDB internal error
Summary: running with process record over a fork causes GDB internal error
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: record (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 7.12
Assignee: Pedro Alves
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-29 18:23 UTC by Marcin Kościelnicki
Modified: 2016-08-10 22:11 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcin Kościelnicki 2015-10-29 18:23:26 UTC
$ cat f.c
#include <unistd.h>
#include <stdio.h>

int main() {
        int a = fork();
        printf("%d\n", a);
        return 0;
}
$ gcc f.c -g
$ gdb ./a.out
GNU gdb (GDB) 7.10
Copyright (C) 2015 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-unknown-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"...
Reading symbols from ./a.out...done.
(gdb) break main
Breakpoint 1 at 0x40054e: file f.c, line 5.
(gdb) r
Starting program: /home/mwk/a.out 

Breakpoint 1, main () at f.c:5
5               int a = fork();
(gdb) record
(gdb) step
0
record-full.c:1716: internal-error: record_full_remove_breakpoint: removing unknown breakpoint
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n) 

The bug occurs on 7.10, as well as current HEAD.  I have submitted a testcase covering this bug at https://sourceware.org/ml/gdb-patches/2015-10/msg00444.html (linux-waitpid-reverse.exp).  The bug appears to be architecture-independent - I've tested on i386, x86_64, x32, s390, s390x.
Comment 1 Yao Qi 2015-11-13 09:56:52 UTC
The fail appears on arm and aarch64 too.
Comment 2 Pedro Alves 2016-02-14 18:39:08 UTC
Fix sent here:
 https://sourceware.org/ml/gdb-patches/2016-02/msg00448.html
Comment 3 Yao Qi 2016-07-25 13:55:05 UTC
(In reply to Pedro Alves from comment #2)
> Fix sent here:
>  https://sourceware.org/ml/gdb-patches/2016-02/msg00448.html


Hi Pedro,
Any reason we still hold this patch series?  I think they should go in before the 7.12 release.
Comment 4 Pedro Alves 2016-07-25 14:34:58 UTC
Looks like I had completely forgotten about this.  

But let me take another look; I don't recall whether I got cold feet before pushing, or whether I just simply forgot.

Setting target milestone to make sure I don't forget again.
Comment 5 cvs-commit@gcc.gnu.org 2016-08-10 22:05:52 UTC
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=834c0d033bdade640aab149d0d4bd7b41dcb16af

commit 834c0d033bdade640aab149d0d4bd7b41dcb16af
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:03:28 2016 +0100

    Simplify remove_breakpoint interface
    
    All callers pass mark_uninserted, so there's no need for the 'is'
    parameter.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* breakpoint.c (remove_breakpoint): Remove 'is' parameter and
    	always pass mark_uninserted to remove_breakpoint_1.
    	(insert_breakpoint_locations, remove_breakpoints)
    	(remove_breakpoints_pid, update_global_location_list): Update
    	callers.
Comment 6 cvs-commit@gcc.gnu.org 2016-08-10 22:05:58 UTC
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b2b6a7dab91de9a616e1d76c869d127c5752b9e6

commit b2b6a7dab91de9a616e1d76c869d127c5752b9e6
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:03:29 2016 +0100

    Introduce 'enum remove_bp_reason'
    
    Makes the code more obvious.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* breakpoint.c (insertion_state_t): Delete.
    	(enum remove_bp_reason): New.
    	(detach_breakpoints, remove_breakpoint_1, remove_breakpoint):
    	Adjust to use enum remove_bp_reason instead of insertion_state_t.
Comment 7 cvs-commit@gcc.gnu.org 2016-08-10 22:06:03 UTC
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=73971819031d74eb846805a9fbfad04ba1dff500

commit 73971819031d74eb846805a9fbfad04ba1dff500
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:03:29 2016 +0100

    Plumb enum remove_bp_reason all the way to target_remove_breakpoint
    
    So the target knows whether we're detaching breakpoints.
    Nothing uses the parameter in this patch yet.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* break-catch-sig.c (signal_catchpoint_remove_location): Adjust
    	interface.
    	* break-catch-syscall.c (remove_catch_syscall):
    	* breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h.
    	(remove_breakpoint_1): Pass 'reason' down.
    	(remove_catch_fork, remove_catch_vfork, remove_catch_solib)
    	(remove_catch_exec, remove_watchpoint, remove_masked_watchpoint)
    	(base_breakpoint_remove_location, bkpt_remove_location)
    	(bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust
    	interface.
    	* breakpoint.h (enum remove_bp_reason): Moved here from
    	breakpoint.c.
    	(struct breakpoint_ops) <remove_location>: Add 'reason' parameter.
    	* corelow.c (core_remove_breakpoint): New function.
    	(init_core_ops): Install it as to_remove_breakpoint method.
    	* exec.c (exec_remove_breakpoint): New function.
    	(init_exec_ops): Install it as to_remove_breakpoint method.
    	* mem-break.c (memory_remove_breakpoint): Adjust interface.
    	* record-btrace.c (record_btrace_remove_breakpoint): Adjust
    	interface.
    	* record-full.c (record_full_remove_breakpoint)
    	(record_full_core_remove_breakpoint): Adjust interface.
    	* remote.c (remote_remove_breakpoint): Adjust interface.
    	* target-debug.h (target_debug_print_enum_remove_bp_reason): New
    	macro.
    	* target-delegates.c: Regenerate.
    	* target.c (target_remove_breakpoint): Add 'reason' parameter.
    	* target.h (struct target_ops) <to_remove_breakpoint>: Add
    	'reason' parameter.
    	(target_remove_breakpoint, memory_remove_breakpoint): Add 'reason'
    	parameter.
Comment 8 cvs-commit@gcc.gnu.org 2016-08-10 22:06:09 UTC
The master branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=01d3dedf60912cee478c242d575f4683adada1d2

commit 01d3dedf60912cee478c242d575f4683adada1d2
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:03:29 2016 +0100

    Fix PR gdb/19187 (process record over a fork causes internal error)
    
    Right after a fork is detected, we detach breakpoints from the child
    (detach_breakpoints), which calls into target_remove_breakpoint with
    inferior_ptid pointing at the child process, but leaves the breakpoint
    marked inserted (in the parent).
    
    The problem is that record-full.c always deletes all knowledge of the
    breakpoint.  Then when we later really delete the breakpoint from the
    parent, we fail the assertion, since the breakpoint is unexpectedly
    not found in the record-full.c breakpoint table.
    
    The fix is simply to not forget about the breakpoint if we're
    detaching it from a fork child.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* record-full.c (record_full_remove_breakpoint): Don't remove the
    	breakpoint from the record_full_breakpoints VEC if we're detaching
    	the breakpoint from a fork child.
    
    gdb/testsuite/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* gdb.reverse/waitpid-reverse.exp: Add comment and remove
    	setup_kfails.
Comment 9 cvs-commit@gcc.gnu.org 2016-08-10 22:07:39 UTC
The gdb-7.12-branch branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7c50accb20e77971c775fdb7f5f1bfa605397e80

commit 7c50accb20e77971c775fdb7f5f1bfa605397e80
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:06:01 2016 +0100

    Simplify remove_breakpoint interface
    
    All callers pass mark_uninserted, so there's no need for the 'is'
    parameter.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* breakpoint.c (remove_breakpoint): Remove 'is' parameter and
    	always pass mark_uninserted to remove_breakpoint_1.
    	(insert_breakpoint_locations, remove_breakpoints)
    	(remove_breakpoints_pid, update_global_location_list): Update
    	callers.
Comment 10 cvs-commit@gcc.gnu.org 2016-08-10 22:07:44 UTC
The gdb-7.12-branch branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bbbbef233724142bd4ff5c288863a62c98b01ff1

commit bbbbef233724142bd4ff5c288863a62c98b01ff1
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:06:01 2016 +0100

    Introduce 'enum remove_bp_reason'
    
    Makes the code more obvious.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* breakpoint.c (insertion_state_t): Delete.
    	(enum remove_bp_reason): New.
    	(detach_breakpoints, remove_breakpoint_1, remove_breakpoint):
    	Adjust to use enum remove_bp_reason instead of insertion_state_t.
Comment 11 cvs-commit@gcc.gnu.org 2016-08-10 22:07:49 UTC
The gdb-7.12-branch branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=aac9c6220f36cea74d13cd02cce6c64d49f41f34

commit aac9c6220f36cea74d13cd02cce6c64d49f41f34
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:06:01 2016 +0100

    Plumb enum remove_bp_reason all the way to target_remove_breakpoint
    
    So the target knows whether we're detaching breakpoints.
    Nothing uses the parameter in this patch yet.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* break-catch-sig.c (signal_catchpoint_remove_location): Adjust
    	interface.
    	* break-catch-syscall.c (remove_catch_syscall):
    	* breakpoint.c (enum remove_bp_reason): Moved to breakpoint.h.
    	(remove_breakpoint_1): Pass 'reason' down.
    	(remove_catch_fork, remove_catch_vfork, remove_catch_solib)
    	(remove_catch_exec, remove_watchpoint, remove_masked_watchpoint)
    	(base_breakpoint_remove_location, bkpt_remove_location)
    	(bkpt_probe_remove_location, bkpt_probe_remove_location): Adjust
    	interface.
    	* breakpoint.h (enum remove_bp_reason): Moved here from
    	breakpoint.c.
    	(struct breakpoint_ops) <remove_location>: Add 'reason' parameter.
    	* corelow.c (core_remove_breakpoint): New function.
    	(init_core_ops): Install it as to_remove_breakpoint method.
    	* exec.c (exec_remove_breakpoint): New function.
    	(init_exec_ops): Install it as to_remove_breakpoint method.
    	* mem-break.c (memory_remove_breakpoint): Adjust interface.
    	* record-btrace.c (record_btrace_remove_breakpoint): Adjust
    	interface.
    	* record-full.c (record_full_remove_breakpoint)
    	(record_full_core_remove_breakpoint): Adjust interface.
    	* remote.c (remote_remove_breakpoint): Adjust interface.
    	* target-debug.h (target_debug_print_enum_remove_bp_reason): New
    	macro.
    	* target-delegates.c: Regenerate.
    	* target.c (target_remove_breakpoint): Add 'reason' parameter.
    	* target.h (struct target_ops) <to_remove_breakpoint>: Add
    	'reason' parameter.
    	(target_remove_breakpoint, memory_remove_breakpoint): Add 'reason'
    	parameter.
Comment 12 cvs-commit@gcc.gnu.org 2016-08-10 22:07:56 UTC
The gdb-7.12-branch branch has been updated by Pedro Alves <palves@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=4dfacf706adacbefbc38d7ba59212566f77bd051

commit 4dfacf706adacbefbc38d7ba59212566f77bd051
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Aug 10 23:06:02 2016 +0100

    Fix PR gdb/19187 (process record over a fork causes internal error)
    
    Right after a fork is detected, we detach breakpoints from the child
    (detach_breakpoints), which calls into target_remove_breakpoint with
    inferior_ptid pointing at the child process, but leaves the breakpoint
    marked inserted (in the parent).
    
    The problem is that record-full.c always deletes all knowledge of the
    breakpoint.  Then when we later really delete the breakpoint from the
    parent, we fail the assertion, since the breakpoint is unexpectedly
    not found in the record-full.c breakpoint table.
    
    The fix is simply to not forget about the breakpoint if we're
    detaching it from a fork child.
    
    gdb/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* record-full.c (record_full_remove_breakpoint): Don't remove the
    	breakpoint from the record_full_breakpoints VEC if we're detaching
    	the breakpoint from a fork child.
    
    gdb/testsuite/ChangeLog:
    2016-08-10  Pedro Alves  <palves@redhat.com>
    
    	PR gdb/19187
    	* gdb.reverse/waitpid-reverse.exp: Add comment and remove
    	setup_kfails.
Comment 13 Pedro Alves 2016-08-10 22:11:26 UTC
Fixed.