This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4 00/18] All-stop on top of non-stop


On 08/12/2015 04:59 PM, Joel Brobecker wrote:
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.

Ok. So i think i misunderstood the purpose of the testcase there. In reality the testcase is not testing the fix itself, but rather introducing a new test not related to the problem, except in the amd64 architecture, where it really tests the problem.

It just confused me that the test is generic for other non-amd64 architectures and specific to the amd64 problem you saw.

I'd expect a generic solib test to be included in one of our shared library tests, but then you'd have to shape it in a way that would exercise your displaced stepping problem.


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.

It is not a big deal. In recent times i've been seeing a number of testcases that shouldn't be generally executed for all architectures. That ends up causing spurious failures and bringing the signal/noise ratio of the testsuite down for the affect architectures (usually not x86 or Linux).

I see your targets are mostly x86. I can give it a try on a few more (powerpc, mips, arm, nios) and let you know what i see. How does that sound?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]