[PATCH] Fix gdb.ada/watch_arg.exp test with always inserted and displaced stepping on

[PATCH] Fix gdb.ada/watch_arg.exp test with always inserted and displaced stepping on

[Pedro Alves]

Doing a testsuite run with both always inserted and displaced stepping
on, shows a regression in gdb.ada/watch_arg.exp (compared to pristine
gdb):

 (gdb) break watch.adb:21
 Breakpoint 1 at 0x401157: file /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb, line 21.
 (gdb) run
 Starting program: /home/pedro/gdb/mygit/build/gdb/testsuite/gdb.ada/watch_arg/watch

 Breakpoint 1, watch.foo (x=1) at /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb:21
 21	      X := 3;  -- BREAK1
 (gdb) watch x
 Hardware watchpoint 2: x
 (gdb) PASS: gdb.ada/watch_arg.exp: Set watchpoint on function argument X
 break watch.adb:28
 Breakpoint 3 at 0x40117f: file /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb, line 28.
 (gdb) PASS: gdb.ada/watch_arg.exp: insert second breakpoint in watch.adb
 cont
 Continuing.
 Hardware watchpoint 2: x

 Old value = 1
 New value = 3
 watch.foo (x=3) at /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb:22
 22	   end Foo;
 (gdb) FAIL: gdb.ada/watch_arg.exp: Continuing to second breakpoint

vs:

 (gdb) watch x
 Hardware watchpoint 2: x
 (gdb) PASS: gdb.ada/watch_arg.exp: Set watchpoint on function argument X
 break watch.adb:28
 Breakpoint 3 at 0x40117f: file /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb, line 28.
 (gdb) PASS: gdb.ada/watch_arg.exp: insert second breakpoint in watch.adb
 cont
 Continuing.

 Breakpoint 3, watch () at /home/pedro/gdb/mygit/gdb/gdb/testsuite/gdb.ada/watch_arg/watch.adb:28
 28         X := 2;  -- BREAK2
 (gdb) PASS: gdb.ada/watch_arg.exp: Continuing to second breakpoint


The "problem" is that setting both always inserted and displaced
stepping on, actually fixes PR7143.  That is, the test sets a
watchpoint on X, and assumes that it does not trigger.  But, it
actually should trigger.  The instruction under breakpoint 1 does
change X.  A pristine GDB doesn't trigger the watchpoint because we
blindly remove all breakpoints along with all watchpoints when
stepping over breakpoint 1, thus missing the watchpoint.

Looking at the original description of the problem:

 http://www.sourceware.org/ml/gdb-patches/2006-10/msg00044.html

I notice that what the Foo procedure does is not really important,
so an easy fix is to make Foo not touch X, thus we have the same
behavior even if PR7143 is fixed.

Okay?

2011-12-05  Pedro Alves  <pedro@codesourcery.com>

	testsuite/
	* gdb.ada/watch_arg/watch.adb (Watch.Foo): New Y parameter.
	(Watch): New Y local.  Pass it to Foo.
---
 gdb/testsuite/gdb.ada/watch_arg/watch.adb |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.ada/watch_arg/watch.adb b/gdb/testsuite/gdb.ada/watch_arg/watch.adb
index d10e63d..deb54e3 100644
--- a/gdb/testsuite/gdb.ada/watch_arg/watch.adb
+++ b/gdb/testsuite/gdb.ada/watch_arg/watch.adb
@@ -16,15 +16,16 @@
 
 procedure Watch is
 
-   procedure Foo (X : in out Integer) is
+   procedure Foo (X, Y : in out Integer) is
    begin
-      X := 3;  -- BREAK1
+      Y := 3;  -- BREAK1
    end Foo;
 
    X : Integer := 1;
+   Y : Integer := 1;
 
 begin
-   Foo (X);
+   Foo (X, Y);
    X := 2;  -- BREAK2
 end Watch;
 

Re: [PATCH] Fix gdb.ada/watch_arg.exp test with always inserted and displaced stepping on

[Joel Brobecker]

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

> The "problem" is that setting both always inserted and displaced
> stepping on, actually fixes PR7143.  That is, the test sets a
> watchpoint on X, and assumes that it does not trigger.  But, it
> actually should trigger.

Sometimes, I'm not sure what, if anything, goes through my brain...
You're right, of course.

> I notice that what the Foo procedure does is not really important,
> so an easy fix is to make Foo not touch X, thus we have the same
> behavior even if PR7143 is fixed.

I would rather make sure that X still gets referenced, to make sure
a smart tool/compiler/linker does not play games. Can you try the
attached?

-- 
Joel

[-- Attachment #2: watch.diff --]
[-- Type: text/x-diff, Size: 3446 bytes --]

commit 8a01a822c37a69ace62b15d733d61500eb4752fe
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Tue Dec 6 11:29:39 2011 +0100

    fix incorrect assumption in gdb.ada/watch_arg
    
    The testcase is assuming that the parameter being watched isn't being
    modified. But the way the test program is written, this is not true
    at all.  So this changes fixes the code to still reference the variable,
    but in a way that does not modify its value.
    
    gdb/testsuite:
    
            * gdb.ada/watch_arg/pck.ads, gdb.ada/watch_arg/pck.adb: New files.
            * gdb.ada/watch_arg/watch.adb: Adjust code to avoid modification
            of parameter X in procedure Foo.

diff --git a/gdb/testsuite/gdb.ada/watch_arg/pck.adb b/gdb/testsuite/gdb.ada/watch_arg/pck.adb
new file mode 100644
index 0000000..a9c836b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/watch_arg/pck.adb
@@ -0,0 +1,23 @@
+--  Copyright 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/>.
+
+package body Pck is
+
+   procedure Do_Nothing (A : System.Address) is
+   begin
+      null;
+   end Do_Nothing;
+
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/watch_arg/pck.ads b/gdb/testsuite/gdb.ada/watch_arg/pck.ads
new file mode 100644
index 0000000..f8a4ed5
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/watch_arg/pck.ads
@@ -0,0 +1,19 @@
+--  Copyright 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/>.
+
+with System;
+package Pck is
+   procedure Do_Nothing (A : System.Address);
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/watch_arg/watch.adb b/gdb/testsuite/gdb.ada/watch_arg/watch.adb
index d10e63d..d5e2c33 100644
--- a/gdb/testsuite/gdb.ada/watch_arg/watch.adb
+++ b/gdb/testsuite/gdb.ada/watch_arg/watch.adb
@@ -14,11 +14,14 @@
 --  You should have received a copy of the GNU General Public License
 --  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+with Pck; use Pck;
+
 procedure Watch is
 
    procedure Foo (X : in out Integer) is
    begin
-      X := 3;  -- BREAK1
+      --  Reference X in a way that does not change its value.
+      Do_Nothing (X'Address);  -- BREAK1
    end Foo;
 
    X : Integer := 1;

Re: [PATCH] Fix gdb.ada/watch_arg.exp test with always inserted and displaced stepping on

[Pedro Alves]

On Tuesday 06 December 2011 10:34:17, Joel Brobecker wrote:

> I would rather make sure that X still gets referenced, to make sure
> a smart tool/compiler/linker does not play games. Can you try the
> attached?

Yep, works fine.  Thanks!

-- 
Pedro Alves

Re: [PATCH] Fix gdb.ada/watch_arg.exp test with always inserted and displaced stepping on

[Joel Brobecker]

> > I would rather make sure that X still gets referenced, to make sure
> > a smart tool/compiler/linker does not play games. Can you try the
> > attached?
> 
> Yep, works fine.  Thanks!

Awesome, thanks! I've just checked the patch in. 

-- 
Joel