gdb: cleanup of displaced_step_inferior_state::reset/displaced_step_clear
authorSimon Marchi <simon.marchi@efficios.com>
Fri, 14 Feb 2020 20:11:58 +0000 (15:11 -0500)
committerSimon Marchi <simon.marchi@efficios.com>
Wed, 10 Jun 2020 21:25:48 +0000 (17:25 -0400)
commitb8bfbca548dc14482d3a2ed08315cdf0d16dfdc7
treee6c17c7e6e6fe7acddb4a6ac2bb833200fb4783b
parentec980634d2a10045bf79605ca0f3806f817ae2b7
gdb: cleanup of displaced_step_inferior_state::reset/displaced_step_clear

displaced_step_inferior_state::reset and displaced_step_clear appear to
have the same goal, but they don't do the same thing.
displaced_step_inferior_state::reset clears more things than
displaced_step_clear, but it misses free'ing the closure, which
displaced_step_clear does.

This patch replaces displaced_step_clear's implementation with just a call to
displaced_step_inferior_state::reset.  It then changes
displaced_step_inferior_state::step_closure to be a unique_ptr, to indicate the
fact that displaced_step_inferior_state owns the closure (and so that it is
automatically freed when the field is reset).

The test gdb.base/step-over-syscall.exp caught a problem when doing this, which
I consider to be a latent bug which my cleanup exposes.  In
handle_inferior_event, in the TARGET_WAITKIND_FORKED case, if we displaced-step
over a fork syscall, we make sure to restore the memory that we used as a
displaced-stepping buffer in the child.  We do so using the
displaced_step_inferior_state of the parent.  However, we do it after calling
displaced_step_fixup for the parent, which clears the information in the
parent's displaced_step_inferior_state.  It worked fine before, because
displaced_step_clear didn't completely clear the displaced_step_inferior_state
structure, so the required information (in this case the gdbarch) was
still available after clearing.

I fixed it by making GDB restore the child's memory before calling the
displaced_step_fixup on the parent.  This way, the data in the
displaced_step_inferior_state structure is still valid when we use it for the
child.  This is the error you would get in
gdb.base/step-over-syscall.exp without this fix:

    /home/smarchi/src/binutils-gdb/gdb/gdbarch.c:3911: internal-error: ULONGEST gdbarch_max_insn_length(gdbarch*): Assertion `gdbarch != NULL' failed.

gdb/ChangeLog:

* infrun.c (get_displaced_step_closure_by_addr): Adjust to
std::unique_ptr.
(displaced_step_clear): Rename to...
(displaced_step_reset): ... this.  Just call displaced->reset ().
(displaced_step_clear_cleanup): Rename to...
(displaced_step_reset_cleanup): ... this.
(displaced_step_prepare_throw): Adjust to std::unique_ptr.
(displaced_step_fixup): Likewise.
(resume_1): Likewise.
(handle_inferior_event): Restore child's memory before calling
displaced_step_fixup on the parent.
* infrun.h (displaced_step_inferior_state) <reset>: Adjust
to std::unique_ptr.
<step_closure>: Change type to std::unique_ptr.
gdb/ChangeLog
gdb/infrun.c
gdb/infrun.h
This page took 0.024984 seconds and 4 git commands to generate.