This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v4 00/18] All-stop on top of non-stop
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Luis Machado <lgustavo at codesourcery dot com>
- Cc: Pedro Alves <palves at redhat dot com>, gdb-patches at sourceware dot org
- Date: Wed, 12 Aug 2015 12:59:48 -0700
- Subject: Re: [PATCH v4 00/18] All-stop on top of non-stop
- Authentication-results: sourceware.org; auth=none
- References: <1432250354-2721-1-git-send-email-palves at redhat dot com> <55C4E3BD dot 8040801 at redhat dot com> <20150812183208 dot GA24901 at adacore dot com> <55CBA0D1 dot 5000203 at codesourcery dot com>
> Two things about the patch. I see that the change to GDB's code is almost
> trivial, but the testcase looks quite involved.
>
> The first thing is that one wouldn't be able to tell what the testcase does
> without looking at the commit log. dso2dso doesn't particularly translate to
> "displaced stepping instruction masking problem on amd64". Should we change
> the testcase name to something a bit more meaningful? Maybe document it a
> bit?
>
> The second point is, should we restrict this testcase to be executed only
> for amd64? Maybe move it to gdb.arch? Unless you actually tried this
> testcase with different architectures and confirmed the testcase is sane
> there, it feels a bit iffy to add/execute this for non-amd64 targets.
>
> In the worst case, we could have a failure due to different instruction
> scheduling schemes (or maybe even a displaced stepping bug) on non-amd64
> targets. At a minimum, we'd have a spurious PASS that wouldn't be meaningful
> for the overall test results since things never failed on said architecture
> in the first place.
>
> Alternatively, for such a trivial fix, shouldn't we go without a testcase?
Some good points. To me, the purpose of the test is to verify that
stepping from some code in one DSO over a call to a function in another
DSO, is working on all platforms that support shared libraries. That's
the main purpose of the test. If, one day, the implementation changes
so that displaced stepping is no longer necessary, then what matters
at the user level is that this testcase continues to behave the same.
Even if you wanted a gdb.arch test that verified this specific bug,
you'd have to find a way for the test to load at an address that's
high enough that the mask starts causing problems. I don't think
that's very easy, and I'm not sure it's worth our time.
Regarding the complexity of the test, I think we need to try it and
see. At AdaCore, we run the equivalent Ada testcase nightly on Linux
(x86, x86_64 and ppc), Windows (x86 and x86_64), Solaris (sparc) and
Darwin (x86_64), and it passes everywhere. It passes with GDBserver
too. My take would be that we'd be fixing trivial errors in the testcase
itself, and just accept the failures on platforms where it reveals
a bona fide issue, as we've beeing doing with all the other tests.
--
Joel