* [PATCH 1/2] Remove a VEC from ppc-linux-nat.c
2019-04-27 14:31 [PATCH 0/2] Remove two VECs Tom Tromey
2019-04-27 14:31 ` [PATCH 2/2] Remove a VEC from aarch64-tdep.c Tom Tromey
@ 2019-04-27 14:31 ` Tom Tromey
2019-04-27 15:23 ` Simon Marchi
1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2019-04-27 14:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This replaces a VEC in ppc-linux-nat.c with a std::vector.
gdb/ChangeLog
2019-04-27 Tom Tromey <tom@tromey.com>
* ppc-linux-nat.c (thread_points_p): Remove typedef and DEF_VEC.
(ppc_threads): Now a std::vector. Now static.
(hwdebug_find_thread_points_by_tid)
(ppc_linux_nat_target::low_new_thread, ppc_linux_thread_exit):
Update.
---
gdb/ChangeLog | 8 ++++++++
gdb/ppc-linux-nat.c | 30 +++++++++++++-----------------
2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 628e3d5e8f6..45cf594ffca 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -1578,7 +1578,7 @@ struct hw_break_tuple
/* This is an internal VEC created to store information about *points inserted
for each thread. This is used when PowerPC HWDEBUG ptrace interface is
available. */
-typedef struct thread_points
+struct thread_points
{
/* The TID to which this *point relates. */
int tid;
@@ -1589,10 +1589,9 @@ typedef struct thread_points
size of these vector is MAX_SLOTS_NUMBER. If the hw_break element of
the tuple is NULL, then the position in the vector is free. */
struct hw_break_tuple *hw_breaks;
- } *thread_points_p;
-DEF_VEC_P (thread_points_p);
+ };
-VEC(thread_points_p) *ppc_threads = NULL;
+static std::vector<thread_points *> ppc_threads;
/* The version of the PowerPC HWDEBUG kernel interface that we will use, if
available. */
@@ -1758,14 +1757,11 @@ hwdebug_point_cmp (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
static struct thread_points *
hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
{
- int i;
- struct thread_points *t;
-
- for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
+ for (thread_points *t : ppc_threads)
if (t->tid == tid)
return t;
- t = NULL;
+ struct thread_points *t = NULL;
/* Do we need to allocate a new point_item
if the wanted one does not exist? */
@@ -1774,7 +1770,7 @@ hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
t = XNEW (struct thread_points);
t->hw_breaks = XCNEWVEC (struct hw_break_tuple, max_slots_number);
t->tid = tid;
- VEC_safe_push (thread_points_p, ppc_threads, t);
+ ppc_threads.push_back (t);
}
return t;
@@ -2359,11 +2355,11 @@ ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
struct thread_points *p;
struct hw_break_tuple *hw_breaks;
- if (VEC_empty (thread_points_p, ppc_threads))
+ if (ppc_threads.empty ())
return;
/* Get a list of breakpoints from any thread. */
- p = VEC_last (thread_points_p, ppc_threads);
+ p = ppc_threads.last ();
hw_breaks = p->hw_breaks;
/* Copy that thread's breakpoints and watchpoints to the new thread. */
@@ -2392,22 +2388,22 @@ ppc_linux_thread_exit (struct thread_info *tp, int silent)
int i;
int tid = tp->ptid.lwp ();
struct hw_break_tuple *hw_breaks;
- struct thread_points *t = NULL, *p;
+ struct thread_points *t = NULL;
if (!have_ptrace_hwdebug_interface ())
return;
- for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, p); i++)
- if (p->tid == tid)
+ for (i = 0; i < ppc_threads.size (); i++)
+ if (ppc_threads[i].tid == tid)
{
- t = p;
+ t = &ppc_threads[i];
break;
}
if (t == NULL)
return;
- VEC_unordered_remove (thread_points_p, ppc_threads, i);
+ unordered_remove (ppc_threads, i);
hw_breaks = t->hw_breaks;
--
2.17.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/2] Remove two VECs
@ 2019-04-27 14:31 Tom Tromey
2019-04-27 14:31 ` [PATCH 2/2] Remove a VEC from aarch64-tdep.c Tom Tromey
2019-04-27 14:31 ` [PATCH 1/2] Remove a VEC from ppc-linux-nat.c Tom Tromey
0 siblings, 2 replies; 7+ messages in thread
From: Tom Tromey @ 2019-04-27 14:31 UTC (permalink / raw)
To: gdb-patches
This series removes a couple more uses of VEC from gdb, replacing them
with std::vector.
I ran this through the buildbot, but I don't think it actually tests
any of this code, so extra scrutiny would be appreciated.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] Remove a VEC from aarch64-tdep.c
2019-04-27 14:31 [PATCH 0/2] Remove two VECs Tom Tromey
@ 2019-04-27 14:31 ` Tom Tromey
2019-04-27 15:39 ` Simon Marchi
2019-04-27 14:31 ` [PATCH 1/2] Remove a VEC from ppc-linux-nat.c Tom Tromey
1 sibling, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2019-04-27 14:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
This removes a VEC from aarch64-tdep.c, replacing it with a
std::vector.
gdb/ChangeLog
2019-04-27 Tom Tromey <tom@tromey.com>
* aarch64-tdep.c (stack_item_t): Remove typedef and DEF_VEC.
(struct aarch64_call_info): Add initializers.
<si>: Now a std::vector.
(pass_on_stack, aarch64_push_dummy_call): Update.
---
gdb/ChangeLog | 7 +++++++
gdb/aarch64-tdep.c | 36 +++++++++++++++---------------------
2 files changed, 22 insertions(+), 21 deletions(-)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index f8b4fa4c974..2c8c6a1bdc3 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -1206,7 +1206,7 @@ aarch64_execute_dwarf_cfa_vendor_op (struct gdbarch *gdbarch, gdb_byte op,
/* When arguments must be pushed onto the stack, they go on in reverse
order. The code below implements a FILO (stack) to do this. */
-typedef struct
+struct stack_item_t
{
/* Value to pass on stack. It can be NULL if this item is for stack
padding. */
@@ -1214,9 +1214,7 @@ typedef struct
/* Size in bytes of value to pass on stack. */
int len;
-} stack_item_t;
-
-DEF_VEC_O (stack_item_t);
+};
/* Implement the gdbarch type alignment method, overrides the generic
alignment algorithm for anything that is aarch64 specific. */
@@ -1392,22 +1390,22 @@ aapcs_is_vfp_call_or_return_candidate (struct type *type, int *count,
struct aarch64_call_info
{
/* the current argument number. */
- unsigned argnum;
+ unsigned argnum = 0;
/* The next general purpose register number, equivalent to NGRN as
described in the AArch64 Procedure Call Standard. */
- unsigned ngrn;
+ unsigned ngrn = 0;
/* The next SIMD and floating point register number, equivalent to
NSRN as described in the AArch64 Procedure Call Standard. */
- unsigned nsrn;
+ unsigned nsrn = 0;
/* The next stacked argument address, equivalent to NSAA as
described in the AArch64 Procedure Call Standard. */
- unsigned nsaa;
+ unsigned nsaa = 0;
/* Stack item vector. */
- VEC(stack_item_t) *si;
+ std::vector<stack_item_t> si;
};
/* Pass a value in a sequence of consecutive X registers. The caller
@@ -1521,7 +1519,7 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
item.len = len;
item.data = buf;
- VEC_safe_push (stack_item_t, info->si, &item);
+ info->si.push_back (item);
info->nsaa += len;
if (info->nsaa & (align - 1))
@@ -1532,7 +1530,7 @@ pass_on_stack (struct aarch64_call_info *info, struct type *type,
item.len = pad;
item.data = NULL;
- VEC_safe_push (stack_item_t, info->si, &item);
+ info->si.push_back (item);
info->nsaa += pad;
}
}
@@ -1632,8 +1630,6 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
int argnum;
struct aarch64_call_info info;
- memset (&info, 0, sizeof (info));
-
/* We need to know what the type of the called function is in order
to determine the number of named/anonymous arguments for the
actual argument placement, and the return type in order to handle
@@ -1762,18 +1758,16 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
if (info.nsaa & 15)
sp -= 16 - (info.nsaa & 15);
- while (!VEC_empty (stack_item_t, info.si))
+ while (!info.si.empty ())
{
- stack_item_t *si = VEC_last (stack_item_t, info.si);
+ const stack_item_t &si = info.si.back ();
- sp -= si->len;
- if (si->data != NULL)
- write_memory (sp, si->data, si->len);
- VEC_pop (stack_item_t, info.si);
+ sp -= si.len;
+ if (si.data != NULL)
+ write_memory (sp, si.data, si.len);
+ info.si.pop_back ();
}
- VEC_free (stack_item_t, info.si);
-
/* Finally, update the SP register. */
regcache_cooked_write_unsigned (regcache, AARCH64_SP_REGNUM, sp);
--
2.17.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove a VEC from ppc-linux-nat.c
2019-04-27 14:31 ` [PATCH 1/2] Remove a VEC from ppc-linux-nat.c Tom Tromey
@ 2019-04-27 15:23 ` Simon Marchi
2019-04-27 15:31 ` Simon Marchi
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2019-04-27 15:23 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
Hi Tom,
LGTM, I just wrote some suggestions below.
On 2019-04-27 10:31 a.m., Tom Tromey wrote:
> @@ -1758,14 +1757,11 @@ hwdebug_point_cmp (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
> static struct thread_points *
> hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
> {
> - int i;
> - struct thread_points *t;
> -
> - for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
> + for (thread_points *t : ppc_threads)
> if (t->tid == tid)
> return t;
Could you add braces to this for, to match our coding style?
> @@ -2392,22 +2388,22 @@ ppc_linux_thread_exit (struct thread_info *tp, int silent)
> int i;
> int tid = tp->ptid.lwp ();
> struct hw_break_tuple *hw_breaks;
> - struct thread_points *t = NULL, *p;
> + struct thread_points *t = NULL;
>
> if (!have_ptrace_hwdebug_interface ())
> return;
>
> - for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, p); i++)
> - if (p->tid == tid)
> + for (i = 0; i < ppc_threads.size (); i++)
> + if (ppc_threads[i].tid == tid)
> {
> - t = p;
> + t = &ppc_threads[i];
> break;
> }
Here too?
I believe this loop could be a range-based one, though it also works like this.
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove a VEC from ppc-linux-nat.c
2019-04-27 15:23 ` Simon Marchi
@ 2019-04-27 15:31 ` Simon Marchi
2019-05-04 20:26 ` Tom Tromey
0 siblings, 1 reply; 7+ messages in thread
From: Simon Marchi @ 2019-04-27 15:31 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2019-04-27 11:23 a.m., Simon Marchi wrote:
> Hi Tom,
>
> LGTM, I just wrote some suggestions below.
>
> On 2019-04-27 10:31 a.m., Tom Tromey wrote:
>> @@ -1758,14 +1757,11 @@ hwdebug_point_cmp (struct ppc_hw_breakpoint *a, struct ppc_hw_breakpoint *b)
>> static struct thread_points *
>> hwdebug_find_thread_points_by_tid (int tid, int alloc_new)
>> {
>> - int i;
>> - struct thread_points *t;
>> -
>> - for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, t); i++)
>> + for (thread_points *t : ppc_threads)
>> if (t->tid == tid)
>> return t;
>
> Could you add braces to this for, to match our coding style?
>
>> @@ -2392,22 +2388,22 @@ ppc_linux_thread_exit (struct thread_info *tp, int silent)
>> int i;
>> int tid = tp->ptid.lwp ();
>> struct hw_break_tuple *hw_breaks;
>> - struct thread_points *t = NULL, *p;
>> + struct thread_points *t = NULL;
>>
>> if (!have_ptrace_hwdebug_interface ())
>> return;
>>
>> - for (i = 0; VEC_iterate (thread_points_p, ppc_threads, i, p); i++)
>> - if (p->tid == tid)
>> + for (i = 0; i < ppc_threads.size (); i++)
>> + if (ppc_threads[i].tid == tid)
>> {
>> - t = p;
>> + t = &ppc_threads[i];
>> break;
>> }
>
> Here too?
>
> I believe this loop could be a range-based one, though it also works like this.
Nevermind about this last suggestion, as the value of the `i` variable is required
below to do the removal.
But actually when looking at the code a bit closer, I am not sure it works, since
ppc_threads[i] yields a pointer, so using the '.' operator is not valid. And indeed:
/home/simark/src/binutils-gdb/gdb/ppc-linux-nat.c: In function 'void ppc_linux_thread_exit(thread_info*, int)':
/home/simark/src/binutils-gdb/gdb/ppc-linux-nat.c:2397:24: error: request for member 'tid' in 'ppc_threads.std::vector<_Tp, _Alloc>::operator[]<thread_points*, std::allocator<thread_points*> >(((std::vector<thread_points*>::size_type)i))', which is of pointer type '__gnu_cxx::__alloc_traits<std::allocator<thread_points*> >::value_type {aka thread_points*}' (maybe you meant to use '->' ?)
if (ppc_threads[i].tid == tid)
^~~
/home/simark/src/binutils-gdb/gdb/ppc-linux-nat.c:2399:20: error: cannot convert 'thread_points**' to 'thread_points*' in assignment
t = &ppc_threads[i];
^
Building also points out this error, where you probably meant to use .back ():
/home/simark/src/binutils-gdb/gdb/ppc-linux-nat.c: In member function 'virtual void ppc_linux_nat_target::low_new_thread(lwp_info*)':
/home/simark/src/binutils-gdb/gdb/ppc-linux-nat.c:2362:23: error: 'class std::vector<thread_points*>' has no member named 'last'; did you mean 'at'?
p = ppc_threads.last ();
^~~~
Applying this fixup to your patch makes it build for me:
From b687773f708d21e04dfe3b7d7094514d4cf8b7b7 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 27 Apr 2019 11:30:58 -0400
Subject: [PATCH] fixup
---
gdb/ppc-linux-nat.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 45cf594ffcac..59e7ca5863f1 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -2359,7 +2359,7 @@ ppc_linux_nat_target::low_new_thread (struct lwp_info *lp)
return;
/* Get a list of breakpoints from any thread. */
- p = ppc_threads.last ();
+ p = ppc_threads.back ();
hw_breaks = p->hw_breaks;
/* Copy that thread's breakpoints and watchpoints to the new thread. */
@@ -2394,9 +2394,9 @@ ppc_linux_thread_exit (struct thread_info *tp, int silent)
return;
for (i = 0; i < ppc_threads.size (); i++)
- if (ppc_threads[i].tid == tid)
+ if (ppc_threads[i]->tid == tid)
{
- t = &ppc_threads[i];
+ t = ppc_threads[i];
break;
}
--
2.21.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] Remove a VEC from aarch64-tdep.c
2019-04-27 14:31 ` [PATCH 2/2] Remove a VEC from aarch64-tdep.c Tom Tromey
@ 2019-04-27 15:39 ` Simon Marchi
0 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi @ 2019-04-27 15:39 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 2019-04-27 10:31 a.m., Tom Tromey wrote:
> This removes a VEC from aarch64-tdep.c, replacing it with a
> std::vector.
This LGTM, thanks.
Simon
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Remove a VEC from ppc-linux-nat.c
2019-04-27 15:31 ` Simon Marchi
@ 2019-05-04 20:26 ` Tom Tromey
0 siblings, 0 replies; 7+ messages in thread
From: Tom Tromey @ 2019-05-04 20:26 UTC (permalink / raw)
To: Simon Marchi; +Cc: Tom Tromey, gdb-patches
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Applying this fixup to your patch makes it build for me:
Thanks. I've included this & made the other changes you requested.
Tom
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-04 20:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-27 14:31 [PATCH 0/2] Remove two VECs Tom Tromey
2019-04-27 14:31 ` [PATCH 2/2] Remove a VEC from aarch64-tdep.c Tom Tromey
2019-04-27 15:39 ` Simon Marchi
2019-04-27 14:31 ` [PATCH 1/2] Remove a VEC from ppc-linux-nat.c Tom Tromey
2019-04-27 15:23 ` Simon Marchi
2019-04-27 15:31 ` Simon Marchi
2019-05-04 20:26 ` Tom Tromey
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).