Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Joel Brobecker <brobecker@adacore.com>
To: Luis Machado <lgustavo@codesourcery.com>
Cc: Pedro Alves <palves@redhat.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH v4 00/18] All-stop on top of non-stop
Date: Wed, 12 Aug 2015 19:59:00 -0000	[thread overview]
Message-ID: <20150812195948.GH22245@adacore.com> (raw)
In-Reply-To: <55CBA0D1.5000203@codesourcery.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


  parent reply	other threads:[~2015-08-12 19:59 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-21 23:19 Pedro Alves
2015-05-21 23:19 ` [PATCH 01/18] Fix and test "checkpoint" in non-stop mode Pedro Alves
2015-05-21 23:19 ` [PATCH 06/18] Use keep_going in proceed and start_step_over too Pedro Alves
2015-05-21 23:19 ` [PATCH 18/18] native Linux: enable always non-stop by default Pedro Alves
2015-05-21 23:19 ` [PATCH 12/18] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop Pedro Alves
2015-05-21 23:19 ` [PATCH 08/18] Add comments to currently_stepping and target_resume Pedro Alves
2015-05-21 23:19 ` [PATCH 05/18] Embed the pending step-over chain in thread_info objects Pedro Alves
2015-05-21 23:19 ` [PATCH 02/18] Change adjust_pc_after_break's prototype Pedro Alves
2015-05-21 23:19 ` [PATCH 03/18] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG Pedro Alves
2015-05-21 23:19 ` [PATCH 16/18] PPC64: Fix gdb.arch/ppc64-atomic-inst.exp with displaced stepping Pedro Alves
2015-05-21 23:19 ` [PATCH 09/18] Factor out code to re-resume stepped thread Pedro Alves
2015-05-21 23:19 ` [PATCH 11/18] Implement all-stop on top of a target running non-stop mode Pedro Alves
2015-09-11 20:53   ` Jan Kratochvil
2015-09-14 14:24     ` Pedro Alves
2015-05-21 23:19 ` [PATCH 14/18] Fix step-over-{trips-on-watchpoint|lands-on-breakpoint}.exp race Pedro Alves
2015-05-21 23:19 ` [PATCH 04/18] Make thread_still_needs_step_over consider stepping_over_watchpoint too Pedro Alves
2015-05-21 23:25 ` [PATCH 07/18] Misc switch_back_to_stepped_thread cleanups Pedro Alves
2015-05-21 23:28 ` [PATCH 13/18] Fix interrupt-noterm.exp on targets always in non-stop Pedro Alves
2015-05-21 23:50 ` [PATCH 17/18] S/390: displaced stepping and PC-relative RIL-b/RIL-c instructions Pedro Alves
2015-05-21 23:56 ` [PATCH 10/18] Teach non-stop to do in-line step-overs (stop all, step, restart) Pedro Alves
2015-05-21 23:59 ` [PATCH 15/18] Disable displaced stepping if trying it fails Pedro Alves
2015-08-07 16:58 ` [PATCH v4 00/18] All-stop on top of non-stop Pedro Alves
2015-08-12 18:32   ` Joel Brobecker
2015-08-12 19:05     ` Pedro Alves
2015-08-12 20:26       ` Joel Brobecker
2015-08-12 20:31         ` Luis Machado
2015-08-12 20:50           ` Joel Brobecker
2015-08-12 21:07             ` Luis Machado
2015-08-13 16:04             ` Pedro Alves
2015-08-13 18:17               ` Joel Brobecker
2015-08-13 17:27         ` Pedro Alves
2015-08-13 18:29           ` Joel Brobecker
2015-08-12 19:39     ` Luis Machado
2015-08-12 19:51       ` Sergio Durigan Junior
2015-08-12 19:59       ` Joel Brobecker [this message]
2015-08-12 20:18         ` Luis Machado
2015-08-12 20:33           ` Joel Brobecker
2015-08-12 20:40             ` Luis Machado
2015-08-12 20:56               ` Joel Brobecker

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=20150812195948.GH22245@adacore.com \
    --to=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=lgustavo@codesourcery.com \
    --cc=palves@redhat.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