Fix gdb.threads/multiple-step-overs.exp fails on arm
authorYao Qi <yao.qi@linaro.org>
Tue, 17 Nov 2015 15:40:29 +0000 (15:40 +0000)
committerYao Qi <yao.qi@linaro.org>
Tue, 17 Nov 2015 15:40:29 +0000 (15:40 +0000)
commit1946c4ccca04e374acc040fc30c8b44d2c9ca0a8
treef7325d683a06bcd287a94920d295f10351c687fd
parentb6b806729dd5c4c46981d59f351460b4d7989548
Fix gdb.threads/multiple-step-overs.exp fails on arm

Hi,
Some tests in gdb.threads/multiple-step-overs.exp fail on arm target
when the displaced stepping on, but they pass when displaced stepping
is off.

 FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: step: step
 FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: next: next
 FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: continue: continue
 FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: signal thr1: continue to sigusr1_handler

when displaced stepping is on,

Sending packet: $vCont;c#a8...infrun: infrun_async(1)^M <--- [1]
infrun: prepare_to_wait^M
infrun: target_wait (-1.0.0, status) =^M
infrun:   -1.0.0 [Thread 0],^M
infrun:   status->kind = ignore^M
infrun: TARGET_WAITKIND_IGNORE^M
infrun: prepare_to_wait^M
Packet received: T05swbreak:;0b:f8faffbe;0d:409ee7b6;0f:d0880000;thread:p635.636;core:0;^M
infrun: target_wait (-1.0.0, status) =^M
infrun:   1589.1590.0 [Thread 1590],^M
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP^M
infrun: TARGET_WAITKIND_STOPPED^M
infrun: stop_pc = 0x88d0^M
infrun: context switch^M
infrun: Switching context from Thread 1591 to Thread 1590^

GDB resumes the whole process (all threads) rather than the specific
thread for which GDB wants to step over the breakpoint (as shown in [1]).
That is wrong because we resume a single thread and leave others stopped
when doing a normal step over where we temporarily remove the breakpoint,
single-step, reinsert the breakpoint, is that if we let other threads run
in the period while the breakpoint is removed, then these other threads
could miss the breakpoint.  Since with displaced stepping, we don't ever
remove the breakpoint, it should be fine to let other threads run.  However,
there's another reason that we should not let other threads run: that is
the case where some of those threads are also stopped for a breakpoint that
itself needs to be stepped over.  If we just let those threads run, then
they immediately re-trap their breakpoint again.

when displaced stepping is off, GDB behaves correctly, only resumes
the specific thread (as shown in [2]).

Sending packet: $vCont;c:p611.613#b2...infrun: infrun_async(1)^M <-- [2]
infrun: prepare_to_wait^M
infrun: target_wait (-1.0.0, status) =^M
infrun:   -1.0.0 [Thread 0],^M
infrun:   status->kind = ignore^M
infrun: TARGET_WAITKIND_IGNORE^M
infrun: prepare_to_wait^M
Packet received: T05swbreak:;0b:f8faffbe;0d:409e67b6;0f:48880000;thread:p611.613;core:1;^M
infrun: target_wait (-1.0.0, status) =^M
infrun:   1553.1555.0 [Thread 1555],^M
infrun:   status->kind = stopped, signal = GDB_SIGNAL_TRAP^M
infrun: TARGET_WAITKIND_STOPPED^M
infrun: clear_step_over_info^M
infrun: stop_pc = 0x8848

The current logic in GDB on deciding the set of threads to resume is:

  /* Decide the set of threads to ask the target to resume.  */
  if ((step || thread_has_single_step_breakpoints_set (tp))
      && tp->control.trap_expected)
    {
      /* We're allowing a thread to run past a breakpoint it has
 hit, by single-stepping the thread with the breakpoint
 removed.  In which case, we need to single-step only this
 thread, and keep others stopped, as they can miss this
 breakpoint if allowed to run.  */
      resume_ptid = inferior_ptid;
    }
  else
    resume_ptid = internal_resume_ptid (user_step);

it doesn't handle the case correctly that GDB continue (instead of
single step) the thread for displaced stepping.

I also update the comment below to reflect the code.  I remove the
"with the breakpoint removed" comment, because GDB doesn't remove
breakpoints in displaced stepping, so we don't have to worry that
other threads may miss the breakpoint.

Patch is regression tested on both x86_64-linux and arm-linux.

gdb:

2015-11-17  Yao Qi  <yao.qi@linaro.org>

* infrun.c (resume): Check control.trap_expected only
when deciding the set of threads to resume.
gdb/ChangeLog
gdb/infrun.c