From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id cXl9IAcng2qoLS0AWB0awg (envelope-from ) for ; Mon, 17 Aug 2026 11:21:43 -0400 Received: by simark.ca (Postfix, from userid 112) id 7F8D91E167; Mon, 17 Aug 2026 11:21:43 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-5.3 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 91C8B1E033 for ; Mon, 17 Aug 2026 11:21:42 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 251BB4BA9020 for ; Mon, 17 Aug 2026 15:21:42 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 251BB4BA9020 Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id BE9C74BA9006; Mon, 17 Aug 2026 15:18:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BE9C74BA9006 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=efficios.com Authentication-Results: sourceware.org; spf=fail smtp.mailfrom=efficios.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BE9C74BA9006 Authentication-Results: sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1786979933; cv=none; b=iljDF3Gv9DwMTg2xewDma9fW1MoyxXGw41BGDiRnze8sgkUS/u4fyoZL1Z/WlzhO87DRFstgL2VK9GK4V38uIKjE7U7y6QApnTT58WQ0EtPmha0FHN/v77YHrTlzkJVrWR/I/UKcxdeEUxtxkPkGpFgSxfEhezwH/VvN6GR+kxQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1786979933; c=relaxed/simple; bh=MTZFZHdqSI5fJAQGNODTnnSjH7DHkbA26s/puha/lKY=; h=From:To:Subject:Date:Message-ID:MIME-Version; b=WoqlcEUuwK2PyXhyTEz+Y8kBwdSA+L+nMbg9uylYQAiKwsM4mtU7vvLK5j7cHqUziuRV5938BfUE/ep+dnjQXli5fjVWOeixAcR15WP5madaQm6KpRlM6SeDvXWwARiu+BoIHbDA/ysbxgsof9GUpV6Ze3p7loXgSaFnEVj3/OI= ARC-Authentication-Results: i=1; sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org BE9C74BA9006 Received: by simark.ca (Postfix) id 64A331E167; Mon, 17 Aug 2026 11:18:53 -0400 (EDT) From: Simon Marchi To: gdb-patches@sourceware.org, binutils@sourceware.org Cc: Simon Marchi Subject: [PATCH 10/13] gdbsupport: add xstrcpy Date: Mon, 17 Aug 2026 11:16:15 -0400 Message-ID: <20260817151646.152571-11-simon.marchi@efficios.com> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260817151646.152571-1-simon.marchi@efficios.com> References: <20260817151646.152571-1-simon.marchi@efficios.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Add xstrcpy, a "safe" alternative to strcpy. It works like strcpy, but accepts the size of the destination buffer, and asserts that the string fits in it. Return the number of characters copied, so that it's possible to easily chain calls like this: p += xstrcpy (p, end - p, ",C"); Context: I want to replace some code that uses strcpy and strcat to build strings with something that has bound checks. We already have xsnprintf, but sometimes we want to add fixed strings, so xsnprintf is a bit overkill. xstrcpy is essentially xsnprintf but without the formatting. I looked around and changed a bunch of calls to xsnprintf that can be replaced one for one with xstrcpy. Change-Id: Icff6978b7431581732184d045c6ec047e2c78bcb --- gdb/fbsd-nat.c | 2 +- gdb/nat/netbsd-nat.c | 2 +- gdb/remote.c | 67 ++++++++++++-------------- gdb/unittests/common-utils-selftests.c | 29 +++++++++++ gdbsupport/agent.cc | 2 +- gdbsupport/common-utils.cc | 13 +++++ gdbsupport/common-utils.h | 9 ++++ gdbsupport/ptid.cc | 4 +- 8 files changed, 88 insertions(+), 40 deletions(-) diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c index cf59ae21efc0..2c914c0e75be 100644 --- a/gdb/fbsd-nat.c +++ b/gdb/fbsd-nat.c @@ -897,7 +897,7 @@ fbsd_nat_target::thread_name (struct thread_info *thr) return nullptr; if (streq (kp.ki_comm, pl.pl_tdname)) return NULL; - xsnprintf (buf, sizeof buf, "%s", pl.pl_tdname); + xstrcpy (buf, sizeof buf, pl.pl_tdname); return buf; } #endif diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c index 2fe2889d7129..d88dd05cbf57 100644 --- a/gdb/nat/netbsd-nat.c +++ b/gdb/nat/netbsd-nat.c @@ -135,7 +135,7 @@ thread_name (ptid_t ptid) { if (kl->l_lid == lwp) { - xsnprintf (buf, sizeof buf, "%s", kl->l_name); + xstrcpy (buf, sizeof buf, kl->l_name); return true; } return false; diff --git a/gdb/remote.c b/gdb/remote.c index 194c4cbd9bb0..3d38a9c7c8c9 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3045,7 +3045,7 @@ remote_target::remote_query_attached (int pid) if (m_features.remote_multi_process_p ()) xsnprintf (rs->buf.data (), size, "qAttached:%x", pid); else - xsnprintf (rs->buf.data (), size, "qAttached"); + xstrcpy (rs->buf.data (), size, "qAttached"); putpkt (rs->buf); getpkt (&rs->buf); @@ -3509,11 +3509,11 @@ remote_target::set_thread (ptid_t ptid, int gen) *buf++ = 'H'; *buf++ = gen ? 'g' : 'c'; if (ptid == magic_null_ptid) - xsnprintf (buf, endbuf - buf, "0"); + xstrcpy (buf, endbuf - buf, "0"); else if (ptid == any_thread_ptid) - xsnprintf (buf, endbuf - buf, "0"); + xstrcpy (buf, endbuf - buf, "0"); else if (ptid == minus_one_ptid) - xsnprintf (buf, endbuf - buf, "-1"); + xstrcpy (buf, endbuf - buf, "-1"); else write_ptid (buf, endbuf, ptid); putpkt (rs->buf); @@ -4633,8 +4633,7 @@ remote_target::extra_thread_info (thread_info *tp) char *b = rs->buf.data (); char *endb = b + get_remote_packet_size (); - xsnprintf (b, endb - b, "qThreadExtraInfo,"); - b += strlen (b); + b += xstrcpy (b, endb - b, "qThreadExtraInfo,"); write_ptid (b, endb, tp->ptid); putpkt (rs->buf); @@ -4682,8 +4681,7 @@ remote_target::static_tracepoint_marker_at (CORE_ADDR addr, struct remote_state *rs = get_remote_state (); char *p = rs->buf.data (); - xsnprintf (p, get_remote_packet_size (), "qTSTMat:"); - p += strlen (p); + p += xstrcpy (p, get_remote_packet_size (), "qTSTMat:"); p += hexnumstr (p, addr); putpkt (rs->buf); getpkt (&rs->buf); @@ -7215,14 +7213,14 @@ remote_target::append_resumption (char *p, char *endp, addr_size)); } else - p += xsnprintf (p, endp - p, ";s"); + p += xstrcpy (p, endp - p, ";s"); } else if (step) - p += xsnprintf (p, endp - p, ";s"); + p += xstrcpy (p, endp - p, ";s"); else if (siggnal != GDB_SIGNAL_0) p += xsnprintf (p, endp - p, ";C%02x", siggnal); else - p += xsnprintf (p, endp - p, ";c"); + p += xstrcpy (p, endp - p, ";c"); if (m_features.remote_multi_process_p () && ptid.is_pid ()) { @@ -7231,12 +7229,12 @@ remote_target::append_resumption (char *p, char *endp, /* All (-1) threads of process. */ nptid = ptid_t (ptid.pid (), -1); - p += xsnprintf (p, endp - p, ":"); + p += xstrcpy (p, endp - p, ":"); p = write_ptid (p, endp, nptid); } else if (ptid != minus_one_ptid) { - p += xsnprintf (p, endp - p, ":"); + p += xstrcpy (p, endp - p, ":"); p = write_ptid (p, endp, ptid); } @@ -7357,7 +7355,7 @@ remote_target::remote_resume_with_vcont (ptid_t scope_ptid, int step, about overflowing BUF. Should there be a generic "multi-part-packet" packet? */ - p += xsnprintf (p, endp - p, "vCont"); + p += xstrcpy (p, endp - p, "vCont"); if (scope_ptid == magic_null_ptid) { @@ -7535,7 +7533,7 @@ vcont_builder::restart () m_p = rs->buf.data (); m_endp = m_p + m_remote->get_remote_packet_size (); - m_p += xsnprintf (m_p, m_endp - m_p, "vCont"); + m_p += xstrcpy (m_p, m_endp - m_p, "vCont"); m_first_action = m_p; } @@ -7889,12 +7887,12 @@ remote_target::remote_stop_ns (ptid_t ptid) if (ptid == minus_one_ptid || (!m_features.remote_multi_process_p () && ptid.is_pid ())) - p += xsnprintf (p, endp - p, "vCont;t"); + p += xstrcpy (p, endp - p, "vCont;t"); else { ptid_t nptid; - p += xsnprintf (p, endp - p, "vCont;t:"); + p += xstrcpy (p, endp - p, "vCont;t:"); if (ptid.is_pid ()) /* All (-1) threads of process. */ @@ -7955,7 +7953,7 @@ remote_target::remote_interrupt_ns () char *p = rs->buf.data (); char *endp = p + get_remote_packet_size (); - xsnprintf (p, endp - p, "vCtrlC"); + xstrcpy (p, endp - p, "vCtrlC"); /* In non-stop, we get an immediate OK reply. The stop reply will come in asynchronously by notification. */ @@ -9313,7 +9311,7 @@ remote_target::send_g_packet () struct remote_state *rs = get_remote_state (); int buf_len; - xsnprintf (rs->buf.data (), get_remote_packet_size (), "g"); + xstrcpy (rs->buf.data (), get_remote_packet_size (), "g"); putpkt (rs->buf); getpkt (&rs->buf); packet_result result = packet_check_result (rs->buf); @@ -11342,8 +11340,8 @@ remote_target::extended_remote_set_inferior_cwd () { /* An empty inferior_cwd means that the user wants us to reset the remote server's inferior's cwd. */ - xsnprintf (rs->buf.data (), get_remote_packet_size (), - "QSetWorkingDir:"); + xstrcpy (rs->buf.data (), get_remote_packet_size (), + "QSetWorkingDir:"); } putpkt (rs->buf); @@ -11529,8 +11527,7 @@ remote_add_target_side_condition (struct gdbarch *gdbarch, return 0; buf += strlen (buf); - xsnprintf (buf, buf_end - buf, "%s", ";"); - buf++; + buf += xstrcpy (buf, buf_end - buf, ";"); /* Send conditions to the target. */ for (agent_expr *aexpr : bp_tgt->conditions) @@ -14990,7 +14987,7 @@ remote_target::get_min_fast_tracepoint_insn_len () /* Make sure the remote is pointing at the right process. */ set_general_process (); - xsnprintf (rs->buf.data (), get_remote_packet_size (), "qTMinFTPILen"); + xstrcpy (rs->buf.data (), get_remote_packet_size (), "qTMinFTPILen"); putpkt (rs->buf); reply = remote_get_noisy_reply (); if (*reply == '\0') @@ -15015,7 +15012,7 @@ remote_target::set_trace_buffer_size (LONGEST val) char *endbuf = buf + get_remote_packet_size (); gdb_assert (val >= 0 || val == -1); - buf += xsnprintf (buf, endbuf - buf, "QTBuffer:size:"); + buf += xstrcpy (buf, endbuf - buf, "QTBuffer:size:"); /* Send -1 as literal "-1" to avoid host size dependency. */ if (val < 0) { @@ -15049,24 +15046,24 @@ remote_target::set_trace_notes (const char *user, const char *notes, char *endbuf = buf + get_remote_packet_size (); int nbytes; - buf += xsnprintf (buf, endbuf - buf, "QTNotes:"); + buf += xstrcpy (buf, endbuf - buf, "QTNotes:"); if (user) { - buf += xsnprintf (buf, endbuf - buf, "user:"); + buf += xstrcpy (buf, endbuf - buf, "user:"); nbytes = bin2hex ((gdb_byte *) user, buf, strlen (user)); buf += 2 * nbytes; *buf++ = ';'; } if (notes) { - buf += xsnprintf (buf, endbuf - buf, "notes:"); + buf += xstrcpy (buf, endbuf - buf, "notes:"); nbytes = bin2hex ((gdb_byte *) notes, buf, strlen (notes)); buf += 2 * nbytes; *buf++ = ';'; } if (stop_notes) { - buf += xsnprintf (buf, endbuf - buf, "tstop:"); + buf += xstrcpy (buf, endbuf - buf, "tstop:"); nbytes = bin2hex ((gdb_byte *) stop_notes, buf, strlen (stop_notes)); buf += 2 * nbytes; *buf++ = ';'; @@ -15661,8 +15658,8 @@ remote_target::enable_btrace (thread_info *tp, ptid_t ptid = tp->ptid; set_general_thread (ptid); - buf += xsnprintf (buf, endbuf - buf, "%s", - packets_descriptions[which_packet].name); + buf += xstrcpy (buf, endbuf - buf, + packets_descriptions[which_packet].name); putpkt (rs->buf); getpkt (&rs->buf); @@ -15702,8 +15699,8 @@ remote_target::disable_btrace (struct btrace_target_info *tinfo) set_general_thread (tinfo->ptid); - buf += xsnprintf (buf, endbuf - buf, "%s", - packets_descriptions[PACKET_Qbtrace_off].name); + buf += xstrcpy (buf, endbuf - buf, + packets_descriptions[PACKET_Qbtrace_off].name); putpkt (rs->buf); getpkt (&rs->buf); @@ -16074,8 +16071,8 @@ remote_target::commit_requested_thread_options () char *obuf_endp = obuf + max_options_size; *obuf_p++ = ';'; - obuf_p += xsnprintf (obuf_p, obuf_endp - obuf_p, "%s", - phex_nz (options)); + obuf_p += xstrcpy (obuf_p, obuf_endp - obuf_p, + phex_nz (options)); if (tp.ptid != magic_null_ptid) { *obuf_p++ = ':'; diff --git a/gdb/unittests/common-utils-selftests.c b/gdb/unittests/common-utils-selftests.c index eb9c83616f08..4940e412a41a 100644 --- a/gdb/unittests/common-utils-selftests.c +++ b/gdb/unittests/common-utils-selftests.c @@ -125,6 +125,34 @@ string_vappendf_tests () test_appendf_func (string_vappendf_wrapper); } +static void +xstrcpy_tests () +{ + char buf[8]; + char *p; + char *end = buf + sizeof (buf); + + memset (buf, 'x', sizeof (buf)); + p = buf; + p += xstrcpy (p, end - p, "ab"); + SELF_CHECK (p == buf + 2); + p += xstrcpy (p, end - p, "cd"); + SELF_CHECK (p == buf + 4); + SELF_CHECK (strcmp (buf, "abcd") == 0); + + /* A string of exactly SIZE - 1 characters fits. */ + memset (buf, 'x', sizeof (buf)); + p = buf; + SELF_CHECK (xstrcpy (p, end - p, "1234567") == 7); + SELF_CHECK (strcmp (buf, "1234567") == 0); + + /* An empty string is fine, even in a buffer of size 1. */ + memset (buf, 'x', sizeof (buf)); + p = buf; + SELF_CHECK (xstrcpy (p, 1, "") == 0); + SELF_CHECK (strcmp (p, "") == 0); +} + } /* namespace selftests */ INIT_GDB_FILE (common_utils_selftests) @@ -134,4 +162,5 @@ INIT_GDB_FILE (common_utils_selftests) selftests::register_test ("string_appendf", selftests::string_appendf_tests); selftests::register_test ("string_vappendf", selftests::string_vappendf_tests); + selftests::register_test ("xstrcpy", selftests::xstrcpy_tests); } diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc index 44b6fcdcf5c7..2054815bfc8b 100644 --- a/gdbsupport/agent.cc +++ b/gdbsupport/agent.cc @@ -154,7 +154,7 @@ gdb_connect_sync_socket (int pid) addr.sun_family = AF_UNIX; - res = xsnprintf (addr.sun_path, UNIX_PATH_MAX, "%s", path); + res = xstrcpy (addr.sun_path, UNIX_PATH_MAX, path); if (res >= UNIX_PATH_MAX) { warning (_("string overflow allocating socket name")); diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc index f31699be13a1..4aeaaf99f787 100644 --- a/gdbsupport/common-utils.cc +++ b/gdbsupport/common-utils.cc @@ -86,6 +86,19 @@ xsnprintf (char *str, size_t size, const char *format, ...) return ret; } +/* See common-utils.h. */ + +int +xstrcpy (char *str, size_t size, const char *src) +{ + size_t len = strlen (src); + + gdb_assert (len < size); + memcpy (str, src, len + 1); + + return len; +} + /* See documentation in common-utils.h. */ std::string diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h index de83a715ac45..0c4dcb8efe86 100644 --- a/gdbsupport/common-utils.h +++ b/gdbsupport/common-utils.h @@ -51,6 +51,15 @@ gdb::unique_xmalloc_ptr xstrvprintf (const char *format, va_list ap) int xsnprintf (char *str, size_t size, const char *format, ...) ATTRIBUTE_PRINTF (3, 4); +/* Like strcpy, but takes the size of the destination buffer STR as SIZE, + and throws an error if SRC does not fit in it. + + Return the number of characters copied, excluding the terminating null + character. + + This is equivalent to xsnprintf when no formatting is needed. */ +int xstrcpy (char *str, size_t size, const char *src); + /* Returns a std::string built from a printf-style format string. */ std::string string_printf (const char* fmt, ...) ATTRIBUTE_PRINTF (1, 2); diff --git a/gdbsupport/ptid.cc b/gdbsupport/ptid.cc index 933e441f9b87..d5a5fce4c123 100644 --- a/gdbsupport/ptid.cc +++ b/gdbsupport/ptid.cc @@ -38,12 +38,12 @@ ptid_t::to_rsp_string (bool multi) const if (multi) { if (m_pid == -1) - buf += xsnprintf (buf, endbuf - buf, "p-1."); + buf += xstrcpy (buf, endbuf - buf, "p-1."); else buf += xsnprintf (buf, endbuf - buf, "p%x.", (unsigned) m_pid); } if (m_lwp == -1) - xsnprintf (buf, endbuf - buf, "-1"); + xstrcpy (buf, endbuf - buf, "-1"); else xsnprintf (buf, endbuf - buf, "%lx", (unsigned long) m_lwp); -- 2.55.0