This is the mail archive of the gdb-cvs@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

gdb and binutils branch master updated. af48d08f97fa678571a42be35a8a77b61e36d7d7


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "gdb and binutils".

The branch, master has been updated
       via  af48d08f97fa678571a42be35a8a77b61e36d7d7 (commit)
       via  1a853c5224e2b8fedfac6d029365522b83080b40 (commit)
       via  ae9bb220caeb7d51fce6f54a182477247d8e3ac3 (commit)
      from  6bb3e67958b0ee59f1b69619761e6d5ad1f7544b (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

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

commit af48d08f97fa678571a42be35a8a77b61e36d7d7
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Nov 12 10:10:49 2014 +0000

    fix skipping permanent breakpoints
    
    The gdb.arch/i386-bp_permanent.exp test is currently failing an
    assertion recently added:
    
     (gdb) stepi
     ../../src/gdb/infrun.c:2237: internal-error: resume: Assertion `sig != GDB_SIGNAL_0' failed.
     A problem internal to GDB has been detected,
     further debugging may prove unreliable.
     Quit this debugging session? (y or n)
     FAIL: gdb.arch/i386-bp_permanent.exp: Single stepping past permanent breakpoint. (GDB internal error)
    
    The assertion expects that the only reason we currently need to step a
    breakpoint instruction is when we have a signal to deliver.  But when
    stepping a permanent breakpoint (with or without a signal) we also
    reach this code.
    
    The assertion is correct and the permanent breakpoints skipping code
    is wrong.
    
    Consider the case of the user doing "step/stepi" when stopped at a
    permanent breakpoint.  GDB's `resume' calls the
    gdbarch_skip_permanent_breakpoint hook and then happily continues
    stepping:
    
      /* Normally, by the time we reach `resume', the breakpoints are either
         removed or inserted, as appropriate.  The exception is if we're sitting
         at a permanent breakpoint; we need to step over it, but permanent
         breakpoints can't be removed.  So we have to test for it here.  */
      if (breakpoint_here_p (aspace, pc) == permanent_breakpoint_here)
        {
          gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
        }
    
    But since gdbarch_skip_permanent_breakpoint already advanced the PC
    manually, this ends up executing the instruction that is _after_ the
    breakpoint instruction.  The user-visible result is that a single-step
    steps two instructions.
    
    The gdb.arch/i386-bp_permanent.exp test is actually ensuring that
    that's indeed how things work.  It runs to an int3 instruction, does
    "stepi", and checks that "leave" was executed with that "stepi".  Like
    this:
    
     (gdb) b *0x0804848c
     Breakpoint 2 at 0x804848c
     (gdb) c
     Continuing.
    
     Breakpoint 2, 0x0804848c in standard ()
     (gdb) disassemble
     Dump of assembler code for function standard:
        0x08048488 <+0>:     push   %ebp
        0x08048489 <+1>:     mov    %esp,%ebp
        0x0804848b <+3>:     push   %edi
     => 0x0804848c <+4>:     int3
        0x0804848d <+5>:     leave
        0x0804848e <+6>:     ret
        0x0804848f <+7>:     nop
     (gdb) si
     0x0804848e in standard ()
     (gdb) disassemble
     Dump of assembler code for function standard:
        0x08048488 <+0>:     push   %ebp
        0x08048489 <+1>:     mov    %esp,%ebp
        0x0804848b <+3>:     push   %edi
        0x0804848c <+4>:     int3
        0x0804848d <+5>:     leave
     => 0x0804848e <+6>:     ret
        0x0804848f <+7>:     nop
     End of assembler dump.
     (gdb)
    
    One would instead expect that a stepi at 0x0804848c stops at
    0x0804848d, _before_ the "leave" is executed.  This commit changes GDB
    this way.  Care is taken to make stepping into a signal handler when
    the step starts at a permanent breakpoint instruction work correctly.
    
    The patch adjusts gdb.arch/i386-bp_permanent.exp in this direction,
    and also makes it work on x86_64 (currently it only works on i*86).
    
    The patch also adds a new gdb.base/bp-permanent.exp test that
    exercises many different code paths related to stepping permanent
    breakpoints, including the stepping with signals cases.  The test uses
    "hack/trick" to make it work on all (or most) platforms -- it doesn't
    really hard code a breakpoint instruction.
    
    Tested on x86_64 Fedora 20, native and gdbserver.
    
    gdb/
    2014-11-12  Pedro Alves  <palves@redhat.com>
    
    	* infrun.c (resume): Clear the thread's 'stepped_breakpoint' flag.
    	Rewrite stepping over a permanent breakpoint.
    	(thread_still_needs_step_over, proceed): Don't set
    	stepping_over_breakpoint for permanent breakpoints.
    	(handle_signal_stop): Don't clear stepped_breakpoint.  Also pull
    	single-step breakpoints out of the target on hardware step
    	targets.
    	(process_event_stop_test): If stepping a permanent breakpoint
    	doesn't hit the step-resume breakpoint, delete the step-resume
    	breakpoint.
    	(switch_back_to_stepped_thread): Also check if the stepped thread
    	has advanced already on hardware step targets.
    	(currently_stepping): Return true if the thread stepped a
    	breakpoint.
    
    gdb/testsuite/
    2014-11-12  Pedro Alves  <palves@redhat.com>
    
    	* gdb.arch/i386-bp_permanent.c: New file.
    	* gdb.arch/i386-bp_permanent.exp: Don't skip on x86_64.
    	(srcfile): Set to i386-bp_permanent.c.
    	(top level): Adjust to work in both 32-bit and 64-bit modes.  Test
    	that stepi does not execute the 'leave' instruction, instead of
    	testing it does execute.
    	* gdb.base/bp-permanent.c: New file.
    	* gdb.base/bp-permanent.exp: New file.

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

commit 1a853c5224e2b8fedfac6d029365522b83080b40
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Nov 12 10:10:49 2014 +0000

    make "permanent breakpoints" per location and disableable
    
    "permanent"-ness is currently a property of the breakpoint.  But, it
    should actually be an implementation detail of a _location_.  Consider
    this bit in infrun.c:
    
      /* Normally, by the time we reach `resume', the breakpoints are either
         removed or inserted, as appropriate.  The exception is if we're sitting
         at a permanent breakpoint; we need to step over it, but permanent
         breakpoints can't be removed.  So we have to test for it here.  */
      if (breakpoint_here_p (aspace, pc) == permanent_breakpoint_here)
        {
          if (gdbarch_skip_permanent_breakpoint_p (gdbarch))
    	gdbarch_skip_permanent_breakpoint (gdbarch, regcache);
          else
    	error (_("\
    The program is stopped at a permanent breakpoint, but GDB does not know\n\
    how to step past a permanent breakpoint on this architecture.  Try using\n\
    a command like `return' or `jump' to continue execution."));
        }
    
    This will wrongly skip a non-breakpoint instruction if we have a
    multiple location breakpoint where the whole breakpoint was set to
    "permanent" because one of the locations happened to be permanent,
    even if the one GDB is resuming from is not.
    
    Related, because the permanent breakpoints are only marked as such in
    init_breakpoint_sal, we currently miss marking momentary breakpoints
    as permanent.  A test added by a following patch trips on that.
    Making permanent-ness be per-location, and marking locations as such
    in add_location_to_breakpoint, the natural place to do this, fixes
    this issue...
    
    ... and then exposes a latent issue with mark_breakpoints_out.  It's
    clearing the inserted flag of permanent breakpoints.  This results in
    assertions failing like this:
    
     Breakpoint 1, main () at testsuite/gdb.base/callexit.c:32
     32        return 0;
     (gdb) call callexit()
     [Inferior 1 (process 15849) exited normally]
     gdb/breakpoint.c:12854: internal-error: allegedly permanent breakpoint is not actually inserted
     A problem internal to GDB has been detected,
     further debugging may prove unreliable.
    
    The call dummy breakpoint, which is a momentary breakpoint, is set on
    top of a manually inserted breakpoint instruction, and so is now
    rightfully marked as a permanent breakpoint.  See "Write a legitimate
    instruction at the point where the infcall breakpoint is going to be
    inserted." comment in infcall.c.
    
    Re. make_breakpoint_permanent.  That's only called by solib-pa64.c.
    Permanent breakpoints were actually originally invented for HP-UX [1].
    I believe that that call (the only one in the tree) is unnecessary
    nowadays, given that nowadays the core breakpoints code analyzes the
    instruction under the breakpoint to automatically detect whether it's
    setting a breakpoint on top of a breakpoint instruction in the
    program.  I know close to nothing about HP-PA/HP-UX, though.
    
    [1] https://sourceware.org/ml/gdb-patches/1999-q3/msg00245.html, and
        https://sourceware.org/ml/gdb-patches/1999-q3/msg00242.html
    
    In addition to the per-location issue, "permanent breakpoints" are
    currently always displayed as enabled=='n':
    
     (gdb) b main
     Breakpoint 3 at 0x40053c: file ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S, line 29.
     (gdb) info breakpoints
     Num     Type           Disp Enb Address            What
     3       breakpoint     keep n   0x000000000040053c ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S:29
    
    But OTOH they're always enabled; there's no way to disable them...
    
    In turn, this means that if one adds commands to such a breakpoint,
    they're _always_ run:
    
     (gdb) start
     Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.arch/i386-permbkpt
     ...
     Temporary breakpoint 1, main () at ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S:29
     29              int3
     (gdb) b main
     Breakpoint 2 at 0x40053c: file ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S, line 29.
     (gdb) info breakpoints
     Num     Type           Disp Enb Address            What
     2       breakpoint     keep n   0x000000000040053c ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S:29
     (gdb) commands
     Type commands for breakpoint(s) 2, one per line.
     End with a line saying just "end".
     >echo "hello!"
     >end
     (gdb) disable 2
     (gdb) start
     The program being debugged has been started already.
     Start it from the beginning? (y or n) y
     Temporary breakpoint 3 at 0x40053c: file ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S, line 29.
     Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.arch/i386-permbkpt
    
     Breakpoint 2, main () at ../../../src/gdb/testsuite/gdb.arch/i386-permbkpt.S:29
     29              int3
     "hello!"(gdb)
    
    IMO, one should be able to disable such a breakpoint, and GDB should
    then behave just like if the user hadn't created the breakpoint in the
    first place (that is, report a SIGTRAP).
    
    By making permanent-ness a property of the location, and eliminating
    the bp_permanent enum enable_state state ends up fixing that as well.
    
    No tests are added for these changes yet; they'll be added in a follow
    up patch, as skipping permanent breakpoints is currently broken and
    trips on an assertion in infrun.
    
    Tested on x86_64 Fedora 20, native and gdbserver.
    
    gdb/ChangeLog:
    2014-11-12  Pedro Alves  <palves@redhat.com>
    
    	Mark locations as permanent, not the whole breakpoint.
    	* breakpoint.c (remove_breakpoint_1, remove_breakpoint): Adjust.
    	(mark_breakpoints_out): Don't mark permanent breakpoints as
    	uninserted.
    	(breakpoint_init_inferior): Use mark_breakpoints_out.
    	(breakpoint_here_p): Adjust.
    	(bpstat_stop_status, describe_other_breakpoints): Remove handling
    	of permanent breakpoints.
    	(make_breakpoint_permanent): Mark each location as permanent,
    	instead of marking the breakpoint.
    	(add_location_to_breakpoint): If the location is permanent, mark
    	it as such, and as inserted.
    	(init_breakpoint_sal): Don't make the breakpoint permanent here.
    	(bp_location_compare, update_global_location_list): Adjust.
    	(update_breakpoint_locations): Don't make the breakpoint permanent
    	here.
    	(disable_breakpoint, enable_breakpoint_disp): Don't skip permanent
    	breakpoints.
    	* breakpoint.h (enum enable_state) <bp_permanent>: Delete field.
    	(struct bp_location) <permanent>: New field.
    	* guile/scm-breakpoint.c (bpscm_enable_state_to_string): Remove
    	reference to bp_permanent.

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

commit ae9bb220caeb7d51fce6f54a182477247d8e3ac3
Author: Pedro Alves <palves@redhat.com>
Date:   Wed Nov 12 10:10:48 2014 +0000

    add a default method for gdbarch_skip_permanent_breakpoint
    
    breakpoint.c uses gdbarch_breakpoint_from_pc to determine whether a
    breakpoint location points at a permanent breakpoint:
    
     static int
     bp_loc_is_permanent (struct bp_location *loc)
     {
     ...
       addr = loc->address;
       bpoint = gdbarch_breakpoint_from_pc (loc->gdbarch, &addr, &len);
     ...
      if (target_read_memory (loc->address, target_mem, len) == 0
          && memcmp (target_mem, bpoint, len) == 0)
        retval = 1;
     ...
    
    So I think we should default the gdbarch_skip_permanent_breakpoint
    hook to advancing the PC by the length of the breakpoint instruction,
    as determined by gdbarch_breakpoint_from_pc.  I believe that simple
    implementation does the right thing for most architectures.  If
    there's an oddball architecture where that doesn't work, then it
    should override the hook, just like it should be overriding the hook
    if there was no default anyway.
    
    The only two implementation of skip_permanent_breakpoint are
    i386_skip_permanent_breakpoint, for x86, and
    hppa_skip_permanent_breakpoint, for PA-RISC/HP-UX
    
    The x86 implementation is trivial, and can clearly be replaced by the
    new default.
    
    I don't know about the HP-UX one though, I know almost nothing about
    PA.  It may well be advancing the PC ends up being equivalent.
    Otherwise, it must be that "jump $pc_after_bp" doesn't work either...
    
    Tested on x86_64 Fedora 20 native and gdbserver.
    
    gdb/
    2014-11-12  Pedro Alves  <palves@redhat.com>
    
    	* arch-utils.c (default_skip_permanent_breakpoint): New function.
    	* arch-utils.h (default_skip_permanent_breakpoint): New
    	declaration.
    	* gdbarch.sh (skip_permanent_breakpoint): Now an 'f' function.
    	Install default_skip_permanent_breakpoint as default method.
    	* i386-tdep.c (i386_skip_permanent_breakpoint): Delete function.
    	(i386_gdbarch_init): Don't install it.
    	* infrun.c (resume): Assume there's always a
    	gdbarch_skip_permanent_breakpoint implementation.
    	* gdbarch.h, gdbarch.c: Regenerate.

-----------------------------------------------------------------------

Summary of changes:
 gdb/ChangeLog                                |   55 +++++
 gdb/arch-utils.c                             |   13 ++-
 gdb/arch-utils.h                             |    8 +
 gdb/breakpoint.c                             |   71 +++----
 gdb/breakpoint.h                             |   13 +-
 gdb/gdbarch.c                                |   13 +-
 gdb/gdbarch.h                                |    2 -
 gdb/gdbarch.sh                               |    2 +-
 gdb/guile/scm-breakpoint.c                   |    1 -
 gdb/i386-tdep.c                              |   15 --
 gdb/infrun.c                                 |  175 +++++++++++++----
 gdb/testsuite/ChangeLog                      |   11 +
 gdb/testsuite/gdb.arch/i386-bp_permanent.c   |   57 ++++++
 gdb/testsuite/gdb.arch/i386-bp_permanent.exp |   49 ++---
 gdb/testsuite/gdb.base/bp-permanent.c        |  128 ++++++++++++
 gdb/testsuite/gdb.base/bp-permanent.exp      |  275 ++++++++++++++++++++++++++
 16 files changed, 738 insertions(+), 150 deletions(-)
 create mode 100644 gdb/testsuite/gdb.arch/i386-bp_permanent.c
 create mode 100644 gdb/testsuite/gdb.base/bp-permanent.c
 create mode 100644 gdb/testsuite/gdb.base/bp-permanent.exp


hooks/post-receive
-- 
gdb and binutils


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]