diff mbox

: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha

Message ID CAFULd4ZGUgGneAb9m_Z=9HeKjjcXpv4mtp=uWbf98-XH43ptxA@mail.gmail.com
State New, archived
Headers

Commit Message

Uros Bizjak Dec. 15, 2017, 12:11 p.m. UTC 
  Hello!

Attached patch fixes PR19061, where gdb hangs/spins-on-cpu when
debugging any program on Alpha. The patch is effectively a forward
port of Richard's patch from the Comment #5 of the PR [1].


2017-12-15  Uros Bizjak  <ubizjak@gmail.com>
        Richard Henderson  <rth@redhat.com>

    PR gdb/19061
    * alpha-tdep.c (alpha_software_single_step): Call
    alpha_deal_with_atomic_sequence here.
    (set_gdbarch_software_single_step): Set to alpha_software_single_step.
    * nat/linux-ptrace.h [__alpha__]: Define GDB_ARCH_IS_TRAP_BRKPT
    and GDB_ARCH_IS_TRAP_HWBKPT.

Patch was tested on alphaev68-linux-gnu, also tested with gcc's
testsuite, where it fixed all hangs in guality.exp and
simulate-thread.exp testcases.

Please note that I have no commit access, so if approved, please
commit the patch to the source repository for me. I also have
functionally equivalent patch for gdb-8 branch which I plan to submit
later.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=19061#c5

Uros.

Comments

Pedro Alves Dec. 15, 2017, 6:21 p.m. UTC | #1 
On 12/15/2017 12:11 PM, Uros Bizjak wrote:
> 
> Please note that I have no commit access, so if approved, please
> commit the patch to the source repository for me. 

Merged.

Thanks,
Pedro Alves
Simon Marchi May 30, 2018, 2:53 p.m. UTC | #2 
On 2017-12-15 07:11 AM, Uros Bizjak wrote:
> Hello!
> 
> Attached patch fixes PR19061, where gdb hangs/spins-on-cpu when
> debugging any program on Alpha. The patch is effectively a forward
> port of Richard's patch from the Comment #5 of the PR [1].
> 
> 
> 2017-12-15  Uros Bizjak  <ubizjak@gmail.com>
>         Richard Henderson  <rth@redhat.com>
> 
>     PR gdb/19061
>     * alpha-tdep.c (alpha_software_single_step): Call
>     alpha_deal_with_atomic_sequence here.
>     (set_gdbarch_software_single_step): Set to alpha_software_single_step.
>     * nat/linux-ptrace.h [__alpha__]: Define GDB_ARCH_IS_TRAP_BRKPT
>     and GDB_ARCH_IS_TRAP_HWBKPT.
> 
> Patch was tested on alphaev68-linux-gnu, also tested with gcc's
> testsuite, where it fixed all hangs in guality.exp and
> simulate-thread.exp testcases.
> 
> Please note that I have no commit access, so if approved, please
> commit the patch to the source repository for me. I also have
> functionally equivalent patch for gdb-8 branch which I plan to submit
> later.
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=19061#c5
> 
> Uros.
> 

Hi Uros and Richard,

I would need your input.  Using this cross-compiler:

alphaev67-unknown-linux-gnu-gcc (crosstool-NG crosstool-ng-1.22.0-677-ga3dd55b9) 6.3.0
Copyright (C) 2016 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I get this error:

  CXX    linux-nat.o
/home/simark/src/binutils-gdb/gdb/linux-nat.c: In function 'void save_stop_reason(lwp_info*)':
/home/simark/src/binutils-gdb/gdb/linux-nat.c:2718:9: error: duplicated 'if' condition [-Werror=duplicated-cond]
    else if (GDB_ARCH_IS_TRAP_HWBKPT (siginfo.si_code))
         ^~
In file included from /home/simark/src/binutils-gdb/gdb/linux-nat.c:31:0:
/home/simark/src/binutils-gdb/gdb/nat/linux-ptrace.h:173:41: note: previously used here
 # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
                                    ~~~~~^~~~~~~~~~~~~~
/home/simark/src/binutils-gdb/gdb/linux-nat.c:2709:13: note: in expansion of macro 'GDB_ARCH_IS_TRAP_BRKPT'
    else if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
             ^~~~~~~~~~~~~~~~~~~~~~

Does Alpha even have hardware breakpoints?  If not, I would suggest
defining GDB_ARCH_IS_TRAP_HWBKPT to 0 for __alpha__.  It would get
rid of the error, and be more exact (no si_code can mean "hardware
breakpoint" on alpha).

Simon
Uros Bizjak May 30, 2018, 6:07 p.m. UTC | #3 
On Wed, May 30, 2018 at 4:53 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
> On 2017-12-15 07:11 AM, Uros Bizjak wrote:
>> Hello!
>>
>> Attached patch fixes PR19061, where gdb hangs/spins-on-cpu when
>> debugging any program on Alpha. The patch is effectively a forward
>> port of Richard's patch from the Comment #5 of the PR [1].
>>
>>
>> 2017-12-15  Uros Bizjak  <ubizjak@gmail.com>
>>         Richard Henderson  <rth@redhat.com>
>>
>>     PR gdb/19061
>>     * alpha-tdep.c (alpha_software_single_step): Call
>>     alpha_deal_with_atomic_sequence here.
>>     (set_gdbarch_software_single_step): Set to alpha_software_single_step.
>>     * nat/linux-ptrace.h [__alpha__]: Define GDB_ARCH_IS_TRAP_BRKPT
>>     and GDB_ARCH_IS_TRAP_HWBKPT.
>>
>> Patch was tested on alphaev68-linux-gnu, also tested with gcc's
>> testsuite, where it fixed all hangs in guality.exp and
>> simulate-thread.exp testcases.
>>
>> Please note that I have no commit access, so if approved, please
>> commit the patch to the source repository for me. I also have
>> functionally equivalent patch for gdb-8 branch which I plan to submit
>> later.
>>
>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=19061#c5
>>
>> Uros.
>>
>
> Hi Uros and Richard,
>
> I would need your input.  Using this cross-compiler:
>
> alphaev67-unknown-linux-gnu-gcc (crosstool-NG crosstool-ng-1.22.0-677-ga3dd55b9) 6.3.0
> Copyright (C) 2016 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> I get this error:
>
>   CXX    linux-nat.o
> /home/simark/src/binutils-gdb/gdb/linux-nat.c: In function 'void save_stop_reason(lwp_info*)':
> /home/simark/src/binutils-gdb/gdb/linux-nat.c:2718:9: error: duplicated 'if' condition [-Werror=duplicated-cond]
>     else if (GDB_ARCH_IS_TRAP_HWBKPT (siginfo.si_code))
>          ^~
> In file included from /home/simark/src/binutils-gdb/gdb/linux-nat.c:31:0:
> /home/simark/src/binutils-gdb/gdb/nat/linux-ptrace.h:173:41: note: previously used here
>  # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
>                                     ~~~~~^~~~~~~~~~~~~~
> /home/simark/src/binutils-gdb/gdb/linux-nat.c:2709:13: note: in expansion of macro 'GDB_ARCH_IS_TRAP_BRKPT'
>     else if (GDB_ARCH_IS_TRAP_BRKPT (siginfo.si_code))
>              ^~~~~~~~~~~~~~~~~~~~~~
>
> Does Alpha even have hardware breakpoints?  If not, I would suggest
> defining GDB_ARCH_IS_TRAP_HWBKPT to 0 for __alpha__.  It would get
> rid of the error, and be more exact (no si_code can mean "hardware
> breakpoint" on alpha).

Richard said that:

The hardware does not, but the linux kernel pretends.
IIRC the number of round-trips and flushes is supposed
to be lower doing it from within the kernel.

BR,
Uros.
Simon Marchi May 30, 2018, 6:39 p.m. UTC | #4 
On 2018-05-30 14:07, Uros Bizjak wrote:
> Richard said that:
> 
> The hardware does not, but the linux kernel pretends.
> IIRC the number of round-trips and flushes is supposed
> to be lower doing it from within the kernel.

I don't really understand your response.  The hardware does not have 
hardware breakpoints, but the linux kernel pretends that it has?  Are 
you actually talking about single step?

Simon
diff mbox

Patch

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 21199bd988..f09050a73d 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -767,10 +767,8 @@  static const int stq_c_opcode = 0x2f;
    the sequence.  */
 
 static std::vector<CORE_ADDR>
-alpha_deal_with_atomic_sequence (struct regcache *regcache)
+alpha_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  struct gdbarch *gdbarch = regcache->arch ();
-  CORE_ADDR pc = regcache_read_pc (regcache);
   CORE_ADDR breaks[2] = {-1, -1};
   CORE_ADDR loc = pc;
   CORE_ADDR closing_insn; /* Instruction that closes the atomic sequence.  */
@@ -1723,9 +1721,19 @@  alpha_next_pc (struct regcache *regcache, CORE_ADDR pc)
 std::vector<CORE_ADDR>
 alpha_software_single_step (struct regcache *regcache)
 {
-  CORE_ADDR pc = alpha_next_pc (regcache, regcache_read_pc (regcache));
+  struct gdbarch *gdbarch = regcache->arch ();
+  CORE_ADDR pc, next_pc;
+
+  pc = regcache_read_pc (regcache);
+  std::vector<CORE_ADDR> next_pcs
+    = alpha_deal_with_atomic_sequence (gdbarch, pc);
+
+  if (!next_pcs.empty ())
+    return next_pcs;
+
+  next_pc = alpha_next_pc (regcache, pc);
 
-  return {pc};
+  return {next_pc};
 }
 
 
@@ -1821,7 +1829,7 @@  alpha_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
   set_gdbarch_cannot_step_breakpoint (gdbarch, 1);
 
   /* Handles single stepping of atomic sequences.  */
-  set_gdbarch_software_single_step (gdbarch, alpha_deal_with_atomic_sequence);
+  set_gdbarch_software_single_step (gdbarch, alpha_software_single_step);
 
   /* Hook in ABI-specific overrides, if they have been registered.  */
   gdbarch_init_osabi (info, gdbarch);
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index 59549452c0..8a8c4c6d3e 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -155,6 +155,8 @@  struct buffer;
    Beginning with Linux 4.6, the MIPS port reports proper TRAP_BRKPT and
    TRAP_HWBKPT codes, so we also match them.
 
+   The Alpha kernel uses TRAP_BRKPT for all traps.
+
    The generic Linux target code should use GDB_ARCH_IS_TRAP_* instead
    of TRAP_* to abstract out these peculiarities.  */
 #if defined __i386__ || defined __x86_64__
@@ -166,6 +168,9 @@  struct buffer;
 #elif defined __mips__
 # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == SI_KERNEL || (X) == TRAP_BRKPT)
 # define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == SI_KERNEL || (X) == TRAP_HWBKPT)
+#elif defined __alpha__
+# define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
+# define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == TRAP_BRKPT)
 #else
 # define GDB_ARCH_IS_TRAP_BRKPT(X) ((X) == TRAP_BRKPT)
 # define GDB_ARCH_IS_TRAP_HWBKPT(X) ((X) == TRAP_HWBKPT)