Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@imgtec.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: <gdb-patches@sourceware.org>, Yao Qi <yao.qi@linaro.org>,
	Joel Brobecker	<brobecker@adacore.com>
Subject: Re: [PATCH] mem-break: Fix breakpoint insertion location
Date: Fri, 04 Aug 2017 13:58:00 -0000	[thread overview]
Message-ID: <alpine.DEB.2.00.1708041445410.17596@tp.orcam.me.uk> (raw)
In-Reply-To: <172ea7a987fae99d7438bee77a704c76@polymtl.ca>

On Fri, 4 Aug 2017, Simon Marchi wrote:

> On 2017-08-01 18:36, Maciej W. Rozycki wrote:
> > Fix a commit cd6c3b4ffc4e ("New gdbarch methods breakpoint_kind_from_pc
> > and sw_breakpoint_from_kind") regression and restore the use of
> > ->placed_size rather than ->reqstd_address as the location for a memory
> > breakpoint to be inserted at.  Previously `gdbarch_breakpoint_from_pc'
> > was used that made that adjustment in `default_memory_insert_breakpoint'
> > from the preinitialized value, however with the said commit that call is
> > gone, so the passed ->placed_size has to be used for the initialization.
[...]
> IIUC, we end up writing the good breakpoint kind, but at the wrong address?
> For example, if the requested address is 0x1001, it means that there should be
> a micro/compressed MIPS breakpoint at address 0x1000, but that bug caused the
> breakpoint to be written at address 0x1001 instead.  Is that right?

 Exactly!

 Moreover, as the breakpoint is removed the original instruction bytes 
will be written back to 0x1000, further corrupting the executable, as 
`default_memory_remove_breakpoint' already correctly uses 
`->placed_address'.

 I can see now that I incorrectly wrote `->placed_size' across the patch 
description where I meant `->placed_address'.  I'll correct that and 
repost the patch with PR annotation additionally included.

  Maciej


  reply	other threads:[~2017-08-04 13:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-01 16:36 Maciej W. Rozycki
2017-08-02 17:53 ` Maciej W. Rozycki
2017-08-04 13:17 ` Simon Marchi
2017-08-04 13:58   ` Maciej W. Rozycki [this message]
2017-08-04 13:24 ` Yao Qi
2017-08-04 14:13   ` [PATCH v2] PR breakpoints/21886: " Maciej W. Rozycki
2017-08-07 15:35     ` Yao Qi
2017-08-07 16:05       ` Maciej W. Rozycki
2017-08-09  7:44         ` Yao Qi
2017-08-11 14:31           ` Maciej W. Rozycki
2017-08-11 15:12             ` Yao Qi
2017-08-11 16:14               ` Maciej W. Rozycki

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=alpine.DEB.2.00.1708041445410.17596@tp.orcam.me.uk \
    --to=macro@imgtec.com \
    --cc=brobecker@adacore.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@polymtl.ca \
    --cc=yao.qi@linaro.org \
    /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