* [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers
@ 2017-03-22 17:29 Simon Marchi
2017-03-22 17:51 ` Simon Marchi
2017-03-23 16:19 ` Pedro Alves
0 siblings, 2 replies; 5+ messages in thread
From: Simon Marchi @ 2017-03-22 17:29 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Now that the to_fetch_registers, to_store_registers and
to_prepare_to_store target methods don't rely on the value of
inferior_ptid anymore, we can remove a bunch of now unnecessary setting
and restoring of inferior_ptid.
The asserts added recently in target_fetch_registers and
target_store_registers, which validate that inferior_ptid matches the
regcache's ptid, must go away. It's the whole point of this effort, to
not require inferior_ptid to have a particular value when calling these
functions.
One thing that I noticed is how sol-thread.c's ps_lgetregs and friends
use the current value of inferior_ptid instead of what's passed as
argument (ph->ptid), unlike proc-service.c's versions of the same
functions. Is it expected? I left it like this in the current patch,
but unless there's a good reason for it to be that way, I guess we
should make it use the parameter.
gdb/ChangeLog:
* fbsd-tdep.c (fbsd_corefile_thread): Don't set/restore
inferior_ptid.
* proc-service.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
ps_lsetfpregs): Likewise.
* regcache.c (regcache_raw_update, regcache_raw_write): Likewise.
* sol-thread.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
ps_lsetfpregs): Likewise.
* target.c (target_fetch_registers, target_store_registers):
Remove asserts.
---
gdb/fbsd-tdep.c | 4 ----
gdb/proc-service.c | 36 ++++++++++++------------------------
gdb/regcache.c | 17 +++--------------
gdb/sol-thread.c | 46 ++++++++++++----------------------------------
gdb/target.c | 4 ----
5 files changed, 27 insertions(+), 80 deletions(-)
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index e757910023..46875d8efd 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -193,15 +193,11 @@ static void
fbsd_corefile_thread (struct thread_info *info,
struct fbsd_corefile_thread_data *args)
{
- struct cleanup *old_chain;
struct regcache *regcache;
regcache = get_thread_arch_regcache (info->ptid, args->gdbarch);
- old_chain = save_inferior_ptid ();
- inferior_ptid = info->ptid;
target_fetch_registers (regcache, -1);
- do_cleanups (old_chain);
args->note_data = fbsd_collect_thread_registers
(regcache, info->ptid, args->obfd, args->note_data,
diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index d39830cb69..415ba0a7f3 100644
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -158,16 +158,13 @@ ps_pdwrite (gdb_ps_prochandle_t ph, psaddr_t addr,
ps_err_e
ps_lgetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid, prgregset_t gregset)
{
- struct cleanup *old_chain = save_inferior_ptid ();
- struct regcache *regcache;
-
- inferior_ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
target_fetch_registers (regcache, -1);
fill_gregset (regcache, (gdb_gregset_t *) gregset, -1);
- do_cleanups (old_chain);
return PS_OK;
}
@@ -177,16 +174,13 @@ ps_lgetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid, prgregset_t gregset)
ps_err_e
ps_lsetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid, const prgregset_t gregset)
{
- struct cleanup *old_chain = save_inferior_ptid ();
- struct regcache *regcache;
-
- inferior_ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
supply_gregset (regcache, (const gdb_gregset_t *) gregset);
target_store_registers (regcache, -1);
- do_cleanups (old_chain);
return PS_OK;
}
@@ -197,16 +191,13 @@ ps_err_e
ps_lgetfpregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
gdb_prfpregset_t *fpregset)
{
- struct cleanup *old_chain = save_inferior_ptid ();
- struct regcache *regcache;
-
- inferior_ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
target_fetch_registers (regcache, -1);
fill_fpregset (regcache, (gdb_fpregset_t *) fpregset, -1);
- do_cleanups (old_chain);
return PS_OK;
}
@@ -217,16 +208,13 @@ ps_err_e
ps_lsetfpregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
const gdb_prfpregset_t *fpregset)
{
- struct cleanup *old_chain = save_inferior_ptid ();
- struct regcache *regcache;
-
- inferior_ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (ph->ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
supply_fpregset (regcache, (const gdb_fpregset_t *) fpregset);
target_store_registers (regcache, -1);
- do_cleanups (old_chain);
return PS_OK;
}
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 58d4f56292..71223a1dc4 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -666,11 +666,7 @@ regcache_raw_update (struct regcache *regcache, int regnum)
if (!regcache->readonly_p
&& regcache_register_status (regcache, regnum) == REG_UNKNOWN)
{
- struct cleanup *old_chain = save_inferior_ptid ();
-
- inferior_ptid = regcache->ptid;
target_fetch_registers (regcache, regnum);
- do_cleanups (old_chain);
/* A number of targets can't access the whole set of raw
registers (because the debug API provides no means to get at
@@ -937,8 +933,7 @@ void
regcache_raw_write (struct regcache *regcache, int regnum,
const gdb_byte *buf)
{
- struct cleanup *chain_before_save_inferior;
- struct cleanup *chain_before_invalidate_register;
+ struct cleanup *old_chain;
gdb_assert (regcache != NULL && buf != NULL);
gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
@@ -956,24 +951,18 @@ regcache_raw_write (struct regcache *regcache, int regnum,
regcache->descr->sizeof_register[regnum]) == 0))
return;
- chain_before_save_inferior = save_inferior_ptid ();
- inferior_ptid = regcache->ptid;
-
target_prepare_to_store (regcache);
regcache_raw_set_cached_value (regcache, regnum, buf);
/* Register a cleanup function for invalidating the register after it is
written, in case of a failure. */
- chain_before_invalidate_register
- = make_cleanup_regcache_invalidate (regcache, regnum);
+ old_chain = make_cleanup_regcache_invalidate (regcache, regnum);
target_store_registers (regcache, regnum);
/* The target did not throw an error so we can discard invalidating the
register and restore the cleanup chain to what it was. */
- discard_cleanups (chain_before_invalidate_register);
-
- do_cleanups (chain_before_save_inferior);
+ discard_cleanups (old_chain);
}
void
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index efb3bd0b21..ae9b1411cf 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -881,19 +881,13 @@ ps_ptwrite (gdb_ps_prochandle_t ph, gdb_ps_addr_t addr,
ps_err_e
ps_lgetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid, prgregset_t gregset)
{
- struct cleanup *old_chain;
- struct regcache *regcache;
-
- old_chain = save_inferior_ptid ();
-
- inferior_ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
target_fetch_registers (regcache, -1);
fill_gregset (regcache, (gdb_gregset_t *) gregset, -1);
- do_cleanups (old_chain);
-
return PS_OK;
}
@@ -903,13 +897,9 @@ ps_err_e
ps_lsetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
const prgregset_t gregset)
{
- struct cleanup *old_chain;
- struct regcache *regcache;
-
- old_chain = save_inferior_ptid ();
-
- inferior_ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
supply_gregset (regcache, (const gdb_gregset_t *) gregset);
target_store_registers (regcache, -1);
@@ -961,19 +951,13 @@ ps_err_e
ps_lgetfpregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
prfpregset_t *fpregset)
{
- struct cleanup *old_chain;
- struct regcache *regcache;
-
- old_chain = save_inferior_ptid ();
-
- inferior_ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
target_fetch_registers (regcache, -1);
fill_fpregset (regcache, (gdb_fpregset_t *) fpregset, -1);
- do_cleanups (old_chain);
-
return PS_OK;
}
@@ -983,19 +967,13 @@ ps_err_e
ps_lsetfpregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
const prfpregset_t * fpregset)
{
- struct cleanup *old_chain;
- struct regcache *regcache;
-
- old_chain = save_inferior_ptid ();
-
- inferior_ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
- regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch ());
+ ptid_t ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
+ struct regcache *regcache
+ = get_thread_arch_regcache (ptid, target_gdbarch ());
supply_fpregset (regcache, (const gdb_fpregset_t *) fpregset);
target_store_registers (regcache, -1);
- do_cleanups (old_chain);
-
return PS_OK;
}
diff --git a/gdb/target.c b/gdb/target.c
index 359bf0dec9..0ff8515d3b 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3587,8 +3587,6 @@ debug_print_register (const char * func,
void
target_fetch_registers (struct regcache *regcache, int regno)
{
- gdb_assert (ptid_equal (regcache_get_ptid (regcache), inferior_ptid));
-
current_target.to_fetch_registers (¤t_target, regcache, regno);
if (targetdebug)
debug_print_register ("target_fetch_registers", regcache, regno);
@@ -3600,8 +3598,6 @@ target_store_registers (struct regcache *regcache, int regno)
if (!may_write_registers)
error (_("Writing to registers is not allowed (regno %d)"), regno);
- gdb_assert (ptid_equal (regcache_get_ptid (regcache), inferior_ptid));
-
current_target.to_store_registers (¤t_target, regcache, regno);
if (targetdebug)
{
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers
2017-03-22 17:29 [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers Simon Marchi
@ 2017-03-22 17:51 ` Simon Marchi
2017-03-23 17:00 ` Pedro Alves
2017-03-23 16:19 ` Pedro Alves
1 sibling, 1 reply; 5+ messages in thread
From: Simon Marchi @ 2017-03-22 17:51 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches
On 2017-03-22 13:29, Simon Marchi wrote:
> diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
> index efb3bd0b21..ae9b1411cf 100644
> --- a/gdb/sol-thread.c
> +++ b/gdb/sol-thread.c
> @@ -903,13 +897,9 @@ ps_err_e
> ps_lsetregs (gdb_ps_prochandle_t ph, lwpid_t lwpid,
> const prgregset_t gregset)
> {
> - struct cleanup *old_chain;
> - struct regcache *regcache;
> -
> - old_chain = save_inferior_ptid ();
> -
> - inferior_ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
> - regcache = get_thread_arch_regcache (inferior_ptid, target_gdbarch
> ());
> + ptid_t ptid = ptid_build (ptid_get_pid (inferior_ptid), lwpid, 0);
> + struct regcache *regcache
> + = get_thread_arch_regcache (ptid, target_gdbarch ());
>
> supply_gregset (regcache, (const gdb_gregset_t *) gregset);
> target_store_registers (regcache, -1);
I actually tried to build sol-thread.o on Solaris and noticed I forgot
to remove a "do_cleanups (old_chain);" just below here. I fixed it
locally.
I couldn't get the whole of GDB to build however:
g++ -std=gnu++11 -g3 -O0 -I. -I. -I./common -I./config
-DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H
-I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I./../zlib
-I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber
-I./gnulib/import -Ibuild-gnulib/import -DTUI=1
-I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wpointer-arith
-Wno-unused -Wunused-value -Wunused-function -Wno-switch
-Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter
-Wunused-but-set-variable -Wno-sign-compare -Wno-write-strings
-Wno-narrowing -Wformat-nonliteral -Wno-deprecated-declarations -Werror
-c -o i386-sol2-nat.o -MT i386-sol2-nat.o -MMD -MP -MF
.deps/i386-sol2-nat.Tpo i386-sol2-nat.c
In file included from /usr/include/sys/procfs.h:26:0,
from i386-sol2-nat.c:23:
/usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in
the large file compilation environment"
#error "Cannot use procfs in the large file compilation environment"
^
gmake: *** [i386-sol2-nat.o] Error 1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers
2017-03-22 17:29 [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers Simon Marchi
2017-03-22 17:51 ` Simon Marchi
@ 2017-03-23 16:19 ` Pedro Alves
2017-03-23 17:39 ` Simon Marchi
1 sibling, 1 reply; 5+ messages in thread
From: Pedro Alves @ 2017-03-23 16:19 UTC (permalink / raw)
To: Simon Marchi, gdb-patches
On 03/22/2017 05:29 PM, Simon Marchi wrote:
> Now that the to_fetch_registers, to_store_registers and
> to_prepare_to_store target methods don't rely on the value of
> inferior_ptid anymore, we can remove a bunch of now unnecessary setting
> and restoring of inferior_ptid.
>
> The asserts added recently in target_fetch_registers and
> target_store_registers, which validate that inferior_ptid matches the
> regcache's ptid, must go away. It's the whole point of this effort, to
> not require inferior_ptid to have a particular value when calling these
> functions.
>
> One thing that I noticed is how sol-thread.c's ps_lgetregs and friends
> use the current value of inferior_ptid instead of what's passed as
> argument (ph->ptid), unlike proc-service.c's versions of the same
> functions. Is it expected? I left it like this in the current patch,
> but unless there's a good reason for it to be that way, I guess we
> should make it use the parameter.
Probably no good reason.
>
> gdb/ChangeLog:
>
> * fbsd-tdep.c (fbsd_corefile_thread): Don't set/restore
> inferior_ptid.
> * proc-service.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
> ps_lsetfpregs): Likewise.
> * regcache.c (regcache_raw_update, regcache_raw_write): Likewise.
> * sol-thread.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
> ps_lsetfpregs): Likewise.
> * target.c (target_fetch_registers, target_store_registers):
> Remove asserts.
LGTM.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers
2017-03-22 17:51 ` Simon Marchi
@ 2017-03-23 17:00 ` Pedro Alves
0 siblings, 0 replies; 5+ messages in thread
From: Pedro Alves @ 2017-03-23 17:00 UTC (permalink / raw)
To: Simon Marchi, Simon Marchi; +Cc: gdb-patches
On 03/22/2017 05:51 PM, Simon Marchi wrote:
> I couldn't get the whole of GDB to build however:
>
> g++ -std=gnu++11 -g3 -O0 -I. -I. -I./common -I./config
> -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H
> -I./../include/opcode -I./../opcodes/.. -I./../readline/.. -I./../zlib
> -I../bfd -I./../bfd -I./../include -I../libdecnumber -I./../libdecnumber
> -I./gnulib/import -Ibuild-gnulib/import -DTUI=1
> -I/usr/include/python2.7 -I/usr/include/python2.7 -Wall -Wpointer-arith
> -Wno-unused -Wunused-value -Wunused-function -Wno-switch
> -Wno-char-subscripts -Wempty-body -Wunused-but-set-parameter
> -Wunused-but-set-variable -Wno-sign-compare -Wno-write-strings
> -Wno-narrowing -Wformat-nonliteral -Wno-deprecated-declarations -Werror
> -c -o i386-sol2-nat.o -MT i386-sol2-nat.o -MMD -MP -MF
> .deps/i386-sol2-nat.Tpo i386-sol2-nat.c
> In file included from /usr/include/sys/procfs.h:26:0,
> from i386-sol2-nat.c:23:
> /usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in
> the large file compilation environment"
> #error "Cannot use procfs in the large file compilation environment"
> ^
> gmake: *** [i386-sol2-nat.o] Error 1
>
Yeah, the 32-bit build runs into that, unless you configure with
--disable-largefile. See:
https://sourceware.org/ml/gdb-patches/2016-10/msg00510.html
If you target 64-bit mode (may need CC="gcc -m64" CXX="g++ -m64",
and/or explicit --host, I don't recall exactly), then it should
build OOTB.
Thanks,
Pedro Alves
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers
2017-03-23 16:19 ` Pedro Alves
@ 2017-03-23 17:39 ` Simon Marchi
0 siblings, 0 replies; 5+ messages in thread
From: Simon Marchi @ 2017-03-23 17:39 UTC (permalink / raw)
To: Pedro Alves; +Cc: Simon Marchi, gdb-patches
On 2017-03-23 12:19, Pedro Alves wrote:
> On 03/22/2017 05:29 PM, Simon Marchi wrote:
>> Now that the to_fetch_registers, to_store_registers and
>> to_prepare_to_store target methods don't rely on the value of
>> inferior_ptid anymore, we can remove a bunch of now unnecessary
>> setting
>> and restoring of inferior_ptid.
>>
>> The asserts added recently in target_fetch_registers and
>> target_store_registers, which validate that inferior_ptid matches the
>> regcache's ptid, must go away. It's the whole point of this effort,
>> to
>> not require inferior_ptid to have a particular value when calling
>> these
>> functions.
>>
>> One thing that I noticed is how sol-thread.c's ps_lgetregs and friends
>> use the current value of inferior_ptid instead of what's passed as
>> argument (ph->ptid), unlike proc-service.c's versions of the same
>> functions. Is it expected? I left it like this in the current patch,
>> but unless there's a good reason for it to be that way, I guess we
>> should make it use the parameter.
>
> Probably no good reason.
>
>
>>
>> gdb/ChangeLog:
>>
>> * fbsd-tdep.c (fbsd_corefile_thread): Don't set/restore
>> inferior_ptid.
>> * proc-service.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
>> ps_lsetfpregs): Likewise.
>> * regcache.c (regcache_raw_update, regcache_raw_write): Likewise.
>> * sol-thread.c (ps_lgetregs, ps_lsetregs, ps_lgetfpregs,
>> ps_lsetfpregs): Likewise.
>> * target.c (target_fetch_registers, target_store_registers):
>> Remove asserts.
>
> LGTM.
Thanks, pushed.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-23 17:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 17:29 [PATCH] Remove some unnecessary inferior_ptid setting/restoring when fetching/storing registers Simon Marchi
2017-03-22 17:51 ` Simon Marchi
2017-03-23 17:00 ` Pedro Alves
2017-03-23 16:19 ` Pedro Alves
2017-03-23 17:39 ` Simon Marchi
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).