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

Joel Brobecker
2015-08-12T19:59:53Z

> 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