Fix PR 17206

Message ID 53D6511E.3040404@codesourcery.com
State Committed
Headers

Commit Message

Yao Qi July 28, 2014, 1:33 p.m. UTC
  On 07/28/2014 08:37 PM, Pedro Alves wrote:
> This looks right, but, could we add a test to the test suite?

Sure, how about the test below?  Without the fix, I get the fail

FAIL: gdb.base/until-range-step.exp: until 2 (GDB internal error)

on x86-linux and arm-none-eabi.  With the fix applied, the fail goes
away.  I am not sure the test case name is good or clear enough.
Maybe we can rename it to pr17206.exp or something else.
  

Comments

Pedro Alves July 28, 2014, 2:26 p.m. UTC | #1
On 07/28/2014 02:33 PM, Yao Qi wrote:
> On 07/28/2014 08:37 PM, Pedro Alves wrote:
>> This looks right, but, could we add a test to the test suite?
> 
> Sure, how about the test below?  Without the fix, I get the fail
> 
> FAIL: gdb.base/until-range-step.exp: until 2 (GDB internal error)
> 
> on x86-linux and arm-none-eabi.  With the fix applied, the fail goes
> away.  I am not sure the test case name is good or clear enough.

From a high-level perspective, the issue triggered when you did
"until" and PC pointed somewhere we had no debug info for, and there
was no breakpoint at PC that needed to be stepped over.  That is
main use case and code path that we didn't have a test for.

Note that although it happened to be a range-stepping-related assertion
that triggered, the code was wrong even without range-stepping.  E.g.,
if the instruction at PC is a conditional jmp to PC (like a spinlock),
even without range-stepping, "until" should continue stepping until
PC moves past the jump (that's the whole point of until), while
it was stopping after one single-step, thus still pointing at the
same PC.

So I think "until-nodebug.exp" would be a good name for this test.

> Maybe we can rename it to pr17206.exp or something else.

We avoid such numeric names that make it harder to tell what's
being exercised.

The test itself looks good.  Thanks!

Pedro Alves
  
Yao Qi July 29, 2014, 6:36 a.m. UTC | #2
On 07/28/2014 10:26 PM, Pedro Alves wrote:
> From a high-level perspective, the issue triggered when you did
> "until" and PC pointed somewhere we had no debug info for, and there
> was no breakpoint at PC that needed to be stepped over.  That is
> main use case and code path that we didn't have a test for.
> 
> Note that although it happened to be a range-stepping-related assertion
> that triggered, the code was wrong even without range-stepping.  E.g.,
> if the instruction at PC is a conditional jmp to PC (like a spinlock),
> even without range-stepping, "until" should continue stepping until
> PC moves past the jump (that's the whole point of until), while
> it was stopping after one single-step, thus still pointing at the
> same PC.
> 
> So I think "until-nodebug.exp" would be a good name for this test.

That is OK to me.  This bug is on 7.8 branch too.  The patch is pushed
to mainline and 7.8 branch.  I've added it to
https://sourceware.org/gdb/wiki/GDB_7.8_Release
  

Patch

diff --git a/gdb/testsuite/gdb.base/until-range-step.exp b/gdb/testsuite/gdb.base/until-range-step.exp
new file mode 100644
index 0000000..a7e75e2
--- /dev/null
+++ b/gdb/testsuite/gdb.base/until-range-step.exp
@@ -0,0 +1,37 @@ 
+# Copyright 2014 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/>.  */
+
+# Test that the address range for stepping is correctly set in command
+# until when there is no debug information.
+
+standard_testfile advance.c
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile nodebug]} {
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main"
+    return 0
+}
+
+# Without debug information, the program stops at the next
+# instruction, which is still in main.
+gdb_test "until" "in main .*" "until 1"
+
+# If the stepping range is correctly set, the program stops at the next
+# instruction.  Otherwise, an internal error will be triggered.  See
+# PR gdb/17206.
+gdb_test "until" "in main .*" "until 2"