* [PATCH] gdbserver: make target_pid_to_str return std::string
@ 2021-10-25 14:20 Simon Marchi
2021-10-25 18:29 ` Tom Tromey
0 siblings, 1 reply; 3+ messages in thread
From: Simon Marchi @ 2021-10-25 14:20 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
I wanted to write a warning that included two target_pid_to_str calls,
like this:
warning (_("Blabla %s, blabla %s"),
target_pid_to_str (ptid1),
target_pid_to_str (ptid2));
This doesn't work, because target_pid_to_str stores its result in a
static buffer, so my message would show twice the same ptid. Change
target_pid_to_str to return an std::string to avoid this. I don't think
we save much by using a static buffer, but it is more error-prone.
Change-Id: Ie3f649627686b84930529cc5c7c691ccf5d36dc2
---
gdbserver/linux-low.cc | 62 ++++++++++++++++++++-------------------
gdbserver/remote-utils.cc | 2 +-
gdbserver/server.cc | 4 +--
gdbserver/target.cc | 23 +++++++--------
gdbserver/target.h | 2 +-
gdbserver/tracepoint.cc | 8 ++---
6 files changed, 50 insertions(+), 51 deletions(-)
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index b6820adbd56..34ede238d19 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -842,7 +842,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
struct thread_info *thr = get_lwp_thread (lwp);
debug_printf ("CSBB: %s stopped by software breakpoint\n",
- target_pid_to_str (ptid_of (thr)));
+ target_pid_to_str (ptid_of (thr)).c_str ());
}
/* Back up the PC if necessary. */
@@ -863,7 +863,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
struct thread_info *thr = get_lwp_thread (lwp);
debug_printf ("CSBB: %s stopped by hardware breakpoint\n",
- target_pid_to_str (ptid_of (thr)));
+ target_pid_to_str (ptid_of (thr)).c_str ());
}
}
else if (lwp->stop_reason == TARGET_STOPPED_BY_WATCHPOINT)
@@ -873,7 +873,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
struct thread_info *thr = get_lwp_thread (lwp);
debug_printf ("CSBB: %s stopped by hardware watchpoint\n",
- target_pid_to_str (ptid_of (thr)));
+ target_pid_to_str (ptid_of (thr)).c_str ());
}
}
else if (lwp->stop_reason == TARGET_STOPPED_BY_SINGLE_STEP)
@@ -883,7 +883,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
struct thread_info *thr = get_lwp_thread (lwp);
debug_printf ("CSBB: %s stopped by trace\n",
- target_pid_to_str (ptid_of (thr)));
+ target_pid_to_str (ptid_of (thr)).c_str ());
}
}
@@ -1249,7 +1249,7 @@ linux_kill_one_lwp (struct lwp_info *lwp)
int save_errno = errno;
debug_printf ("LKL: kill_lwp (SIGKILL) %s, 0, 0 (%s)\n",
- target_pid_to_str (ptid_of (thr)),
+ target_pid_to_str (ptid_of (thr)).c_str (),
save_errno ? safe_strerror (save_errno) : "OK");
}
@@ -1260,7 +1260,7 @@ linux_kill_one_lwp (struct lwp_info *lwp)
int save_errno = errno;
debug_printf ("LKL: PTRACE_KILL %s, 0, 0 (%s)\n",
- target_pid_to_str (ptid_of (thr)),
+ target_pid_to_str (ptid_of (thr)).c_str (),
save_errno ? safe_strerror (save_errno) : "OK");
}
}
@@ -1325,7 +1325,7 @@ kill_one_lwp_callback (thread_info *thread, int pid)
{
if (debug_threads)
debug_printf ("lkop: is last of process %s\n",
- target_pid_to_str (thread->id));
+ target_pid_to_str (thread->id).c_str ());
return;
}
@@ -1399,7 +1399,7 @@ get_detach_signal (struct thread_info *thread)
{
if (debug_threads)
debug_printf ("GPS: lwp %s hasn't stopped: no pending signal\n",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
return 0;
}
@@ -1409,7 +1409,7 @@ get_detach_signal (struct thread_info *thread)
if (debug_threads)
debug_printf ("GPS: lwp %s had stopped with extended "
"status: no pending signal\n",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
return 0;
}
@@ -1419,7 +1419,7 @@ get_detach_signal (struct thread_info *thread)
{
if (debug_threads)
debug_printf ("GPS: lwp %s had signal %s, but it is in nopass state\n",
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
gdb_signal_to_string (signo));
return 0;
}
@@ -1433,7 +1433,7 @@ get_detach_signal (struct thread_info *thread)
debug_printf ("GPS: lwp %s had signal %s, "
"but we don't know if we should pass it. "
"Default to not.\n",
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
gdb_signal_to_string (signo));
return 0;
}
@@ -1441,7 +1441,7 @@ get_detach_signal (struct thread_info *thread)
{
if (debug_threads)
debug_printf ("GPS: lwp %s has pending signal %s: delivering it.\n",
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
gdb_signal_to_string (signo));
return WSTOPSIG (status);
@@ -1460,7 +1460,7 @@ linux_process_target::detach_one_lwp (lwp_info *lwp)
{
if (debug_threads)
debug_printf ("Sending SIGCONT to %s\n",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
kill_lwp (lwpid_of (thread), SIGCONT);
lwp->stop_expected = 0;
@@ -1516,14 +1516,14 @@ linux_process_target::detach_one_lwp (lwp_info *lwp)
else
{
error (_("Can't detach %s: %s"),
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
safe_strerror (save_errno));
}
}
else if (debug_threads)
{
debug_printf ("PTRACE_DETACH (%s, %s, 0) (OK)\n",
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
strsignal (sig));
}
@@ -2410,7 +2410,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
SIGSTOP as a normal event. */
if (debug_threads)
debug_printf ("LLW: resume_stop SIGSTOP caught for %s.\n",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
}
else if (stopping_threads != NOT_STOPPING_THREADS)
{
@@ -2419,7 +2419,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
if (debug_threads)
debug_printf ("LLW: SIGSTOP caught for %s "
"while stopping threads.\n",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
return;
}
else
@@ -2428,7 +2428,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
if (debug_threads)
debug_printf ("LLW: %s %s, 0, 0 (discard delayed SIGSTOP)\n",
child->stepping ? "step" : "continue",
- target_pid_to_str (ptid_of (thread)));
+ target_pid_to_str (ptid_of (thread)).c_str ());
resume_one_lwp (child, child->stepping, 0, NULL);
return;
@@ -2471,7 +2471,7 @@ linux_process_target::resume_stopped_resumed_lwps (thread_info *thread)
if (debug_threads)
debug_printf ("RSRL: resuming stopped-resumed LWP %s at %s: step=%d\n",
- target_pid_to_str (ptid_of (thread)),
+ target_pid_to_str (ptid_of (thread)).c_str (),
paddress (lp->stop_pc),
step);
@@ -2717,7 +2717,7 @@ select_event_lwp (struct lwp_info **orig_lp)
{
if (debug_threads)
debug_printf ("SEL: Select single-step %s\n",
- target_pid_to_str (ptid_of (event_thread)));
+ target_pid_to_str (ptid_of (event_thread)).c_str ());
}
}
if (event_thread == NULL)
@@ -2952,7 +2952,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
if (debug_threads)
{
debug_enter ();
- debug_printf ("wait_1: [%s]\n", target_pid_to_str (ptid));
+ debug_printf ("wait_1: [%s]\n", target_pid_to_str (ptid).c_str ());
}
/* Translate generic target options into linux options. */
@@ -2989,7 +2989,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
{
if (debug_threads)
debug_printf ("step_over_bkpt set [%s], doing a blocking wait\n",
- target_pid_to_str (step_over_bkpt));
+ target_pid_to_str (step_over_bkpt).c_str ());
pid = wait_for_event (step_over_bkpt, &w, options & ~WNOHANG);
}
@@ -3034,7 +3034,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
{
debug_printf ("wait_1 ret = %s, exited with "
"retcode %d\n",
- target_pid_to_str (ptid_of (current_thread)),
+ target_pid_to_str (ptid_of (current_thread)).c_str (),
WEXITSTATUS (w));
debug_exit ();
}
@@ -3047,7 +3047,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
{
debug_printf ("wait_1 ret = %s, terminated with "
"signal %d\n",
- target_pid_to_str (ptid_of (current_thread)),
+ target_pid_to_str (ptid_of (current_thread)).c_str (),
WTERMSIG (w));
debug_exit ();
}
@@ -3085,7 +3085,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
if (debug_threads)
{
debug_printf ("step-over for %s executed software breakpoint\n",
- target_pid_to_str (ptid_of (current_thread)));
+ target_pid_to_str (ptid_of (current_thread)).c_str ());
}
if (increment_pc != 0)
@@ -3254,7 +3254,8 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
{
debug_printf ("wait_1 ret = %s, stopped "
"while stabilizing threads\n",
- target_pid_to_str (ptid_of (current_thread)));
+ target_pid_to_str
+ (ptid_of (current_thread)).c_str ());
debug_exit ();
}
@@ -3658,7 +3659,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
if (debug_threads)
{
debug_printf ("wait_1 ret = %s, %d, %d\n",
- target_pid_to_str (ptid_of (current_thread)),
+ target_pid_to_str (ptid_of (current_thread)).c_str (),
ourstatus->kind (), ourstatus->sig ());
debug_exit ();
}
@@ -3969,9 +3970,10 @@ linux_process_target::stop_all_lwps (int suspend, lwp_info *except)
debug_enter ();
debug_printf ("stop_all_lwps (%s, except=%s)\n",
suspend ? "stop-and-suspend" : "stop",
- except != NULL
- ? target_pid_to_str (ptid_of (get_lwp_thread (except)))
- : "none");
+ (except != NULL
+ ? target_pid_to_str
+ (ptid_of (get_lwp_thread (except))).c_str ()
+ : "none"));
}
stopping_threads = (suspend
diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index a4cc3267996..5bc261a9c7b 100644
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -1087,7 +1087,7 @@ prepare_resume_reply (char *buf, ptid_t ptid,
client_state &cs = get_client_state ();
if (debug_threads)
debug_printf ("Writing resume reply for %s:%d\n",
- target_pid_to_str (ptid), status->kind ());
+ target_pid_to_str (ptid).c_str (), status->kind ());
switch (status->kind ())
{
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 2cb378ce9c9..c58f7e01d8d 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -3310,7 +3310,7 @@ queue_stop_reply_callback (thread_info *thread)
= target_waitstatus_to_string (&thread->last_status);
debug_printf ("Reporting thread %s as already stopped with %s\n",
- target_pid_to_str (thread->id),
+ target_pid_to_str (thread->id).c_str (),
status_string.c_str ());
}
@@ -4640,7 +4640,7 @@ handle_target_event (int err, gdb_client_data client_data)
debug_printf ("GDB not connected; forwarding event %d for"
" [%s]\n",
(int) cs.last_status.kind (),
- target_pid_to_str (cs.last_ptid));
+ target_pid_to_str (cs.last_ptid).c_str ());
if (cs.last_status.kind () == TARGET_WAITKIND_STOPPED)
signal = cs.last_status.sig ();
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index b6a1cb24025..bfa860546a0 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -276,26 +276,23 @@ set_target_ops (process_stratum_target *target)
/* Convert pid to printable format. */
-const char *
+std::string
target_pid_to_str (ptid_t ptid)
{
- static char buf[80];
-
if (ptid == minus_one_ptid)
- xsnprintf (buf, sizeof (buf), "<all threads>");
+ return string_printf("<all threads>");
else if (ptid == null_ptid)
- xsnprintf (buf, sizeof (buf), "<null thread>");
+ return string_printf("<null thread>");
else if (ptid.tid () != 0)
- xsnprintf (buf, sizeof (buf), "Thread %d.0x%s",
- ptid.pid (), phex_nz (ptid.tid (), sizeof (ULONGEST)));
+ return string_printf("Thread %d.0x%s",
+ ptid.pid (),
+ phex_nz (ptid.tid (), sizeof (ULONGEST)));
else if (ptid.lwp () != 0)
- xsnprintf (buf, sizeof (buf), "LWP %d.%ld",
- ptid.pid (), ptid.lwp ());
+ return string_printf("LWP %d.%ld",
+ ptid.pid (), ptid.lwp ());
else
- xsnprintf (buf, sizeof (buf), "Process %d",
- ptid.pid ());
-
- return buf;
+ return string_printf("Process %d",
+ ptid.pid ());
}
int
diff --git a/gdbserver/target.h b/gdbserver/target.h
index 2c4393ec8c6..6c863c84666 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -702,6 +702,6 @@ int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
int set_desired_thread ();
-const char *target_pid_to_str (ptid_t);
+std::string target_pid_to_str (ptid_t);
#endif /* GDBSERVER_TARGET_H */
diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc
index bc8fb267e90..c01973b5e61 100644
--- a/gdbserver/tracepoint.cc
+++ b/gdbserver/tracepoint.cc
@@ -4408,7 +4408,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
wstep_link = &tinfo->while_stepping;
trace_debug ("Thread %s finished a single-step for tracepoint %d at 0x%s",
- target_pid_to_str (tinfo->id),
+ target_pid_to_str (tinfo->id).c_str (),
wstep->tp_number, paddress (wstep->tp_address));
ctx.base.type = trap_tracepoint;
@@ -4421,7 +4421,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
{
trace_debug ("NO TRACEPOINT %d at 0x%s FOR THREAD %s!",
wstep->tp_number, paddress (wstep->tp_address),
- target_pid_to_str (tinfo->id));
+ target_pid_to_str (tinfo->id).c_str ());
/* Unlink. */
*wstep_link = wstep->next;
@@ -4441,7 +4441,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
{
/* The requested numbers of steps have occurred. */
trace_debug ("Thread %s done stepping for tracepoint %d at 0x%s",
- target_pid_to_str (tinfo->id),
+ target_pid_to_str (tinfo->id).c_str (),
wstep->tp_number, paddress (wstep->tp_address));
/* Unlink the wstep. */
@@ -4588,7 +4588,7 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc)
&& tpoint->type != static_tracepoint)
{
trace_debug ("Thread %s at address of tracepoint %d at 0x%s",
- target_pid_to_str (tinfo->id),
+ target_pid_to_str (tinfo->id).c_str (),
tpoint->number, paddress (tpoint->address));
/* Test the condition if present, and collect if true. */
--
2.33.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdbserver: make target_pid_to_str return std::string
2021-10-25 14:20 [PATCH] gdbserver: make target_pid_to_str return std::string Simon Marchi
@ 2021-10-25 18:29 ` Tom Tromey
2021-10-25 18:33 ` Simon Marchi
0 siblings, 1 reply; 3+ messages in thread
From: Tom Tromey @ 2021-10-25 18:29 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Simon Marchi
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> This doesn't work, because target_pid_to_str stores its result in a
Simon> static buffer, so my message would show twice the same ptid. Change
Simon> target_pid_to_str to return an std::string to avoid this. I don't think
Simon> we save much by using a static buffer, but it is more error-prone.
I agree and I think this is fine.
thanks,
Tom
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] gdbserver: make target_pid_to_str return std::string
2021-10-25 18:29 ` Tom Tromey
@ 2021-10-25 18:33 ` Simon Marchi
0 siblings, 0 replies; 3+ messages in thread
From: Simon Marchi @ 2021-10-25 18:33 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches; +Cc: Simon Marchi
On 2021-10-25 14:29, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> This doesn't work, because target_pid_to_str stores its result in a
> Simon> static buffer, so my message would show twice the same ptid. Change
> Simon> target_pid_to_str to return an std::string to avoid this. I don't think
> Simon> we save much by using a static buffer, but it is more error-prone.
>
> I agree and I think this is fine.
>
> thanks,
> Tom
>
Thanks, pushed.
Simon
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-25 18:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-25 14:20 [PATCH] gdbserver: make target_pid_to_str return std::string Simon Marchi
2021-10-25 18:29 ` Tom Tromey
2021-10-25 18:33 ` 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).