public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tom@tromey.com>
To: Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>
Cc: Tom Tromey <tom@tromey.com>,  Simon Marchi <simark@simark.ca>,
	 Simon Marchi <simon.marchi@efficios.com>
Subject: Re: [PATCH 9/9] gdb/testsuite/dap: fix gdb.dap/basic-dap.exp disassembly test for PIE
Date: Thu, 26 Jan 2023 08:08:10 -0700	[thread overview]
Message-ID: <87o7qls59h.fsf@tromey.com> (raw)
In-Reply-To: <62a8ae16-1ebf-ebbb-fb0d-2c23cb56c5dc@simark.ca> (Simon Marchi via Gdb-patches's message of "Wed, 25 Jan 2023 22:40:59 -0500")

>> This is an odd one.  It isn't super clear when memory references are
>> invalidated.  (The spec doesn't even define what a memory reference is.)

Simon> I'm not sure what you mean.

Actually I used the wrong word there.  In this case the term is really
"instruction reference"

Anyway, what I mean is that the DAP spec refers to "instruction
reference" without defining it.  So for example the response to a
setBreakpoints request is essentially an array of Breakpoint, which is
documented as:

  /**
   * A memory reference to where the breakpoint is set.
   */
  instructionReference?: string;

However, what are the semantics of this?  The spec does not say.  So, we
don't know what values might be valid (and pragmatically I think a
client has to assume it must treat them as opaque cookies).  When are
they invalidated, or are they assumed to be valid for the lifetime of
the inferior?

Maybe gdb could send an InvalidatedEvent but even this is pretty vague.

Simon> Since there isn't a DAP event that says when symbols have changed
Simon> (AFAIK), I think DAP clients have to assume that any symbol address may
Simon> be invalid after resuming the program.

I agree, though it's unwritten.

>> I don't think this is possible in DAP.  One of the many holes.

Simon> When I saw this in the DisassembleArguments DAP structure:

Simon>   /**
Simon>    * Memory reference to the base location containing the instructions to
Simon>    * disassemble.
Simon>    */
Simon>   memoryReference: string;

Simon> I assumed that memoryReference could be pretty much anything that
Simon> resolves to an address, essentially the same thing you could pass to
Simon> GDB's disassemble command.  But that's just my interpretation of it.

This would be convenient and perhaps it is what gdb ought to do, but I
don't see any text in the spec supporting this approach, so unless a
client specifically knows it will only talk to gdb, it seems risky for
them to use it.  And of course if you're specifically talking only to
gdb, you might as well use MI, which despite its flaws has solved all
the problems that DAP still has.

Probably what would be better for gdb is something that uses the DAP
transport -- JSON-RPC is just amazingly easy to work with -- but with MI
requests as the high-level protocol.  Or, that would have been better in
some alternate universe anyway.

Anyway I imagine the route forward is to file a bunch of DAP bugs and
try to get them resolved; though I haven't seen much progress on, e.g.,
the multiple location bug.

Tom

  reply	other threads:[~2023-01-26 15:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06 18:57 [PATCH 0/9] Fix gdb.dap/basic-dap.exp " Simon Marchi
2023-01-06 18:57 ` [PATCH 1/9] gdb/testsuite/dap: use gdb_assert in gdb.dap/basic-dap.exp Simon Marchi
2023-01-25 21:58   ` Tom Tromey
2023-01-06 18:57 ` [PATCH 2/9] gdb/testsuite/dap: prefix some procs with _ Simon Marchi
2023-01-25 21:59   ` Tom Tromey
2023-01-06 18:57 ` [PATCH 3/9] gdb/testsuite/dap: write requests to gdb.log Simon Marchi
2023-01-25 21:59   ` Tom Tromey
2023-01-25 22:01   ` Tom Tromey
2023-01-26  3:01     ` Simon Marchi
2023-01-06 18:57 ` [PATCH 4/9] gdb/testsuite/dap: make dap_request_and_response not catch / issue test result Simon Marchi
2023-01-06 18:57 ` [PATCH 5/9] gdb/testsuite/dap: remove catch from dap_read_event Simon Marchi
2023-01-06 18:57 ` [PATCH 6/9] gdb/testsuite/dap: pass around dicts instead of ton objects Simon Marchi
2023-01-25 22:04   ` Tom Tromey
2023-01-26  3:29     ` Simon Marchi
2023-01-26 14:55       ` Tom Tromey
2023-01-06 18:57 ` [PATCH 7/9] gdb/testsuite/dap: rename dap_read_event to dap_wait_for_event_and_check Simon Marchi
2023-01-06 18:57 ` [PATCH 8/9] gdb/testsuite/dap: make dap_wait_for_event_and_check return preceding messages Simon Marchi
2023-01-06 18:57 ` [PATCH 9/9] gdb/testsuite/dap: fix gdb.dap/basic-dap.exp disassembly test for PIE Simon Marchi
2023-01-25 22:10   ` Tom Tromey
2023-01-26  3:40     ` Simon Marchi
2023-01-26 15:08       ` Tom Tromey [this message]
2023-01-26 20:21         ` Simon Marchi
2023-01-16 16:07 ` [PATCH 0/9] Fix gdb.dap/basic-dap.exp " Simon Marchi
2023-01-25 22:10   ` Tom Tromey
2023-01-26 20:22     ` Simon Marchi

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=87o7qls59h.fsf@tromey.com \
    --to=tom@tromey.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simark@simark.ca \
    --cc=simon.marchi@efficios.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).