From: "Joseph S. Myers" <joseph@codesourcery.com>
To: Hans-Peter Nilsson <hp@bitrange.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Remove code handling old ARM aliases from GDB
Date: Thu, 05 May 2011 22:34:00 -0000 [thread overview]
Message-ID: <Pine.LNX.4.64.1105052221050.20285@digraph.polyomino.org.uk> (raw)
In-Reply-To: <alpine.BSF.2.00.1105051748440.67473@dair.pair.com>
On Thu, 5 May 2011, Hans-Peter Nilsson wrote:
> On Thu, 5 May 2011, Joseph S. Myers wrote:
> > On Thu, 5 May 2011, Hans-Peter Nilsson wrote:
> >
> > > > -if { [istarget xscale*-*-*] } {
> > > > +if { [istarget arm*-*-*] } {
> >
> > > How did you test these changes?
> >
> > I considered them sufficiently obviously syntactically correct not to need
> > testing.
>
> It might have seemed like that at the time, but obviously it
> wasn't so.
Well, it was *syntactically correct*, as shown by the tests running in
your log rather than hitting a Tcl syntax error.
> > I don't call that breaking testing; I call that exposing bugs (whether in
> > the simulator or testsuite) that were previously hidden by the use of an
> > obsolete target triplet name.
>
> arm-elf passed before and does not anymore after those changes.
"arm-elf" is not a testcase.
A testsuite regression is when a test assertion fails before and passes
after a patch. A test assertion corresponds to particular text that may
appear after "PASS: " or "FAIL: " in the .sum file (for a properly
functioning DejaGnu testsuite, a slightly looser definition may be
appropriate for GDB at present). A failure of a test that was not run
before the patch is not a regression.
> That's breaking testing.
No, breaking testing would be introducing a syntax error so that tests
that previously run stop running or run incorrectly. Output with FAILs in
it isn't broken, it simply gives information about what works and what
doesn't. If a FAIL replaced a PASS that would be a regression, not
breaking testing, but that isn't the case here either.
> Any other definition falls prey to
> "latent bugs" weaseling.
The simulator bugs exposed weren't latent since the simulator probably had
those bugs for years. So far as there were testsuite bugs (the tests not
running) those were *fixed* by the patch.
I consider the situation here to be exactly the same as if I'd added
target-independent tests to the GDB testsuite for a feature that should
work on all targets but that would require target-specific work for most
targets to make it work properly. Such tests would, of course, expose
bugs in various targets, but it would be up to those interested in
improving results for each target to fix them there; I don't think there's
any expectation that such new tests will pass everywhere. (An example of
potential new tests that would have such an issue would be tests for how
GDB handles argument passing and return involving complex numbers; that
needs ABI support for each target, and very few GDB targets have such ABI
support that actually corresponds to what GCC does.) I think enabling
tests that were wrongly disabled is just like adding new tests - if
someone fixing them is unclear about the semantics of instructions being
tested, I'll try to advise on specific questions, just as I would on
questions about the hypothetical tests for complex number ABIs.
--
Joseph S. Myers
joseph@codesourcery.com
next prev parent reply other threads:[~2011-05-05 22:34 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-04 19:01 Joseph S. Myers
2011-05-04 19:21 ` Mark Kettenis
2011-05-04 19:23 ` Pedro Alves
2011-05-04 19:24 ` Pedro Alves
2011-05-05 13:21 ` Hans-Peter Nilsson
2011-05-05 13:47 ` Hans-Peter Nilsson
2011-05-05 14:34 ` Joseph S. Myers
2011-05-05 22:11 ` Hans-Peter Nilsson
2011-05-05 22:34 ` Joseph S. Myers [this message]
2011-05-05 23:06 ` Hans-Peter Nilsson
2011-05-05 23:19 ` Joseph S. Myers
2011-05-05 23:52 ` Joseph S. Myers
2011-05-06 0:23 ` Joel Brobecker
2011-05-06 3:00 ` Hans-Peter Nilsson
[not found] ` <Pine.LNX.4.64.1105052349190.20285@digraph.polyomino.org.uk>
2011-05-09 4:53 ` Hans-Peter Nilsson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.1105052221050.20285@digraph.polyomino.org.uk \
--to=joseph@codesourcery.com \
--cc=gdb-patches@sourceware.org \
--cc=hp@bitrange.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox