[RFA,3/3,V3] Testcase for pascal/17815
Commit Message
Hi Pedro,
> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : jeudi 22 janvier 2015 13:46
> À : Pierre Muller
> Cc : 'gdb-patches'
> Objet : Re: [RFA PATCH 3/3] Testcase for pascal/17815
>
> Thanks for adding the test case.
>
> On 01/09/2015 04:18 PM, Pierre Muller wrote:
> > This adds the testcase for pascal/17815 bug report.
> >
> >
> > gdb/testsuite/Changelog entry:
> >
> > 2015-01-09 Pierre Muller <muller@sourceware.org>
> >
> > gdb.pascal/gdb17815.pas: New file.
> > gdb.pascal/gdb17815.exp: New file.
>
> Missing '*'.
Fixed below.
> Can you please pick a more descriptive name for the file?
> One that suggests what's being tested, rather than a bug
> number is ideal. E.g., gdb.pascal/case-insensitive.{exp|pas}.
I used your suggestion, but this is not
really a general case insensitiveness test...
> > +load_lib "pascal.exp"
> > +
> > +standard_testfile .pas
> > +
> > +if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}"
> "${binfile}"
> > executable [list debug class]] != "" } {
> > + untested $testfile.exp
> > + return -1
> > +}
> > +
> > +clean_restart ${testfile}
> > +set bp_location [gdb_get_line_number "set breakpoint here"]
> > +
> > +if { [gdb_breakpoint ${srcfile}:${bp_location}] } {
> > + pass "setting breakpoint"
> > +}
> > +
> > +# Verify that "start" lands inside the right procedure.
> > +if { [gdb_start_cmd] < 0 } {
> > + untested start
> > + return -1
> > +}
> > +
> > +gdb_test "" ".* at .*${srcfile}.*" "start"
> > +gdb_test "continue" ""
>
> I won't object to this, given that it seems like all
> pascal tests do it, but I'll note that it's best to avoid
> gdb_start_cmd unless we're really testing the "start" command,
> as otherwise the test won't run against gdbserver
> (try --target_board=native-gdbserver). Doesn't runto_main
> work?
No, I tried and using runto_main,
I got only failures, because it apparently did not stop
at all on the breakpoint set earlier...
Is there some code inside runto_main that
deleted previously installed breakpoints?
Why is that so?
I kept gdb_start_cmd for now.
> > +# We are now inside CHECK method
>
> Could you add a period at end of sentences? Here and throughout.
Tried to do it below.
> > +gdb_test "p X" " = 67"
> > +gdb_test "p B.X" " = -1"
> > +# As A is global, we can also check it global value
> ^^
> Should be "its".
>
> Or, "check its value"? What's a _global_ value?
Comment adapted.
> > +gdb_test "p A.X" " = 67"
> > +gdb_test "p Y" " = 33"
> > +gdb_test "p Y" " = 33"
>
> Duplicate test here. Please try the recipe at:
> https://sourceware.org/gdb/wiki/GDBTestcaseCookbook#Make_sure_test_mess
> ages_are_unique
I used this, and got no duplicate in the new version below.
> Thanks,
Thank you for the review,
here is an updated RFA.
Pierre Muller
gdb/testsuite/ChangeLog entry:
2015-01-23 Pierre Muller <muller@sourceware.org>
PR pascal/17815
* gdb.pascal/case-insensitive.pas: New file.
* gdb.pascal/case-insensitive.exp: New file.
---
gdb/testsuite/gdb.pascal/case-insensitive.exp | 62
+++++++++++++++++++++++++++
gdb/testsuite/gdb.pascal/case-insensitive.pas | 61
++++++++++++++++++++++++++
2 files changed, 123 insertions(+)
create mode 100644 gdb/testsuite/gdb.pascal/case-insensitive.exp
create mode 100644 gdb/testsuite/gdb.pascal/case-insensitive.pas
Comments
On 01/23/2015 12:20 AM, Pierre Muller wrote:
> Hi Pedro,
>
>> -----Message d'origine-----
>> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] De la part de Pedro Alves
>> Envoyé : jeudi 22 janvier 2015 13:46
>> À : Pierre Muller
>> Cc : 'gdb-patches'
>> Objet : Re: [RFA PATCH 3/3] Testcase for pascal/17815
>>
>> Thanks for adding the test case.
>>
>> On 01/09/2015 04:18 PM, Pierre Muller wrote:
>>> This adds the testcase for pascal/17815 bug report.
>>>
>>>
>>> gdb/testsuite/Changelog entry:
>>>
>>> 2015-01-09 Pierre Muller <muller@sourceware.org>
>>>
>>> gdb.pascal/gdb17815.pas: New file.
>>> gdb.pascal/gdb17815.exp: New file.
>>
>> Missing '*'.
> Fixed below.
>
>> Can you please pick a more descriptive name for the file?
>> One that suggests what's being tested, rather than a bug
>> number is ideal. E.g., gdb.pascal/case-insensitive.{exp|pas}.
>
> I used your suggestion, but this is not
> really a general case insensitiveness test...
You mean, it's just for symbols, rather than filesystems, etc?
gdb.pascal/case-insensitive-symbols.{exp|pas} would be my
suggestion then. But these are just suggestions. Feel free to
pick something else.
>
> No, I tried and using runto_main,
> I got only failures, because it apparently did not stop
> at all on the breakpoint set earlier...
> Is there some code inside runto_main that
> deleted previously installed breakpoints?
Yes, it's the first thing runto_main -> runto does.
Why not just set the breakpoints after reaching main?
Or simpler still, use "runto $foo" directly, like:
set bp_location [gdb_get_line_number "set breakpoint here"]
if { ![runto ${srcfile}:${bp_location}] } {
return 0
}
Or simpler still, just:
if { ![runto "check"] } {
return 0
}
Assuming that to set a breakpoint on the "check" function,
you'd do "break check" in GDB. If not, replace "check" with
whatever linespec would be used instead.
The patch looks good to me.
Thanks,
Pedro Alves
b/gdb/testsuite/gdb.pascal/case-insensitive.exp
new file mode 100644
@@ -0,0 +1,62 @@
+# Copyright 2015 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/>.
+
+load_lib "pascal.exp"
+
+standard_testfile .pas
+
+if {[gdb_compile_pascal "${srcdir}/${subdir}/${srcfile}" "${binfile}"
executable [list debug class]] != "" } {
+ untested $testfile.exp
+ return -1
+}
+
+clean_restart ${testfile}
+set bp_location [gdb_get_line_number "set breakpoint here"]
+
+if { [gdb_breakpoint ${srcfile}:${bp_location}] } {
+ pass "setting breakpoint"
+}
+
+# Verify that "start" lands inside the right procedure.
+if { [gdb_start_cmd] < 0 } {
+ untested start
+ return -1
+}
+
+gdb_test "continue" ""
+# We are now inside CHECK method.
+gdb_test "p X" " = 67"
+gdb_test "p B.X" " = -1"
+# As A is global, we can also check its value.
+gdb_test "p A.X" " = 67"
+gdb_test "p Y" " = 33"
+gdb_test "p B.Y" " = -1"
+gdb_test "p A.Y" " = 33"
+# Now test lowercase version.
+gdb_test "p x" " = 67"
+gdb_test "p B.x" " = -1"
+# As A is global, we can also check its value, with lowercase.
+gdb_test "p A.x" " = 67"
+gdb_test "p y" " = 33"
+gdb_test "p B.y" " = -1"
+gdb_test "p A.y" " = 33"
+# Also test lowercase class names.
+gdb_test "p b.X" " = -1"
+gdb_test "p b.x" " = -1"
+gdb_test "p a.Y" " = 33"
+gdb_test "p a.y" " = 33"
+
+gdb_exit
+
b/gdb/testsuite/gdb.pascal/case-insensitive.pas
new file mode 100644
@@ -0,0 +1,61 @@
+{
+ Copyright 2015 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/>.
+}
+
+
+program test_gdb_17815;
+
+
+type
+ TA = class
+ public
+ x, y : integer;
+ constructor Create;
+ function check(b : TA) : boolean;
+ destructor Done; virtual;
+end;
+
+constructor TA.Create;
+begin
+ x:=-1;
+ y:=-1;
+end;
+
+destructor TA.Done;
+begin
+end;
+
+function TA.check (b : TA) : boolean;
+begin
+ check:=(x < b.x); { set breakpoint here }
+end;
+
+
+
+var
+ a, b : TA;
+
+begin
+ a:=TA.Create;
+ b:=TA.Create;
+ a.x := 67;
+ a.y := 33;
+ if a.check (b) then
+ writeln('Error in check')
+ else
+ writeln('check OK');
+end.
+