public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
@ 2017-12-15 12:11 Uros Bizjak
  2017-12-15 18:21 ` Pedro Alves
  2018-05-30 18:07 ` Simon Marchi
  0 siblings, 2 replies; 11+ messages in thread
From: Uros Bizjak @ 2017-12-15 12:11 UTC (permalink / raw)
  To: gdb-patches; +Cc: Richard Henderson, Tobias Klausmann

[-- Attachment #1: Type: text/plain, Size: 1004 bytes --]

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.

[-- Attachment #2: gdb-mainline.diff.txt --]
[-- Type: text/plain, Size: 2677 bytes --]

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};
 }
 
 \f
@@ -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)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2017-12-15 12:11 [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha Uros Bizjak
@ 2017-12-15 18:21 ` Pedro Alves
  2017-12-17 18:11   ` Uros Bizjak
  2018-05-30 18:07 ` Simon Marchi
  1 sibling, 1 reply; 11+ messages in thread
From: Pedro Alves @ 2017-12-15 18:21 UTC (permalink / raw)
  To: Uros Bizjak, gdb-patches; +Cc: Richard Henderson, Tobias Klausmann

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2017-12-15 18:21 ` Pedro Alves
@ 2017-12-17 18:11   ` Uros Bizjak
  2017-12-17 19:14     ` Uros Bizjak
  0 siblings, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2017-12-17 18:11 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Richard Henderson, Tobias Klausmann

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

On Fri, Dec 15, 2017 at 7:21 PM, Pedro Alves <palves@redhat.com> wrote:
> 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!

Attached, please find a patch for gdb-8.0 branch. The patch is tested
in the same way as the patch for mainline, with native
alphaev68-linux-gnu build and tests and with gcc's guality.exp and
simulate-thread.exp testcases.

Uros.

[-- Attachment #2: gdb.diff.txt --]
[-- Type: text/plain, Size: 1549 bytes --]

--- alpha-tdep.c	2017-12-14 15:56:48.955960647 +0100
+++ alpha-tdep.c.ub	2017-12-14 15:56:33.751806965 +0100
@@ -766,10 +766,8 @@
    the sequence.  */
 
 static VEC (CORE_ADDR) *
-alpha_deal_with_atomic_sequence (struct regcache *regcache)
+alpha_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  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.  */
@@ -1721,12 +1719,17 @@
 alpha_software_single_step (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  CORE_ADDR pc;
-  VEC (CORE_ADDR) *next_pcs = NULL;
+  CORE_ADDR pc, next_pc;
+  VEC (CORE_ADDR) *next_pcs;
 
   pc = regcache_read_pc (regcache);
+  next_pcs = alpha_deal_with_atomic_sequence (gdbarch, pc);
+  if (next_pcs != NULL)
+    return next_pcs;
+
+  next_pc = alpha_next_pc (regcache, pc);
 
-  VEC_safe_push (CORE_ADDR, next_pcs, alpha_next_pc (regcache, pc));
+  VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
   return next_pcs;
 }
 
@@ -1826,7 +1829,7 @@
   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);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2017-12-17 18:11   ` Uros Bizjak
@ 2017-12-17 19:14     ` Uros Bizjak
  2018-01-03 16:02       ` Pedro Alves
  0 siblings, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2017-12-17 19:14 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches, Richard Henderson, Tobias Klausmann

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Sun, Dec 17, 2017 at 7:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Dec 15, 2017 at 7:21 PM, Pedro Alves <palves@redhat.com> wrote:
>> 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!
>
> Attached, please find a patch for gdb-8.0 branch. The patch is tested
> in the same way as the patch for mainline, with native
> alphaev68-linux-gnu build and tests and with gcc's guality.exp and
> simulate-thread.exp testcases.

This one is the correct one.

Sorry, for the confusion,
Uros.

[-- Attachment #2: gdb-1.diff.txt --]
[-- Type: text/plain, Size: 2711 bytes --]

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 7c521db24c..2302e6ded3 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -766,10 +766,8 @@ static const int stq_c_opcode = 0x2f;
    the sequence.  */
 
 static VEC (CORE_ADDR) *
-alpha_deal_with_atomic_sequence (struct regcache *regcache)
+alpha_deal_with_atomic_sequence (struct gdbarch *gdbarch, CORE_ADDR pc)
 {
-  struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  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.  */
@@ -1721,12 +1719,17 @@ VEC (CORE_ADDR) *
 alpha_software_single_step (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
-  CORE_ADDR pc;
-  VEC (CORE_ADDR) *next_pcs = NULL;
+  CORE_ADDR pc, next_pc;
+  VEC (CORE_ADDR) *next_pcs;
 
   pc = regcache_read_pc (regcache);
+  next_pcs = alpha_deal_with_atomic_sequence (gdbarch, pc);
+  if (next_pcs != NULL)
+    return next_pcs;
+
+  next_pc = alpha_next_pc (regcache, pc);
 
-  VEC_safe_push (CORE_ADDR, next_pcs, alpha_next_pc (regcache, pc));
+  VEC_safe_push (CORE_ADDR, next_pcs, next_pc);
   return next_pcs;
 }
 
@@ -1826,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)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2017-12-17 19:14     ` Uros Bizjak
@ 2018-01-03 16:02       ` Pedro Alves
  0 siblings, 0 replies; 11+ messages in thread
From: Pedro Alves @ 2018-01-03 16:02 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gdb-patches, Richard Henderson, Tobias Klausmann

On 12/17/2017 07:14 PM, Uros Bizjak wrote:
> On Sun, Dec 17, 2017 at 7:11 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>> Attached, please find a patch for gdb-8.0 branch. The patch is tested
>> in the same way as the patch for mainline, with native
>> alphaev68-linux-gnu build and tests and with gcc's guality.exp and
>> simulate-thread.exp testcases.
> 
> This one is the correct one.

Merged.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2017-12-15 12:11 [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha Uros Bizjak
  2017-12-15 18:21 ` Pedro Alves
@ 2018-05-30 18:07 ` Simon Marchi
  2018-05-30 18:49   ` Uros Bizjak
  2018-05-31 11:10   ` Uros Bizjak
  1 sibling, 2 replies; 11+ messages in thread
From: Simon Marchi @ 2018-05-30 18:07 UTC (permalink / raw)
  To: Uros Bizjak, gdb-patches; +Cc: Richard Henderson, Tobias Klausmann

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2018-05-30 18:07 ` Simon Marchi
@ 2018-05-30 18:49   ` Uros Bizjak
  2018-05-30 19:04     ` Simon Marchi
  2018-05-31 11:10   ` Uros Bizjak
  1 sibling, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2018-05-30 18:49 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Tobias Klausmann, rth

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2018-05-30 18:49   ` Uros Bizjak
@ 2018-05-30 19:04     ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2018-05-30 19:04 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Simon Marchi, gdb-patches, Tobias Klausmann, rth

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2018-05-30 18:07 ` Simon Marchi
  2018-05-30 18:49   ` Uros Bizjak
@ 2018-05-31 11:10   ` Uros Bizjak
  2018-05-31 14:55     ` Simon Marchi
  1 sibling, 1 reply; 11+ messages in thread
From: Uros Bizjak @ 2018-05-31 11:10 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, Tobias Klausmann, rth

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).

I didn't find any mentions of hardware breakpoint support in Alpha
Architecture Handbook v.4.

Please note that save_stop_reason from linux-nat.c has some code to
detect ambigous si_code, where si_code is both TRAP_BRKPT and
TRAP_HWBKPT, and returns TARGET_STOPPED_BY_SW_BREAKPOINT in this case.
So, if we don't define anything for __alpha__ in nat/linux-ptrace.h,
we get the same result.

I have tested the attached patch, and resulting gdb works OK for me.

gdb/ChangeLog:

2018-05-31  Uros Bizjak  <ubizjak@gmail.com>

        * nat/linux-ptrace.h  [__alpha__]: Remove definitions.

--cut here--
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index dc180fbf82..98b44a82a6 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -156,8 +156,6 @@ 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__
@@ -169,9 +167,6 @@ 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)
--cut here--

Uros.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2018-05-31 11:10   ` Uros Bizjak
@ 2018-05-31 14:55     ` Simon Marchi
  2018-05-31 16:18       ` Simon Marchi
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2018-05-31 14:55 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Simon Marchi, gdb-patches, Tobias Klausmann, rth

On 2018-05-31 05:23, Uros Bizjak wrote:
> On Wed, May 30, 2018 at 4:53 PM, Simon Marchi 
> <simon.marchi@ericsson.com> wrote:
>> 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).
> 
> I didn't find any mentions of hardware breakpoint support in Alpha
> Architecture Handbook v.4.
> 
> Please note that save_stop_reason from linux-nat.c has some code to
> detect ambigous si_code, where si_code is both TRAP_BRKPT and
> TRAP_HWBKPT, and returns TARGET_STOPPED_BY_SW_BREAKPOINT in this case.
> So, if we don't define anything for __alpha__ in nat/linux-ptrace.h,
> we get the same result.

TRAP_BKPT and TRAP_HWBKPT are different, so this will never happen with 
the default definitions of 
GDB_ARCH_IS_TRAP_BRKPT/GDB_ARCH_IS_TRAP_HWBKPT.  Since the kernel for 
alpha never sets si_code to TRAP_HWBKPT, it means we'll never get in to 
the HW breakpoint branch, which is fine.

In short, the patch LGTM.

> I have tested the attached patch, and resulting gdb works OK for me.

Thanks!

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha
  2018-05-31 14:55     ` Simon Marchi
@ 2018-05-31 16:18       ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2018-05-31 16:18 UTC (permalink / raw)
  To: Simon Marchi, Uros Bizjak; +Cc: gdb-patches, Tobias Klausmann, rth

On 2018-05-31 10:36 AM, Simon Marchi wrote:
> On 2018-05-31 05:23, Uros Bizjak wrote:
>> On Wed, May 30, 2018 at 4:53 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
>>> 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).
>>
>> I didn't find any mentions of hardware breakpoint support in Alpha
>> Architecture Handbook v.4.
>>
>> Please note that save_stop_reason from linux-nat.c has some code to
>> detect ambigous si_code, where si_code is both TRAP_BRKPT and
>> TRAP_HWBKPT, and returns TARGET_STOPPED_BY_SW_BREAKPOINT in this case.
>> So, if we don't define anything for __alpha__ in nat/linux-ptrace.h,
>> we get the same result.
> 
> TRAP_BKPT and TRAP_HWBKPT are different, so this will never happen with the default definitions of GDB_ARCH_IS_TRAP_BRKPT/GDB_ARCH_IS_TRAP_HWBKPT.  Since the kernel for alpha never sets si_code to TRAP_HWBKPT, it means we'll never get in to the HW breakpoint branch, which is fine.
> 
> In short, the patch LGTM.
> 
>> I have tested the attached patch, and resulting gdb works OK for me.
> 
> Thanks!
> 
> Simon

Since Uros doesn't have write access, I wrote a commit log and pushed
the patch.

Simon


From 8a60efe714e636c9f958058a8dfb12de81bdcbfa Mon Sep 17 00:00:00 2001
From: Uros Bizjak <ubizjak@gmail.com>
Date: Thu, 31 May 2018 11:18:02 -0400
Subject: [PATCH] Fix Alpha native GDB build

[Commit log by Simon Marchi]

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))
             ^~~~~~~~~~~~~~~~~~~~~~

For Alpha, we currently define GDB_ARCH_IS_TRAP_BRKPT and
GDB_ARCH_IS_TRAP_HWBKPT both to ((X) == TRAP_BRKPT), which causes the
two if branches to be duplicated.

Alpha doesn't have hardware breakpoints, so the Linux kernel for Alpha
never sets si_code to TRAP_HWBKPT.  We can just remove the special
definitions of these macros for __alpha__ and rely on the default ones.
Since the kernel will never report TRAP_HWBKPT, we will just never enter
the "hardware breakpoint" branch on Alpha (which is fine since it
doesn't have them).

gdb/ChangeLog:

	* nat/linux-ptrace.h [__alpha__]
	(GDB_ARCH_IS_TRAP_BRKPT, GDB_ARCH_IS_TRAP_HWBKPT): Remove
	definitions.
---
 gdb/ChangeLog          | 6 ++++++
 gdb/nat/linux-ptrace.h | 5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c6c862f..8af7e67 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-05-31  Uros Bizjak  <ubizjak@gmail.com>
+
+	* nat/linux-ptrace.h [__alpha__]
+	(GDB_ARCH_IS_TRAP_BRKPT, GDB_ARCH_IS_TRAP_HWBKPT): Remove
+	definitions.
+
 2018-05-31  Maciej W. Rozycki  <macro@mips.com>

 	* arch-utils.c (gdbarch_info_fill): Set `default_byte_order' to
diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h
index dc180fb..98b44a8 100644
--- a/gdb/nat/linux-ptrace.h
+++ b/gdb/nat/linux-ptrace.h
@@ -156,8 +156,6 @@ 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__
@@ -169,9 +167,6 @@ 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)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-31 15:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-15 12:11 [PATCH]: Fix PR19061, gdb hangs/spins-on-cpu when debugging any program on Alpha Uros Bizjak
2017-12-15 18:21 ` Pedro Alves
2017-12-17 18:11   ` Uros Bizjak
2017-12-17 19:14     ` Uros Bizjak
2018-01-03 16:02       ` Pedro Alves
2018-05-30 18:07 ` Simon Marchi
2018-05-30 18:49   ` Uros Bizjak
2018-05-30 19:04     ` Simon Marchi
2018-05-31 11:10   ` Uros Bizjak
2018-05-31 14:55     ` Simon Marchi
2018-05-31 16:18       ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).