[PATCH 0/5] ptrace low level unification

[PATCH 0/5] ptrace low level unification

[Mike Frysinger]

This is a series of low level ptrace unification steps to make it easier
for common code (like KGDB) to poke at register state.  This also avoids
having to duplicate higher level operations for most ports which don't
have special needs for accessing things.

Mike Frysinger (5):
  asm-generic/ptrace.h: start a common low level ptrace helper
  Blackfin: convert to asm-generic ptrace.h
  x86: convert to asm-generic ptrace.h
  sh: convert to asm-generic ptrace.h
  kgdbts: unify/generalize gdb breakpoint adjustment

 arch/blackfin/include/asm/kgdb.h   |    1 +
 arch/blackfin/include/asm/ptrace.h |    5 +-
 arch/sh/include/asm/kgdb.h         |    1 +
 arch/sh/include/asm/ptrace.h       |    6 ++-
 arch/x86/include/asm/kgdb.h        |    1 +
 arch/x86/include/asm/ptrace.h      |   18 ++------
 drivers/misc/kgdbts.c              |   29 +++++---------
 include/asm-generic/ptrace.h       |   74 ++++++++++++++++++++++++++++++++++++
 8 files changed, 99 insertions(+), 36 deletions(-)
 create mode 100644 include/asm-generic/ptrace.h

-- 
1.7.5.rc1

[PATCH 1/5] asm-generic/ptrace.h: start a common low level ptrace helper

[Mike Frysinger]

This implements a bunch of helper funcs for poking the registers of a
ptrace structure.  Now common code should be able to portably update
specific registers (like kgdb updating the PC).

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 include/asm-generic/ptrace.h |   74 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 include/asm-generic/ptrace.h

diff --git a/include/asm-generic/ptrace.h b/include/asm-generic/ptrace.h
new file mode 100644
index 0000000..82e674f
--- /dev/null
+++ b/include/asm-generic/ptrace.h
@@ -0,0 +1,74 @@
+/*
+ * Common low level (register) ptrace helpers
+ *
+ * Copyright 2004-2011 Analog Devices Inc.
+ *
+ * Licensed under the GPL-2 or later.
+ */
+
+#ifndef __ASM_GENERIC_PTRACE_H__
+#define __ASM_GENERIC_PTRACE_H__
+
+#ifndef __ASSEMBLY__
+
+/* Helpers for working with the instruction pointer */
+#ifndef GET_IP
+#define GET_IP(regs) ((regs)->pc)
+#endif
+#ifndef SET_IP
+#define SET_IP(regs, val) (GET_IP(regs) = (val))
+#endif
+
+static inline unsigned long instruction_pointer(struct pt_regs *regs)
+{
+	return GET_IP(regs);
+}
+static inline void instruction_pointer_set(struct pt_regs *regs,
+                                           unsigned long val)
+{
+	SET_IP(regs, val);
+}
+
+#ifndef profile_pc
+#define profile_pc(regs) instruction_pointer(regs)
+#endif
+
+/* Helpers for working with the user stack pointer */
+#ifndef GET_USP
+#define GET_USP(regs) ((regs)->usp)
+#endif
+#ifndef SET_USP
+#define SET_USP(regs, val) (GET_USP(regs) = (val))
+#endif
+
+static inline unsigned long user_stack_pointer(struct pt_regs *regs)
+{
+	return GET_USP(regs);
+}
+static inline void user_stack_pointer_set(struct pt_regs *regs,
+                                          unsigned long val)
+{
+	SET_USP(regs, val);
+}
+
+/* Helpers for working with the frame pointer */
+#ifndef GET_FP
+#define GET_FP(regs) ((regs)->fp)
+#endif
+#ifndef SET_FP
+#define SET_FP(regs, val) (GET_FP(regs) = (val))
+#endif
+
+static inline unsigned long frame_pointer(struct pt_regs *regs)
+{
+	return GET_FP(regs);
+}
+static inline void frame_pointer_set(struct pt_regs *regs,
+                                     unsigned long val)
+{
+	SET_FP(regs, val);
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif
-- 
1.7.5.rc1

Re: [PATCH 1/5] asm-generic/ptrace.h: start a common low level

[Oleg Nesterov]

On 04/14, Mike Frysinger wrote:
>
> This implements a bunch of helper funcs for poking the registers of a
> ptrace structure.  Now common code should be able to portably update
> specific registers (like kgdb updating the PC).

The whole series looks correct, but I am a bit confused...

> +#ifndef GET_IP
> +#define GET_IP(regs) ((regs)->pc)
> +#endif

Could you explain this ifndef ?

IIUC, this should be included by arch/*/asm/ptrace.h. Isn't it better
to simply require that if you include asm-generic/ptrace.h you should
provide the necessary GET_* macros?

(regs)->pc looks a bit strange in asm-generic. But please feel free
to ignore.

Oleg.

Re: [PATCH 1/5] asm-generic/ptrace.h: start a common low level ptrace helper

[Mike Frysinger]

On Thu, Apr 14, 2011 at 14:09, Oleg Nesterov wrote:
> On 04/14, Mike Frysinger wrote:
>> +#ifndef GET_IP
>> +#define GET_IP(regs) ((regs)->pc)
>> +#endif
>
> Could you explain this ifndef ?
>
> IIUC, this should be included by arch/*/asm/ptrace.h. Isn't it better
> to simply require that if you include asm-generic/ptrace.h you should
> provide the necessary GET_* macros?
>
> (regs)->pc looks a bit strange in asm-generic. But please feel free
> to ignore.

my view of asm-generic is to put as much common/sane-defaults in there
as possible to minimize code duplication in arch code.  when it comes
to the register names, i looked at the arches to see what people used.
while x86 uses "ip", the majority of ports use "pc", thus the
majority of ports wont have to define their own GET_IP helper.
-mike

[PATCH 2/5] Blackfin: convert to asm-generic ptrace.h

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/include/asm/ptrace.h |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/blackfin/include/asm/ptrace.h b/arch/blackfin/include/asm/ptrace.h
index 681a92c..10d8641 100644
--- a/arch/blackfin/include/asm/ptrace.h
+++ b/arch/blackfin/include/asm/ptrace.h
@@ -102,9 +102,6 @@ struct pt_regs {
 /* user_mode returns true if only one bit is set in IPEND, other than the
    master interrupt enable.  */
 #define user_mode(regs) (!(((regs)->ipend & ~0x10) & (((regs)->ipend & ~0x10) - 1)))
-#define instruction_pointer(regs) ((regs)->pc)
-#define user_stack_pointer(regs)  ((regs)->usp)
-#define profile_pc(regs) instruction_pointer(regs)
 
 #define arch_has_single_step()	(1)
 /* common code demands this function */
@@ -127,6 +124,8 @@ extern int is_user_addr_valid(struct task_struct *child,
 	    ((unsigned long)task_stack_page(task) + \
 	     (THREAD_SIZE - sizeof(struct pt_regs)))
 
+#include <asm-generic/ptrace.h>
+
 #endif  /*  __KERNEL__  */
 
 #endif				/* __ASSEMBLY__ */
-- 
1.7.5.rc1

[PATCH 3/5] x86: convert to asm-generic ptrace.h

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/x86/include/asm/ptrace.h |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 78cd1ea..98a5e97 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -136,6 +136,7 @@ struct cpuinfo_x86;
 struct task_struct;
 
 extern unsigned long profile_pc(struct pt_regs *regs);
+#define profile_pc profile_pc
 
 extern unsigned long
 convert_ip_to_linear(struct task_struct *child, struct pt_regs *regs);
@@ -202,20 +203,11 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
 #endif
 }
 
-static inline unsigned long instruction_pointer(struct pt_regs *regs)
-{
-	return regs->ip;
-}
-
-static inline unsigned long frame_pointer(struct pt_regs *regs)
-{
-	return regs->bp;
-}
+#define GET_IP(regs) ((regs)->ip)
+#define GET_FP(regs) ((regs)->bp)
+#define GET_USP(regs) ((regs)->sp)
 
-static inline unsigned long user_stack_pointer(struct pt_regs *regs)
-{
-	return regs->sp;
-}
+#include <asm-generic/ptrace.h>
 
 /* Query offset/name of register from its name/offset */
 extern int regs_query_register_offset(const char *name);
-- 
1.7.5.rc1

Re: [Kgdb-bugreport] [PATCH 3/5] x86: convert to asm-generic ptrace.h

[Sergei Shtylyov]

Hello.

Mike Frysinger wrote:

> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  arch/x86/include/asm/ptrace.h |   18 +++++-------------
>  1 files changed, 5 insertions(+), 13 deletions(-)

> diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
> index 78cd1ea..98a5e97 100644
> --- a/arch/x86/include/asm/ptrace.h
> +++ b/arch/x86/include/asm/ptrace.h
> @@ -136,6 +136,7 @@ struct cpuinfo_x86;
>  struct task_struct;
>  
>  extern unsigned long profile_pc(struct pt_regs *regs);
> +#define profile_pc profile_pc

    Er, won't this lead to infinite recursion in preprocessor? :-)

WBR, Sergei

Re: [Kgdb-bugreport] [PATCH 3/5] x86: convert to asm-generic ptrace.h

[H. Peter Anvin]

On 04/14/2011 07:16 AM, Sergei Shtylyov wrote:
> Hello.
>
> Mike Frysinger wrote:
>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>> ---
>> arch/x86/include/asm/ptrace.h | 18 +++++-------------
>> 1 files changed, 5 insertions(+), 13 deletions(-)
>
>> diff --git a/arch/x86/include/asm/ptrace.h
>> b/arch/x86/include/asm/ptrace.h
>> index 78cd1ea..98a5e97 100644
>> --- a/arch/x86/include/asm/ptrace.h
>> +++ b/arch/x86/include/asm/ptrace.h
>> @@ -136,6 +136,7 @@ struct cpuinfo_x86;
>> struct task_struct;
>>
>> extern unsigned long profile_pc(struct pt_regs *regs);
>> +#define profile_pc profile_pc
>
> Er, won't this lead to infinite recursion in preprocessor? :-)
>
> WBR, Sergei
>

No.

	-hpa

Re: [Kgdb-bugreport] [PATCH 3/5] x86: convert to asm-generic ptrace.h

[Sergei Shtylyov]

Hello.

H. Peter Anvin wrote:

>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>> ---
>>> arch/x86/include/asm/ptrace.h | 18 +++++-------------
>>> 1 files changed, 5 insertions(+), 13 deletions(-)

>>> diff --git a/arch/x86/include/asm/ptrace.h
>>> b/arch/x86/include/asm/ptrace.h
>>> index 78cd1ea..98a5e97 100644
>>> --- a/arch/x86/include/asm/ptrace.h
>>> +++ b/arch/x86/include/asm/ptrace.h
>>> @@ -136,6 +136,7 @@ struct cpuinfo_x86;
>>> struct task_struct;
>>>
>>> extern unsigned long profile_pc(struct pt_regs *regs);
>>> +#define profile_pc profile_pc

>> Er, won't this lead to infinite recursion in preprocessor? :-)

>> WBR, Sergei

> No.

    Strange, I thought the macro expansion is the subject to other macro expansions.

>     -hpa

WBR, Sergei

[PATCH 4/5] sh: convert to asm-generic ptrace.h

[Mike Frysinger]

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/sh/include/asm/ptrace.h |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/sh/include/asm/ptrace.h b/arch/sh/include/asm/ptrace.h
index b219a08..3f2a6e7 100644
--- a/arch/sh/include/asm/ptrace.h
+++ b/arch/sh/include/asm/ptrace.h
@@ -40,9 +40,8 @@
 #include <asm/system.h>
 
 #define user_mode(regs)			(((regs)->sr & 0x40000000)=0)
-#define user_stack_pointer(_regs)	((unsigned long)(_regs)->regs[15])
 #define kernel_stack_pointer(_regs)	((unsigned long)(_regs)->regs[15])
-#define instruction_pointer(regs)	((unsigned long)(regs)->pc)
+#define GET_USP(regs) ((regs)->regs[15])
 
 #define arch_has_single_step()	(1)
 
@@ -137,6 +136,9 @@ static inline unsigned long profile_pc(struct pt_regs *regs)
 
 	return pc;
 }
+#define profile_pc profile_pc
+
+#include <asm-generic/ptrace.h>
 #endif /* __KERNEL__ */
 
 #endif /* __ASM_SH_PTRACE_H */
-- 
1.7.5.rc1

Re: [Kgdb-bugreport] [PATCH 4/5] sh: convert to asm-generic ptrace.h

[Sergei Shtylyov]

Hello.

Mike Frysinger wrote:

> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>  arch/sh/include/asm/ptrace.h |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)

> diff --git a/arch/sh/include/asm/ptrace.h b/arch/sh/include/asm/ptrace.h
> index b219a08..3f2a6e7 100644
> --- a/arch/sh/include/asm/ptrace.h
> +++ b/arch/sh/include/asm/ptrace.h
[...]
> @@ -137,6 +136,9 @@ static inline unsigned long profile_pc(struct pt_regs *regs)
>  
>  	return pc;
>  }
> +#define profile_pc profile_pc

    Same question about preprocessor...

WBR, Sergei

Re: [Kgdb-bugreport] [PATCH 4/5] sh: convert to asm-generic ptrace.h

[Paul Mundt]

On Thu, Apr 14, 2011 at 06:22:17PM +0400, Sergei Shtylyov wrote:
> Mike Frysinger wrote:
> 
> >Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >---
> > arch/sh/include/asm/ptrace.h |    6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> 
> >diff --git a/arch/sh/include/asm/ptrace.h b/arch/sh/include/asm/ptrace.h
> >index b219a08..3f2a6e7 100644
> >--- a/arch/sh/include/asm/ptrace.h
> >+++ b/arch/sh/include/asm/ptrace.h
> [...]
> >@@ -137,6 +136,9 @@ static inline unsigned long profile_pc(struct pt_regs 
> >*regs)
> > 
> > 	return pc;
> > }
> >+#define profile_pc profile_pc
> 
>    Same question about preprocessor...
> 
Same response.

There are many places in the kernel where this convention is used, and
for a good reason. Grep harder.

[PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Mike Frysinger]

The Blackfin arch, like the x86 arch, needs to adjust the PC manually
after a breakpoint is hit as normally this is handled by the remote gdb.
However, rather than starting another arch ifdef mess, create a common
GDB_ADJUSTS_BREAK_OFFSET define for any arch to opt-in via their kgdb.h.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 arch/blackfin/include/asm/kgdb.h |    1 +
 arch/sh/include/asm/kgdb.h       |    1 +
 arch/x86/include/asm/kgdb.h      |    1 +
 drivers/misc/kgdbts.c            |   29 +++++++++++------------------
 4 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/arch/blackfin/include/asm/kgdb.h b/arch/blackfin/include/asm/kgdb.h
index 3ac0c72..aaf8845 100644
--- a/arch/blackfin/include/asm/kgdb.h
+++ b/arch/blackfin/include/asm/kgdb.h
@@ -108,6 +108,7 @@ static inline void arch_kgdb_breakpoint(void)
 #else
 # define CACHE_FLUSH_IS_SAFE	1
 #endif
+#define GDB_ADJUSTS_BREAK_OFFSET
 #define HW_INST_WATCHPOINT_NUM	6
 #define HW_WATCHPOINT_NUM	8
 #define TYPE_INST_WATCHPOINT	0
diff --git a/arch/sh/include/asm/kgdb.h b/arch/sh/include/asm/kgdb.h
index 4235e22..f361395 100644
--- a/arch/sh/include/asm/kgdb.h
+++ b/arch/sh/include/asm/kgdb.h
@@ -34,5 +34,6 @@ static inline void arch_kgdb_breakpoint(void)
 
 #define CACHE_FLUSH_IS_SAFE	1
 #define BREAK_INSTR_SIZE	2
+#define GDB_ADJUSTS_BREAK_OFFSET
 
 #endif /* __ASM_SH_KGDB_H */
diff --git a/arch/x86/include/asm/kgdb.h b/arch/x86/include/asm/kgdb.h
index 396f5b5..77e95f5 100644
--- a/arch/x86/include/asm/kgdb.h
+++ b/arch/x86/include/asm/kgdb.h
@@ -77,6 +77,7 @@ static inline void arch_kgdb_breakpoint(void)
 }
 #define BREAK_INSTR_SIZE	1
 #define CACHE_FLUSH_IS_SAFE	1
+#define GDB_ADJUSTS_BREAK_OFFSET
 
 extern int kgdb_ll_trap(int cmd, const char *str,
 			struct pt_regs *regs, long err, int trap, int sig);
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 59c118c..d475ce0 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -285,33 +285,26 @@ static void hw_break_val_write(void)
 static int check_and_rewind_pc(char *put_str, char *arg)
 {
 	unsigned long addr = lookup_addr(arg);
+	unsigned long ip;
 	int offset = 0;
 
 	kgdb_hex2mem(&put_str[1], (char *)kgdbts_gdb_regs,
 		 NUMREGBYTES);
 	gdb_regs_to_pt_regs(kgdbts_gdb_regs, &kgdbts_regs);
-	v2printk("Stopped at IP: %lx\n", instruction_pointer(&kgdbts_regs));
-#ifdef CONFIG_X86
-	/* On x86 a breakpoint stop requires it to be decremented */
-	if (addr + 1 = kgdbts_regs.ip)
-		offset = -1;
-#elif defined(CONFIG_SUPERH)
-	/* On SUPERH a breakpoint stop requires it to be decremented */
-	if (addr + 2 = kgdbts_regs.pc)
-		offset = -2;
+	ip = instruction_pointer(&kgdbts_regs);
+	v2printk("Stopped at IP: %lx\n", ip);
+#ifdef GDB_ADJUSTS_BREAK_OFFSET
+	/* On some arches, a breakpoint stop requires it to be decremented */
+	if (addr + BREAK_INSTR_SIZE = ip)
+		offset = -BREAK_INSTR_SIZE;
 #endif
-	if (strcmp(arg, "silent") &&
-		instruction_pointer(&kgdbts_regs) + offset != addr) {
+	if (strcmp(arg, "silent") && ip + offset != addr) {
 		eprintk("kgdbts: BP mismatch %lx expected %lx\n",
-			   instruction_pointer(&kgdbts_regs) + offset, addr);
+			   ip + offset, addr);
 		return 1;
 	}
-#ifdef CONFIG_X86
-	/* On x86 adjust the instruction pointer if needed */
-	kgdbts_regs.ip += offset;
-#elif defined(CONFIG_SUPERH)
-	kgdbts_regs.pc += offset;
-#endif
+	/* Readjust the instruction pointer if needed */
+	instruction_pointer_set(&kgdbts_regs, ip + offset);
 	return 0;
 }
 
-- 
1.7.5.rc1

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Paul Mundt]

On Thu, Apr 14, 2011 at 02:01:35AM -0400, Mike Frysinger wrote:
> The Blackfin arch, like the x86 arch, needs to adjust the PC manually
> after a breakpoint is hit as normally this is handled by the remote gdb.
> However, rather than starting another arch ifdef mess, create a common
> GDB_ADJUSTS_BREAK_OFFSET define for any arch to opt-in via their kgdb.h.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

Acked-by: Paul Mundt <lethal@linux-sh.org>

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Arnd Bergmann]

On Monday 18 April 2011, Paul Mundt wrote:
> On Thu, Apr 14, 2011 at 02:01:35AM -0400, Mike Frysinger wrote:
> > The Blackfin arch, like the x86 arch, needs to adjust the PC manually
> > after a breakpoint is hit as normally this is handled by the remote gdb.
> > However, rather than starting another arch ifdef mess, create a common
> > GDB_ADJUSTS_BREAK_OFFSET define for any arch to opt-in via their kgdb.h.
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> Acked-by: Paul Mundt <lethal@linux-sh.org>


This patch is causing build problems now, because the instruction_pointer_set()
that is used now is only defined in asm-generic/ptrace.h, which is only
used on x86, blackfin and sparc. Every other architecture now fails to
build.

	Arnd

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Mike Frysinger]

On Thu, Jun 16, 2011 at 11:07, Arnd Bergmann wrote:
> This patch is causing build problems now, because the instruction_pointer_set()
> that is used now is only defined in asm-generic/ptrace.h, which is only
> used on x86, blackfin and sparc. Every other architecture now fails to
> build.

a fix was already merged over two weeks ago.  so either your tree is
out of date,

your tree is out of date.

you also need a little qualification there ... every arch *which
supports kgdb and enables it* now fails to build.
-mike

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Mike Frysinger]

On Thu, Jun 16, 2011 at 16:06, Mike Frysinger wrote:
> On Thu, Jun 16, 2011 at 11:07, Arnd Bergmann wrote:
>> This patch is causing build problems now, because the instruction_pointer_set()
>> that is used now is only defined in asm-generic/ptrace.h, which is only
>> used on x86, blackfin and sparc. Every other architecture now fails to
>> build.
>
> a fix was already merged over two weeks ago.  so either your tree is
> out of date,
>
> your tree is out of date.

blah, network died before i could finish editing this

your tree is out of date or you're seeing a different issue and need
to post a full build log
-mike

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Arnd Bergmann]

On Thursday 16 June 2011 22:07:55 Mike Frysinger wrote:
> On Thu, Jun 16, 2011 at 16:06, Mike Frysinger wrote:
> > On Thu, Jun 16, 2011 at 11:07, Arnd Bergmann wrote:
> >> This patch is causing build problems now, because the instruction_pointer_set()
> >> that is used now is only defined in asm-generic/ptrace.h, which is only
> >> used on x86, blackfin and sparc. Every other architecture now fails to
> >> build.
> >
> > a fix was already merged over two weeks ago.  so either your tree is
> > out of date,
> >
> > your tree is out of date.
> 
> blah, network died before i could finish editing this
> 
> your tree is out of date or you're seeing a different issue and need
> to post a full build log

Ok, sorry for the noise. I've only now completed going through the build
errors of ARM randconfig on -rc1 and obviously need to rebase my fixes.

	Arnd

Re: [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint adjustment

[Mike Frysinger]

On Thu, Jun 16, 2011 at 16:21, Arnd Bergmann wrote:
> On Thursday 16 June 2011 22:07:55 Mike Frysinger wrote:
>> On Thu, Jun 16, 2011 at 16:06, Mike Frysinger wrote:
>> > On Thu, Jun 16, 2011 at 11:07, Arnd Bergmann wrote:
>> >> This patch is causing build problems now, because the instruction_pointer_set()
>> >> that is used now is only defined in asm-generic/ptrace.h, which is only
>> >> used on x86, blackfin and sparc. Every other architecture now fails to
>> >> build.
>> >
>> > a fix was already merged over two weeks ago.  so either your tree is
>> > out of date,
>> >
>> > your tree is out of date.
>>
>> blah, network died before i could finish editing this
>>
>> your tree is out of date or you're seeing a different issue and need
>> to post a full build log
>
> Ok, sorry for the noise. I've only now completed going through the build
> errors of ARM randconfig on -rc1 and obviously need to rebase my fixes.

yeah, rc1 will be busticated when building kgdb
-mike

Re: [Kgdb-bugreport] [PATCH 5/5] kgdbts: unify/generalize gdb breakpoint

[DDD]

Mike Frysinger wrote:
> The Blackfin arch, like the x86 arch, needs to adjust the PC manually
> after a breakpoint is hit as normally this is handled by the remote gdb.
> However, rather than starting another arch ifdef mess, create a common
> GDB_ADJUSTS_BREAK_OFFSET define for any arch to opt-in via their kgdb.h.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>


Acked-by: Dongdong Deng <dongdong.deng@windriver.com>


> ---
>  arch/blackfin/include/asm/kgdb.h |    1 +
>  arch/sh/include/asm/kgdb.h       |    1 +
>  arch/x86/include/asm/kgdb.h      |    1 +
>  drivers/misc/kgdbts.c            |   29 +++++++++++------------------
>  4 files changed, 14 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/blackfin/include/asm/kgdb.h b/arch/blackfin/include/asm/kgdb.h
> index 3ac0c72..aaf8845 100644
> --- a/arch/blackfin/include/asm/kgdb.h
> +++ b/arch/blackfin/include/asm/kgdb.h
> @@ -108,6 +108,7 @@ static inline void arch_kgdb_breakpoint(void)
>  #else
>  # define CACHE_FLUSH_IS_SAFE	1
>  #endif
> +#define GDB_ADJUSTS_BREAK_OFFSET
>  #define HW_INST_WATCHPOINT_NUM	6
>  #define HW_WATCHPOINT_NUM	8
>  #define TYPE_INST_WATCHPOINT	0
> diff --git a/arch/sh/include/asm/kgdb.h b/arch/sh/include/asm/kgdb.h
> index 4235e22..f361395 100644
> --- a/arch/sh/include/asm/kgdb.h
> +++ b/arch/sh/include/asm/kgdb.h
> @@ -34,5 +34,6 @@ static inline void arch_kgdb_breakpoint(void)
>  
>  #define CACHE_FLUSH_IS_SAFE	1
>  #define BREAK_INSTR_SIZE	2
> +#define GDB_ADJUSTS_BREAK_OFFSET
>  
>  #endif /* __ASM_SH_KGDB_H */
> diff --git a/arch/x86/include/asm/kgdb.h b/arch/x86/include/asm/kgdb.h
> index 396f5b5..77e95f5 100644
> --- a/arch/x86/include/asm/kgdb.h
> +++ b/arch/x86/include/asm/kgdb.h
> @@ -77,6 +77,7 @@ static inline void arch_kgdb_breakpoint(void)
>  }
>  #define BREAK_INSTR_SIZE	1
>  #define CACHE_FLUSH_IS_SAFE	1
> +#define GDB_ADJUSTS_BREAK_OFFSET
>  
>  extern int kgdb_ll_trap(int cmd, const char *str,
>  			struct pt_regs *regs, long err, int trap, int sig);
> diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
> index 59c118c..d475ce0 100644
> --- a/drivers/misc/kgdbts.c
> +++ b/drivers/misc/kgdbts.c
> @@ -285,33 +285,26 @@ static void hw_break_val_write(void)
>  static int check_and_rewind_pc(char *put_str, char *arg)
>  {
>  	unsigned long addr = lookup_addr(arg);
> +	unsigned long ip;
>  	int offset = 0;
>  
>  	kgdb_hex2mem(&put_str[1], (char *)kgdbts_gdb_regs,
>  		 NUMREGBYTES);
>  	gdb_regs_to_pt_regs(kgdbts_gdb_regs, &kgdbts_regs);
> -	v2printk("Stopped at IP: %lx\n", instruction_pointer(&kgdbts_regs));
> -#ifdef CONFIG_X86
> -	/* On x86 a breakpoint stop requires it to be decremented */
> -	if (addr + 1 = kgdbts_regs.ip)
> -		offset = -1;
> -#elif defined(CONFIG_SUPERH)
> -	/* On SUPERH a breakpoint stop requires it to be decremented */
> -	if (addr + 2 = kgdbts_regs.pc)
> -		offset = -2;
> +	ip = instruction_pointer(&kgdbts_regs);
> +	v2printk("Stopped at IP: %lx\n", ip);
> +#ifdef GDB_ADJUSTS_BREAK_OFFSET
> +	/* On some arches, a breakpoint stop requires it to be decremented */
> +	if (addr + BREAK_INSTR_SIZE = ip)
> +		offset = -BREAK_INSTR_SIZE;
>  #endif
> -	if (strcmp(arg, "silent") &&
> -		instruction_pointer(&kgdbts_regs) + offset != addr) {
> +	if (strcmp(arg, "silent") && ip + offset != addr) {
>  		eprintk("kgdbts: BP mismatch %lx expected %lx\n",
> -			   instruction_pointer(&kgdbts_regs) + offset, addr);
> +			   ip + offset, addr);
>  		return 1;
>  	}
> -#ifdef CONFIG_X86
> -	/* On x86 adjust the instruction pointer if needed */
> -	kgdbts_regs.ip += offset;
> -#elif defined(CONFIG_SUPERH)
> -	kgdbts_regs.pc += offset;
> -#endif
> +	/* Readjust the instruction pointer if needed */
> +	instruction_pointer_set(&kgdbts_regs, ip + offset);
>  	return 0;
>  }
>