[PATCH] Fix PR 17206

[PATCH] Fix PR 17206

[Yao Qi]

As reported in PR 17206, an internal error is triggered when command
until is executed.  In infcmd.c:until_next_command, step_range_end is
set to 'pc',

  if (!func)
    {
      struct bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (pc);

      if (msymbol.minsym == NULL)
	error (_("Execution is not within a known function."));

      tp->control.step_range_start = BMSYMBOL_VALUE_ADDRESS (msymbol);
      tp->control.step_range_end = pc;
    }

and later in infrun.c:resume, the assert below is triggered in PR
17206.

  if (tp->control.may_range_step)
    {
      /* If we're resuming a thread with the PC out of the step
	 range, then we're doing some nested/finer run control
	 operation, like stepping the thread out of the dynamic
	 linker or the displaced stepping scratch pad.  We
	 shouldn't have allowed a range step then.  */
      gdb_assert (pc_in_thread_step_range (pc, tp));
    }

In until_next_command, we set step range to [XXX, pc), so pc isn't
within the range.  pc_in_thread_step_range returns false and the
assert is triggered.  AFAICS, the range we want in until_next_command
is [XXX, pc] instead of [XXX, pc), because we want to program step
until greater than pc.  This patch is to set step_range_end to
'pc + 1'.

Regression tested on x86_64-linux, both native and gdbserver.

gdb:

2014-07-28  Yao Qi  <yao@codesourcery.com>

	PR gdb/17206
	* infcmd.c (until_next_command): Set step_range_end to PC + 1.
---
 gdb/infcmd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 76ec560..50cc04b 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1359,7 +1359,9 @@ until_next_command (int from_tty)
 	error (_("Execution is not within a known function."));
 
       tp->control.step_range_start = BMSYMBOL_VALUE_ADDRESS (msymbol);
-      tp->control.step_range_end = pc;
+      /* The upper-bound of step_range is exclusive.  In order to make PC
+	 within the range, set the step_range_end with PC + 1.  */
+      tp->control.step_range_end = pc + 1;
     }
   else
     {
-- 
1.9.0

Re: [PATCH] Fix PR 17206

[Pedro Alves]

Thanks Yao,

On 07/28/2014 12:30 PM, Yao Qi wrote:

> 2014-07-28  Yao Qi  <yao@codesourcery.com>
> 
> 	PR gdb/17206
> 	* infcmd.c (until_next_command): Set step_range_end to PC + 1.

This looks right, but, could we add a test to the test suite?

Thanks,
Pedro Alves

Re: [PATCH] Fix PR 17206

[Yao Qi]

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.

-- 
Yao (齐尧)

gdb/testsuite:

2014-07-28  Yao Qi  <yao@codesourcery.com>

	PR gdb/17206
	* gdb.base/until-range-step.exp: New.
---
 gdb/testsuite/gdb.base/until-range-step.exp | 37 +++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/until-range-step.exp

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"
-- 
1.9.0

Re: [PATCH] Fix PR 17206

[Pedro Alves]

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

Re: [PATCH] Fix PR 17206

[Yao Qi]

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

-- 
Yao (齐尧)