* [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
@ 2011-06-24 2:31 Terry Guo
2011-06-24 3:55 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Terry Guo @ 2011-06-24 2:31 UTC (permalink / raw)
To: gdb-patches
Hello,
This patch addresses the bug in gdb/12703 which sets two different function
breakpoints at same pc address. In this patch I enhanced the way to analyze
the ARM thumb prologue to prevent the function breakpoint from being set
outside the function body.
Patch has been tested against arm-none-eabi with no regressions. OK for
commit?
Thanks,
Terry
gdb/ChangLog:
2011-06-16 Terry Guo <terry.guo@arm.com>
PR gdb/12703
* arm-tdep.c (arm_skip_prologue): Don't scan beyond the end of
the current function.
gdb/testsuite/ChangLog:
2011-06-16 Terry Guo <terry.guo@arm.com>
PR gdb/12703
* gdb.base/break-function.c: New testcase.
* gdb.base/break-function.exp: New script.
diff --git gdb/arm-tdep.c gdb/arm-tdep.c
index 2dd8c9e..c188576 100644
--- gdb/arm-tdep.c
+++ gdb/arm-tdep.c
@@ -1372,13 +1372,13 @@ arm_skip_prologue (struct gdbarch *gdbarch,
CORE_ADDR pc)
enum bfd_endian byte_order_for_code = gdbarch_byte_order_for_code
(gdbarch);
unsigned long inst;
CORE_ADDR skip_pc;
- CORE_ADDR func_addr, limit_pc;
+ CORE_ADDR func_addr, limit_pc, end_pc;
struct symtab_and_line sal;
/* See if we can determine the end of the prologue via the symbol table.
If so, then return either PC, or the PC after the prologue, whichever
is greater. */
- if (find_pc_partial_function (pc, NULL, &func_addr, NULL))
+ if (find_pc_partial_function (pc, NULL, &func_addr, &end_pc))
{
CORE_ADDR post_prologue_pc
= skip_prologue_using_sal (gdbarch, func_addr);
@@ -1439,6 +1439,9 @@ arm_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR
pc)
if (limit_pc == 0)
limit_pc = pc + 64; /* Magic. */
+ /* Don't scan beyond the end of the current function. */
+ if (limit_pc > end_pc)
+ limit_pc = end_pc;
/* Check if this is Thumb code. */
if (arm_pc_is_thumb (gdbarch, pc))
diff --git gdb/testsuite/gdb.base/break-function.c
gdb/testsuite/gdb.base/break-function.c
new file mode 100644
index 0000000..e24cf91
--- /dev/null
+++ gdb/testsuite/gdb.base/break-function.c
@@ -0,0 +1,47 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 1992, 1993, 1994, 1995, 1999, 2002, 2003, 2007, 2008, 2009,
2010,
+ 2011 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+
+
+unsigned long _etext;
+unsigned long _data;
+unsigned long _edata;
+
+void foo(void)
+{
+ while(1)
+ {
+ }
+} /* End of function foo */
+
+void bar(void)
+{
+ unsigned long *pulSrc, *pulDest;
+
+ pulSrc = &_etext;
+ for(pulDest = &_data; pulDest < &_edata; )
+ {
+ *pulDest++ = *pulSrc++;
+ }
+}
+
+
+int main()
+{
+ bar();
+ foo();
+}
diff --git gdb/testsuite/gdb.base/break-function.exp
gdb/testsuite/gdb.base/break-function.exp
new file mode 100644
index 0000000..a42ba79
--- /dev/null
+++ gdb/testsuite/gdb.base/break-function.exp
@@ -0,0 +1,35 @@
+# Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
+# 2000, 2002, 2003, 2007, 2008, 2009, 2010, 2011
+# Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+set srcfile break-function.c
+
+if { [prepare_for_testing break-function.exp "break-function"
{break-function.c}] } {
+ return -1
+}
+
+set bp_location_boundary [gdb_get_line_number "End of function foo"]
+
+gdb_test_multiple "b foo" "Set breakpoint for function foo" {
+ -re "Breakpoint 1 at.*file.*, line (\[0-9\]+).*" {
+ set bp_pos $expect_out(1,string);
+ if { $bp_pos > $bp_location_boundary } {
+ fail "The location of function breakpoint exceeds the body of
the function.\n"
+ } else {
+ pass "PASS";
+ }
+ }
+}
--
1.7.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
2011-06-24 2:31 [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703) Terry Guo
@ 2011-06-24 3:55 ` Yao Qi
2011-06-24 8:59 ` Pedro Alves
0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2011-06-24 3:55 UTC (permalink / raw)
To: gdb-patches
On 06/24/2011 10:30 AM, Terry Guo wrote:
> Hello,
>
> This patch addresses the bug in gdb/12703 which sets two different function
> breakpoints at same pc address. In this patch I enhanced the way to analyze
> the ARM thumb prologue to prevent the function breakpoint from being set
> outside the function body.
>
> Patch has been tested against arm-none-eabi with no regressions. OK for
> commit?
>
> Thanks,
>
> Terry
>
I am not the people to approve or reject this patch. My $0.2 here.
> gdb/ChangLog:
> 2011-06-16 Terry Guo <terry.guo@arm.com>
>
> PR gdb/12703
> * arm-tdep.c (arm_skip_prologue): Don't scan beyond the end of
> the current function.
>
> gdb/testsuite/ChangLog:
> 2011-06-16 Terry Guo <terry.guo@arm.com>
>
> PR gdb/12703
> * gdb.base/break-function.c: New testcase.
> * gdb.base/break-function.exp: New script.
IMO, this is a target-specific bug, so this PR's component should be
tdept, so it should be "PR tdept/12703" instead of "PR gdb/12703".
I'd move your test cases break-function.{c,exp} to gdb.arch/ dir,
because it is target-dependent fix. I am sure this case is useful to
other ports.
> --- /dev/null
> +++ gdb/testsuite/gdb.base/break-function.c
> @@ -0,0 +1,47 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 1992, 1993, 1994, 1995, 1999, 2002, 2003, 2007, 2008, 2009,
> 2010,
> + 2011 Free Software Foundation, Inc.
> +
Your test case is a new one, so the year of copyright should be 2011.
Please remove the rest of them.
> +
> +
> +unsigned long _etext;
> +unsigned long _data;
> +unsigned long _edata;
> +
> +void foo(void)
This doesn't comply to GNU coding standard. Please move "foo ()" to
next line.
> +{
> + while(1)
^ need a space here.
> + {
> + }
> +} /* End of function foo */
Put a "." at the end of comment with two spaces, or one space without ".".
> +
> +void bar(void)
"bar" should be move to next newline. A space is needed ater "bar".
> +{
> + unsigned long *pulSrc, *pulDest;
> +
> + pulSrc = &_etext;
> + for(pulDest = &_data; pulDest < &_edata; )
> + {
> + *pulDest++ = *pulSrc++;
> + }
> +}
Indentation is not correct here.
> +# Copyright 1988, 1990, 1991, 1992, 1994, 1995, 1996, 1997, 1998, 1999,
> +# 2000, 2002, 2003, 2007, 2008, 2009, 2010, 2011
> +# Free Software Foundation, Inc.
> +
Only 2011 is needed here, because this case is created in 2011.
> +
> +set srcfile break-function.c
> +
> +if { [prepare_for_testing break-function.exp "break-function"
> {break-function.c}] } {
> + return -1
> +}
> +
> +set bp_location_boundary [gdb_get_line_number "End of function foo"]
> +
> +gdb_test_multiple "b foo" "Set breakpoint for function foo" {
> + -re "Breakpoint 1 at.*file.*, line (\[0-9\]+).*" {
When using gdb_test_multiple, "$gdb_prompt $" is needed at the end of
your regex. Please reference gdb_test_multiple used in other places.
> + set bp_pos $expect_out(1,string);
> + if { $bp_pos > $bp_location_boundary } {
> + fail "The location of function breakpoint exceeds the body of
> the function.\n"
> + } else {
> + pass "PASS";
The PASS message and FAIL message should be the same. I had the same
problem in my patch yesterday. :)
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
2011-06-24 3:55 ` Yao Qi
@ 2011-06-24 8:59 ` Pedro Alves
2011-06-24 10:39 ` Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Pedro Alves @ 2011-06-24 8:59 UTC (permalink / raw)
To: gdb-patches; +Cc: Yao Qi
On Friday 24 June 2011 04:55:06, Yao Qi wrote:
> On 06/24/2011 10:30 AM, Terry Guo wrote:
>
> IMO, this is a target-specific bug, so this PR's component should be
> tdept, so it should be "PR tdept/12703" instead of "PR gdb/12703".
>
> I'd move your test cases break-function.{c,exp} to gdb.arch/ dir,
> because it is target-dependent fix. I am sure this case is useful to
> other ports.
The testcase might help catch the same issue in other archs.
IMO, it should stay generic if possible.
I agree with Yao when he says in the PR that there seems to be
some other root cause for the bug. Shouldn't
thumb_instruction_changes_pc have caught that "b.n" ?
00008160 <fault_isr>:
8160: e7fe b.n 8160 <fault_isr>
...
00008164 <reset_isr>:
8164: 4a05 ldr r2, [pc, #20] ; (817c <reset_isr+0x18>)
> > +void foo(void)
>
> This doesn't comply to GNU coding standard. Please move "foo ()" to
> next line.
Note that test code does not strictly _need_ to follow the
coding standards. Though it's indeed nice when it does.
GDB should be able to debug non-GNU code too. :-)
--
Pedro Alves
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
2011-06-24 8:59 ` Pedro Alves
@ 2011-06-24 10:39 ` Yao Qi
2011-09-27 12:53 ` [ping]: " Yao Qi
2011-10-10 14:47 ` [PATCH] Fix that different function breakpoints are set@same " Ulrich Weigand
0 siblings, 2 replies; 10+ messages in thread
From: Yao Qi @ 2011-06-24 10:39 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: text/plain, Size: 2626 bytes --]
On 06/24/2011 04:59 PM, Pedro Alves wrote:
> On Friday 24 June 2011 04:55:06, Yao Qi wrote:
>> On 06/24/2011 10:30 AM, Terry Guo wrote:
>
>>
>> IMO, this is a target-specific bug, so this PR's component should be
>> tdept, so it should be "PR tdept/12703" instead of "PR gdb/12703".
>>
>> I'd move your test cases break-function.{c,exp} to gdb.arch/ dir,
>> because it is target-dependent fix. I am sure this case is useful to
>> other ports.
>
> The testcase might help catch the same issue in other archs.
> IMO, it should stay generic if possible.
>
OK. Let us leave it in gdb.base. I suggest that test case can be
renamed to reflect what we want to test here, such as
"break-outside-function.exp".
> I agree with Yao when he says in the PR that there seems to be
> some other root cause for the bug. Shouldn't
> thumb_instruction_changes_pc have caught that "b.n" ?
>
> 00008160 <fault_isr>:
> 8160: e7fe b.n 8160 <fault_isr>
> ...
>
> 00008164 <reset_isr>:
> 8164: 4a05 ldr r2, [pc, #20] ; (817c <reset_isr+0x18>)
>
thumb_instruction_changes_pc can handle "b.n". AFAICS, the problem is
in thumb_analyze_prologue. In thumb_analyze_prologue, there are a lot
if/else branches, like below,
else if ((insn & 0xe000) == 0xe000) // <-- [1]
{
....
else if (thumb2_instruction_changes_pc (insn, inst2))
{
/* Don't scan past anything that might change control flow. */
break;
}
else
{
/* The optimizer might shove anything into the prologue,
so we just skip what we don't recognize. */
unrecognized_pc = start;
}
start += 2;
}
else if (thumb_instruction_changes_pc (insn))
{
/* Don't scan past anything that might change control flow. */
break;
}
The instruction "b.n 8160" is 0xe7fe, so condition check [1] is true,
and thumb_instruction_changes_pc is unreachable. This is cause of this
problem, I doubt.
The line of code [1] is discussed in this patch
[rfa] ARM prologue parsing support for Thumb-2 instructions
http://sourceware.org/ml/gdb-patches/2010-10/msg00132.html
IIUC, condition check [1] is for 32-bit Thumb-2 instructions (I may be
wrong, of course). I have an untested patch.
>>> +void foo(void)
>>
>> This doesn't comply to GNU coding standard. Please move "foo ()" to
>> next line.
>
> Note that test code does not strictly _need_ to follow the
> coding standards. Though it's indeed nice when it does.
> GDB should be able to debug non-GNU code too. :-)
>
Oh, I don't know that. Sorry about the noise I made here.
--
Yao (é½å°§)
[-- Attachment #2: thumb_skip_prologue.patch --]
[-- Type: text/x-patch, Size: 647 bytes --]
gdb/
* arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit
Thumb-2 instructions.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 2dd8c9e..7f5a0e1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -832,8 +832,9 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
constant = read_memory_unsigned_integer (loc, 4, byte_order);
regs[bits (insn, 8, 10)] = pv_constant (constant);
}
- else if ((insn & 0xe000) == 0xe000)
+ else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
{
+ /* 32-bit Thumb-2 instructions. */
unsigned short inst2;
inst2 = read_memory_unsigned_integer (start + 2, 2,
^ permalink raw reply [flat|nested] 10+ messages in thread* [ping]: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
2011-06-24 10:39 ` Yao Qi
@ 2011-09-27 12:53 ` Yao Qi
2011-10-09 15:06 ` [ping 2]: " Yao Qi
2011-10-10 14:47 ` [PATCH] Fix that different function breakpoints are set@same " Ulrich Weigand
1 sibling, 1 reply; 10+ messages in thread
From: Yao Qi @ 2011-09-27 12:53 UTC (permalink / raw)
To: gdb-patches
On 06/24/2011 06:38 PM, Yao Qi wrote:
>> > I agree with Yao when he says in the PR that there seems to be
>> > some other root cause for the bug. Shouldn't
>> > thumb_instruction_changes_pc have caught that "b.n" ?
>> >
>> > 00008160 <fault_isr>:
>> > 8160: e7fe b.n 8160 <fault_isr>
>> > ...
>> >
>> > 00008164 <reset_isr>:
>> > 8164: 4a05 ldr r2, [pc, #20] ; (817c <reset_isr+0x18>)
>> >
> thumb_instruction_changes_pc can handle "b.n". AFAICS, the problem is
> in thumb_analyze_prologue. In thumb_analyze_prologue, there are a lot
> if/else branches, like below,
>
> else if ((insn & 0xe000) == 0xe000) // <-- [1]
> {
> ....
> else if (thumb2_instruction_changes_pc (insn, inst2))
> {
> /* Don't scan past anything that might change control flow. */
> break;
> }
> else
> {
> /* The optimizer might shove anything into the prologue,
> so we just skip what we don't recognize. */
> unrecognized_pc = start;
> }
>
> start += 2;
> }
> else if (thumb_instruction_changes_pc (insn))
> {
> /* Don't scan past anything that might change control flow. */
> break;
> }
>
> The instruction "b.n 8160" is 0xe7fe, so condition check [1] is true,
> and thumb_instruction_changes_pc is unreachable. This is cause of this
> problem, I doubt.
>
>
> The line of code [1] is discussed in this patch
>
> [rfa] ARM prologue parsing support for Thumb-2 instructions
> http://sourceware.org/ml/gdb-patches/2010-10/msg00132.html
>
> IIUC, condition check [1] is for 32-bit Thumb-2 instructions (I may be
> wrong, of course). I have an untested patch.
>
When talking with Terry Guo on HelloGCC workshop last Saturday in
Beijing, it reminds me that I still have a patch pending there, and
forget to ping it.
http://sourceware.org/ml/gdb-patches/2011-06/msg00370.html
I regression tested this patch on armv7l-unknown-linux-gnueabi with
{-mthumb, -marm}, no new fails. OK for mainline?
> gdb/
> * arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit
> Thumb-2 instructions.
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 2dd8c9e..7f5a0e1 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -832,8 +832,9 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
> constant = read_memory_unsigned_integer (loc, 4, byte_order);
> regs[bits (insn, 8, 10)] = pv_constant (constant);
> }
> - else if ((insn & 0xe000) == 0xe000)
> + else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
> {
> + /* 32-bit Thumb-2 instructions. */
> unsigned short inst2;
>
> inst2 = read_memory_unsigned_integer (start + 2, 2,
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread* [ping 2]: [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703)
2011-09-27 12:53 ` [ping]: " Yao Qi
@ 2011-10-09 15:06 ` Yao Qi
0 siblings, 0 replies; 10+ messages in thread
From: Yao Qi @ 2011-10-09 15:06 UTC (permalink / raw)
To: gdb-patches
On 09/27/2011 06:44 PM, Yao Qi wrote:
> http://sourceware.org/ml/gdb-patches/2011-06/msg00370.html
>
> I regression tested this patch on armv7l-unknown-linux-gnueabi with
> {-mthumb, -marm}, no new fails. OK for mainline?
>
Ping again.
>> > gdb/
>> > * arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit
>> > Thumb-2 instructions.
>> >
>> > diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
>> > index 2dd8c9e..7f5a0e1 100644
>> > --- a/gdb/arm-tdep.c
>> > +++ b/gdb/arm-tdep.c
>> > @@ -832,8 +832,9 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
>> > constant = read_memory_unsigned_integer (loc, 4, byte_order);
>> > regs[bits (insn, 8, 10)] = pv_constant (constant);
>> > }
>> > - else if ((insn & 0xe000) == 0xe000)
>> > + else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
>> > {
>> > + /* 32-bit Thumb-2 instructions. */
>> > unsigned short inst2;
>> >
>> > inst2 = read_memory_unsigned_integer (start + 2, 2,
--
Yao (é½å°§)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Fix that different function breakpoints are set@same pc address (PR gdb/12703)
2011-06-24 10:39 ` Yao Qi
2011-09-27 12:53 ` [ping]: " Yao Qi
@ 2011-10-10 14:47 ` Ulrich Weigand
2011-10-12 0:47 ` Yao Qi
1 sibling, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-10-10 14:47 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi wrote:
> gdb/
> * arm-tdep.c (thumb_analyze_prologue): Check condition for 32-bit
> Thumb-2 instructions.
Huh. That check is indeed wrong.
> - else if ((insn & 0xe000) == 0xe000)
> + else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
> {
Instead of open-coding the check, I think it would be preferable to
use the thumb_insn_size routine instead.
Note that there are a number of other places that either already open-
code the correct check, or -worse- use the same incorrect check as the
code above:
- thumb_in_function_epilogue_p
- thumb_get_next_pc_raw
- arm_breakpoint_from_pc
Would you mind converting them all to thumb_insn_size?
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix that different function breakpoints are set@same pc address (PR gdb/12703)
2011-10-10 14:47 ` [PATCH] Fix that different function breakpoints are set@same " Ulrich Weigand
@ 2011-10-12 0:47 ` Yao Qi
2011-10-12 11:58 ` Ulrich Weigand
0 siblings, 1 reply; 10+ messages in thread
From: Yao Qi @ 2011-10-12 0:47 UTC (permalink / raw)
To: Ulrich Weigand; +Cc: gdb-patches
On 10/10/2011 10:46 PM, Ulrich Weigand wrote:
>> > - else if ((insn & 0xe000) == 0xe000)
>> > + else if ((insn & 0xe000) == 0xe000 && (insn & 0x1800) != 0)
>> > {
> Instead of open-coding the check, I think it would be preferable to
> use the thumb_insn_size routine instead.
>
> Note that there are a number of other places that either already open-
> code the correct check, or -worse- use the same incorrect check as the
> code above:
>
> - thumb_in_function_epilogue_p
> - thumb_get_next_pc_raw
> - arm_breakpoint_from_pc
>
> Would you mind converting them all to thumb_insn_size?
Yeah, we should replace these checks around many places with
thumb_insn_size, I agree. Here is the patch for this purpose.
Regression tested on arm-linux-gnueabi with both -marm and -mthumb.
OK for mainline?
--
Yao (é½å°§)
PR gdb/12703
* arm-tdep.c (thumb_analyze_prologue): Call thumb_insn_size to check
whether insn is a 32-bit Thumb-2 instruction.
(thumb_in_function_epilogue_p): Likewise.
(thumb_get_next_pc_raw): Likewise.
(arm_breakpoint_from_pc): Likewise.
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 278e6e9..0db8b5f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -231,6 +231,8 @@ static void arm_neon_quad_write (struct gdbarch *gdbarch,
struct regcache *regcache,
int regnum, const gdb_byte *buf);
+static int thumb_insn_size (unsigned short inst1);
+
struct arm_prologue_cache
{
/* The stack pointer at the time this frame was created; i.e. the
@@ -836,7 +838,7 @@ thumb_analyze_prologue (struct gdbarch *gdbarch,
constant = read_memory_unsigned_integer (loc, 4, byte_order);
regs[bits (insn, 8, 10)] = pv_constant (constant);
}
- else if ((insn & 0xe000) == 0xe000)
+ else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instructions. */
{
unsigned short inst2;
@@ -3093,7 +3095,7 @@ thumb_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
if (insn & 0x0100) /* <registers> include PC. */
found_return = 1;
}
- else if ((insn & 0xe000) == 0xe000) /* 32-bit Thumb-2 instruction */
+ else if (thumb_insn_size (insn) == 4) /* 32-bit Thumb-2 instruction */
{
if (target_read_memory (scan_pc, buf, 2))
break;
@@ -4335,14 +4337,9 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc)
int cond = itstate >> 4;
if (! condition_true (cond, status))
- {
- /* Advance to the next instruction. All the 32-bit
- instructions share a common prefix. */
- if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
- return MAKE_THUMB_ADDR (pc + 4);
- else
- return MAKE_THUMB_ADDR (pc + 2);
- }
+ /* Advance to the next instruction. All the 32-bit
+ instructions share a common prefix. */
+ return MAKE_THUMB_ADDR (pc + thumb_insn_size (inst1));
/* Otherwise, handle the instruction normally. */
}
@@ -4376,7 +4373,7 @@ thumb_get_next_pc_raw (struct frame_info *frame, CORE_ADDR pc)
{
nextpc = pc_val + (sbits (inst1, 0, 10) << 1);
}
- else if ((inst1 & 0xe000) == 0xe000) /* 32-bit instruction */
+ else if (thumb_insn_size (inst1) == 4) /* 32-bit instruction */
{
unsigned short inst2;
inst2 = read_memory_unsigned_integer (pc + 2, 2, byte_order_for_code);
@@ -8473,7 +8470,7 @@ arm_breakpoint_from_pc (struct gdbarch *gdbarch, CORE_ADDR *pcptr, int *lenptr)
{
unsigned short inst1;
inst1 = extract_unsigned_integer (buf, 2, byte_order_for_code);
- if ((inst1 & 0xe000) == 0xe000 && (inst1 & 0x1800) != 0)
+ if (thumb_insn_size (inst1) == 4)
{
*lenptr = tdep->thumb2_breakpoint_size;
return tdep->thumb2_breakpoint;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Fix that different function breakpoints are set@same pc address (PR gdb/12703)
2011-10-12 0:47 ` Yao Qi
@ 2011-10-12 11:58 ` Ulrich Weigand
2011-10-13 8:19 ` [committed] : " Yao Qi
0 siblings, 1 reply; 10+ messages in thread
From: Ulrich Weigand @ 2011-10-12 11:58 UTC (permalink / raw)
To: Yao Qi; +Cc: gdb-patches
Yao Qi wrote:
> PR gdb/12703
> * arm-tdep.c (thumb_analyze_prologue): Call thumb_insn_size to check
> whether insn is a 32-bit Thumb-2 instruction.
> (thumb_in_function_epilogue_p): Likewise.
> (thumb_get_next_pc_raw): Likewise.
> (arm_breakpoint_from_pc): Likewise.
This is OK.
Thanks,
Ulrich
--
Dr. Ulrich Weigand
GNU Toolchain for Linux on System z and Cell BE
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-10-13 8:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-24 2:31 [PATCH] Fix that different function breakpoints are set at same pc address (PR gdb/12703) Terry Guo
2011-06-24 3:55 ` Yao Qi
2011-06-24 8:59 ` Pedro Alves
2011-06-24 10:39 ` Yao Qi
2011-09-27 12:53 ` [ping]: " Yao Qi
2011-10-09 15:06 ` [ping 2]: " Yao Qi
2011-10-10 14:47 ` [PATCH] Fix that different function breakpoints are set@same " Ulrich Weigand
2011-10-12 0:47 ` Yao Qi
2011-10-12 11:58 ` Ulrich Weigand
2011-10-13 8:19 ` [committed] : " Yao Qi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox