Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Simon Marchi (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: Mihails Strasuns <mihails.strasuns@intel.com>,
	gdb-patches@sourceware.org
Subject: [review v2] jit: remove bp locations when unregistering jit code
Date: Tue, 26 Nov 2019 16:58:00 -0000	[thread overview]
Message-ID: <20191126165852.1BBC320AF6@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1574686491000.Id9133540d67fa0c4619ac88324b0349b89e4b2b1@gnutoolchain-gerrit.osci.io>

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/704
......................................................................


Patch Set 2:

> Patch Set 1:
> 
> > Patch Set 1:
> > 
> > Just trying to understand the problem better.  From what I understand, when the breakpoint locations must be updated following a code change event (e.g. solib getting loaded or unloaded), we call breakpoint_re_set, which goes through all breakpoint locations and sees if they must be updated.  Have you tryied calling this from jit_unregister_code?
> 
> Have tried it now, just in case - it doesn't work. As far as I understand the problem here is that from the gdb PoV breakpoint locations with the same address are the same and `locations_are_equal (existing_locations, b->loc)` condition is true. Because of that updating breakpoint locations won't actually install breakpoint traps - gdb will still think that it is already installed. However in JIT case that instruction memory was overwritten and doesn't have a trap anymore.
> 
> Sadly I don't have enough knowledge about gdb architecture to reason if the same problem can possibly manifest with non-jit object files.

There's still something that doesn't connect in my mind.  I may have a wrong picture of what's happening, so please correct me.

From my understanding of the JIT interface (https://sourceware.org/gdb/current/onlinedocs/gdb/JIT-Interface.html), unregistering code is made by the process calling __jit_debug_register_code (on which GDB has a special breakpoint) with action == JIT_UNREGISTER.  Registering code is made by the process calling __jit_debug_register_code with action == JIT_REGISTER.

So to unregister a jit region and register a new one (that would happen to have the exact same code at the exact same address as the previous one), the process would need to call __jit_debug_register_code twice, executing between the two events.  After unregistration, when the execution resumes, shouldn't there be something that deletes the breakpoint locations related to that objfile that was removed?  And then when __jit_debug_register_code for registering the new object, we would re-create brand new breakpoint locations?

Or is it that somehow, the unregistration and re-registration is made during the same stop?


-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id9133540d67fa0c4619ac88324b0349b89e4b2b1
Gerrit-Change-Number: 704
Gerrit-PatchSet: 2
Gerrit-Owner: Mihails Strasuns <mihails.strasuns@intel.com>
Gerrit-Reviewer: Mihails Strasuns <mihails.strasuns@intel.com>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 26 Nov 2019 16:58:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


  parent reply	other threads:[~2019-11-26 16:58 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-25 12:54 [review] " Mihails Strasuns (Code Review)
2019-11-25 12:58 ` Mihails Strasuns (Code Review)
2019-11-25 14:40 ` Simon Marchi (Code Review)
2019-11-26 11:02 ` Mihails Strasuns (Code Review)
2019-11-26 11:27 ` [review v2] " Mihails Strasuns (Code Review)
2019-11-26 11:29 ` Mihails Strasuns (Code Review)
2019-11-26 16:58 ` Simon Marchi (Code Review) [this message]
2019-11-26 17:07 ` Mihails Strasuns (Code Review)
2019-11-27  1:38 ` Luis Machado (Code Review)
2019-11-27  8:22 ` Mihails Strasuns (Code Review)
2019-11-27 12:44 ` Luis Machado (Code Review)
2019-11-27 12:59 ` Mihails Strasuns (Code Review)
2019-11-27 13:40 ` Luis Machado (Code Review)
2019-11-28  5:10 ` Simon Marchi (Code Review)
2019-12-10 15:22 ` Mihails Strasuns (Code Review)
2019-12-11  5:51 ` Simon Marchi (Code Review)
2019-12-11  9:24 ` Mihails Strasuns (Code Review)
2019-12-11 16:19 ` Simon Marchi (Code Review)
2019-12-15  2:41 ` Simon Marchi (Code Review)
2019-12-18 16:37 ` Mihails Strasuns (Code Review)
2019-12-18 17:32 ` Simon Marchi (Code Review)
2019-12-19 10:28 ` [review v3] " Mihails Strasuns (Code Review)
2019-12-19 10:33 ` Mihails Strasuns (Code Review)
2020-01-13 10:00 ` Mihails Strasuns (Code Review)

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=20191126165852.1BBC320AF6@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=gdb-patches@sourceware.org \
    --cc=gnutoolchain-gerrit@osci.io \
    --cc=mihails.strasuns@intel.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