[v3] Fix for PR gdb/17017

Message ID CAGyQ6gxax1ZgbLqLvZjyc--SPPNJ1Y6F-LH6fCTv2eY4kFs-Cg@mail.gmail.com
State New, archived
Headers

Commit Message

Siva Chandra Reddy June 12, 2014, 6:23 p.m. UTC
  The attached patch fixes PR 17017 along the lines suggested by Pedro
here: https://sourceware.org/ml/gdb-patches/2014-06/msg00212.html

It does not use get_integer_valueof as suggested by Pedro as he
pointed out in his review of v2 that we will then have to enclose some
of the tests in with_test_prefix to make the names unique:
https://sourceware.org/ml/gdb-patches/2014-06/msg00308.html

What I have done in v3 is to use the idea of testing C++ method
invocation by testing the value of a global var from Pedro's first
suggestion, but tested the value of the global var using gdb_test
instead of get_integer_valueof.

ChangeLog

testsuite/
2014-06-12  Siva Chandra Reddy  <sivachandra@google.com>

        PR gdb/17017
        * gdb.python/py-xmethods.cc: Add global function call counters and
        increment them in their respective functions.  Remove "cout"
        statements.
        * gdb.python/py-xmethods.exp: Make tests check the global function
        call counters instead of depending on inferior IO.
  

Comments

Pedro Alves June 16, 2014, 2:12 p.m. UTC | #1
On 06/12/2014 07:23 PM, Siva Chandra wrote:

> testsuite/
> 2014-06-12  Siva Chandra Reddy  <sivachandra@google.com>
> 
>         PR gdb/17017
>         * gdb.python/py-xmethods.cc: Add global function call counters and
>         increment them in their respective functions.  Remove "cout"
>         statements.
>         * gdb.python/py-xmethods.exp: Make tests check the global function
>         call counters instead of depending on inferior IO.
> 

Nice, I like this much better.

This is OK.

Thanks,
  
Siva Chandra Reddy June 18, 2014, 11:40 a.m. UTC | #2
On Mon, Jun 16, 2014 at 7:12 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/12/2014 07:23 PM, Siva Chandra wrote:
>
>> testsuite/
>> 2014-06-12  Siva Chandra Reddy  <sivachandra@google.com>
>>
>>         PR gdb/17017
>>         * gdb.python/py-xmethods.cc: Add global function call counters and
>>         increment them in their respective functions.  Remove "cout"
>>         statements.
>>         * gdb.python/py-xmethods.exp: Make tests check the global function
>>         call counters instead of depending on inferior IO.
>>
>
> Nice, I like this much better.
>
> This is OK.

Pushed: 5d376983ca914e1cf36f4968cc87957f9033ebcd

Thank you,
Siva Chandra
  
Jan Kratochvil July 14, 2014, 12:10 p.m. UTC | #3
On Wed, 18 Jun 2014 13:40:49 +0200, Siva Chandra wrote:
> On Mon, Jun 16, 2014 at 7:12 AM, Pedro Alves <palves@redhat.com> wrote:
> > On 06/12/2014 07:23 PM, Siva Chandra wrote:
> >> testsuite/
> >> 2014-06-12  Siva Chandra Reddy  <sivachandra@google.com>
> >>
> >>         PR gdb/17017
> >>         * gdb.python/py-xmethods.cc: Add global function call counters and
> >>         increment them in their respective functions.  Remove "cout"
> >>         statements.
> >>         * gdb.python/py-xmethods.exp: Make tests check the global function
> >>         call counters instead of depending on inferior IO.
[...]
> Pushed: 5d376983ca914e1cf36f4968cc87957f9033ebcd

5d376983ca914e1cf36f4968cc87957f9033ebcd is the first bad commit
commit 5d376983ca914e1cf36f4968cc87957f9033ebcd
Author: Siva Chandra <sivachandra@chromium.org>
Date:   Thu Jun 5 07:03:56 2014 -0700
    Make xmethods tests not to depend on inferior IO.

on Fedora 20 x86_64 (python-2.7.5-13.fc20.x86_64):
	p g.size_diff<  unsigned long  >()^M
	From Python G<>::size_diff()^M
	$35 = 4^M
	(gdb) PASS: gdb.python/py-xmethods.exp: After: g.size_diff<unsigned long>()

on Fedora Rawhide x86_64 (python-2.7.7-2.fc21.x86_64):
	p g.size_diff<  unsigned long  >()^M
	Couldn't find method dop::G<int>::size_diff<  unsigned long  >^M
	(gdb) FAIL: gdb.python/py-xmethods.exp: After: g.size_diff<unsigned long>()

I do not know what it depends on.  When I replace the binaries py-xmethods the
regression remains the same so it does not depend on GCC version (4.8 vs. 4.9).

Maybe it depends on Python version?

Before this commit 5d376983ca914e1cf36f4968cc87957f9033ebcd
both Fedora 20 x86_64 and Fedora Rawhide x86_64 were PASSing everything.

Some other logs suggest it happens on all x86_64, x86_64-m32 and i686 but
I cannot guarantee it right now.


Jan
  
Jan Kratochvil July 14, 2014, 12:32 p.m. UTC | #4
On Mon, 14 Jul 2014 14:10:12 +0200, Jan Kratochvil wrote:
> I do not know what it depends on.  When I replace the binaries py-xmethods the
> regression remains the same so it does not depend on GCC version (4.8 vs. 4.9).
> 
> Maybe it depends on Python version?

It apperently FAILs if debuginfo rpm does not match base package rpm.
After I got debuginfo rpms in sync it PASSes now.

This is obvious in the cases where GDB complaints about non-matching CRC but
here GDB did not complain with such warning.

This is still IMO a GDB testsuite bug but no longer specific to this testcase.
Sorry for the false alarm.


Jan
  

Patch

diff --git a/gdb/testsuite/gdb.python/py-xmethods.cc b/gdb/testsuite/gdb.python/py-xmethods.cc
index 96637d8..96389fd 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.cc
+++ b/gdb/testsuite/gdb.python/py-xmethods.cc
@@ -15,10 +15,6 @@ 
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see  <http://www.gnu.org/licenses/>.  */
 
-#include <iostream>
-
-using namespace std;
-
 namespace dop
 {
 
@@ -35,24 +31,30 @@  public:
 
 A::~A () { }
 
+int a_plus_a = 0;
+
 int
 A::operator+ (const A &obj)
 {
-  cout << "From CC <A_plus_A>:" << endl;
+  a_plus_a++;
   return a + obj.a;
 }
 
+int a_minus_a = 0;
+
 int
 A::operator- (const A &obj)
 {
-  cout << "From CC <A_minus_A>:" << endl;
+  a_minus_a++;
   return a - obj.a;
 }
 
+int a_geta = 0;
+
 int
 A::geta (void)
 {
-  cout << "From CC A::geta:" << endl;
+  a_geta++;
   return a;
 }
 
@@ -62,10 +64,12 @@  public:
   virtual int geta (void);
 };
 
+int b_geta = 0;
+
 int
 B::geta (void)
 {
-  cout << "From CC B::geta:" << endl;
+  b_geta++;
   return 2 * a;
 }
 
@@ -100,30 +104,36 @@  class G
   T t;
 };
 
+int g_size_diff = 0;
+
 template <typename T>
 template <typename T1>
 int
 G<T>::size_diff ()
 {
-  cout << "From CC G<>::size_diff:" << endl;
+  g_size_diff++;
   return sizeof (T1) - sizeof (T);
 }
 
+int g_size_mul = 0;
+
 template <typename T>
 template <int M>
 int
 G<T>::size_mul ()
 {
-  cout << "From CC G<>::size_mul:" << endl;
+  g_size_mul++;
   return M * sizeof (T);
 }
 
+int g_mul = 0;
+
 template <typename T>
 template <typename T1>
 T
 G<T>::mul (const T1 t1)
 {
-  cout << "From CC G<>::mul:" << endl;
+  g_mul++;
   return t1 * t;
 }
 
diff --git a/gdb/testsuite/gdb.python/py-xmethods.exp b/gdb/testsuite/gdb.python/py-xmethods.exp
index 97b6ffa..a455a7a 100644
--- a/gdb/testsuite/gdb.python/py-xmethods.exp
+++ b/gdb/testsuite/gdb.python/py-xmethods.exp
@@ -40,25 +40,45 @@  gdb_breakpoint [gdb_get_line_number "Break here."]
 gdb_continue_to_breakpoint "Break here" ".*Break here.*"
 
 # Tests before loading the debug methods.
-gdb_test "p a1 + a2" "From CC <A_plus_A>.*15" "Before: a1 + a2"
-gdb_test "p a2 - a1" "From CC <A_minus_A>.*5" "Before: a1 - a2"
-gdb_test "p b1 - a1" "From CC <A_minus_A>.*25" "Before: b1 - a1"
-gdb_test "p a1.geta()" "From CC A::geta.*5" "Before: a1.geta()"
+gdb_test "p a1 + a2" ".* = 15" "Before: a1 + a2"
+gdb_test "p a_plus_a" ".* = 1" "Before: a_plus_a 1"
+
+gdb_test "p a2 - a1" ".* = 5" "Before: a2 - a1"
+gdb_test "p a_minus_a" ".* = 1" "Before: a_minus_a 1"
+
+gdb_test "p b1 - a1" ".* = 25" "Before: b1 - a1"
+gdb_test "p a_minus_a" ".* = 2" "Before: a_minus_a 2"
+
+gdb_test "p a1.geta()" ".* = 5" "Before: a1.geta()"
+gdb_test "p a_geta" ".* = 1" "Before: a_geta 1"
+
 gdb_test "p ++a1" "No symbol.*" "Before: ++a1"
 gdb_test "p a1.getarrayind(5)" "Couldn't find method.*" \
   "Before: a1.getarrayind(5)"
-gdb_test "p a_ptr->geta()" "From CC B::geta.*60" "Before: a_ptr->geta()"
-gdb_test "p e.geta()" "From CC A::geta.*100" "Before: e.geta()"
-gdb_test "p g.size_diff<float>()" "From CC G<>::size_diff.*" \
-  "Before: g.size_diff<float>()"
+
+gdb_test "p a_ptr->geta()" ".* = 60" "Before: a_ptr->geta()"
+gdb_test "p b_geta" ".* = 1" "Before: b_geta 1"
+
+gdb_test "p e.geta()" ".* = 100" "Before: e.geta()"
+gdb_test "p a_geta" ".* = 2" "Before: a_geta 2"
+
+# Since g.size_diff operates of sizes of int and float, do not check for
+# actual result value as it could be different on different platforms.
+gdb_test "p g.size_diff<float>()" ".*" "Before: call g.size_diff<float>()"
+gdb_test "p g_size_diff" ".* = 2" "Before: g_size_diff 2"
+
 gdb_test "p g.size_diff<unsigned long>()" "Couldn't find method.*" \
   "Before: g.size_diff<unsigned long>()"
-gdb_test "p g.size_mul<2>()" "From CC G<>::size_mul.*" \
-  "Before: g.size_mul<2>()"
+
+gdb_test "p g.size_mul<2>()" ".*" "Before: g.size_mul<2>()"
+gdb_test "p g_size_mul" ".* = 2" "Before: g_size_mul 2"
+
 gdb_test "p g.size_mul<5>()" "Couldn't find method.*" \
   "Before: g.size_mul<5>()"
-gdb_test "p g.mul<double>(2.0)" "From CC G<>::mul.*" \
-  "Before: g.mul<double>(2.0)"
+
+gdb_test "p g.mul<double>(2.0)" ".* = 10" "Before: g.mul<double>(2.0)"
+gdb_test "p g_mul" ".* = 2" "Before: g_mul 2"
+
 gdb_test "p g.mul<char>('a')" "Couldn't find method.*" \
   "Before: g.mul<char>('a')"
 
@@ -67,9 +87,15 @@  gdb_test_no_output "source ${xmethods_script}" "load the script file"
 
 # Tests after loading debug methods.
 gdb_test "p a1 + a2" "From Python <A_plus_A>.*15" "After: a1 + a2"
-gdb_test "p a2 - a1" "From CC <A_minus_A>.*5" "After: a1 - a2"
+
+gdb_test "p a2 - a1" ".* = 5" "After: a2 - a1"
+gdb_test "p a_minus_a" ".* = 3" "After: a_minus_a 3"
+
 gdb_test "p b1 + a1" "From Python <A_plus_A>.*35" "After: b1 + a1"
-gdb_test "p b1 - a1" "From CC <A_minus_A>.*25" "After: b1 - a1"
+
+gdb_test "p b1 - a1" ".* = 25" "After: b1 - a1"
+gdb_test "p a_minus_a" ".* = 4" "After: a_minus_a 4"
+
 gdb_test "p a1.geta()" "From Python <A_geta>.*5" "After: a1.geta()"
 gdb_test "p ++a1" "From Python <plus_plus_A>.*6" "After: ++a1"
 gdb_test "p a1.getarrayind(5)" "From Python <A_getarrayind>.*5" \