* [PATCH 01/13] gdbsupport: remove uses of vsprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 15:47 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 02/13] gdbsupport: remove uses of sprintf Simon Marchi
` (13 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get:
CXX common-utils.o
/Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:106:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
106 | vsprintf (&str[0], fmt, vp);
| ^
/Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:128:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
128 | vsprintf (&str[0], fmt, args);
| ^
/Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:166:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
166 | vsprintf (&str[curr_size], fmt, args);
| ^
We know that those calls should be safe because we computed the size that
fmt+args take just before, and allocated that many bytes. But I also
don't see a real downside in switching those calls to use vsnprintf and
double check that everything went right.
Change the type of the existing "size" variable in "string_vprintf" to
"int", since that's what vsnprintf returns.
Change-Id: I589d9a170fdd15cc31b44b76689c6d8c324e340a
---
gdbsupport/common-utils.cc | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
index 3ae3afcc380b..f31699be13a1 100644
--- a/gdbsupport/common-utils.cc
+++ b/gdbsupport/common-utils.cc
@@ -92,10 +92,9 @@ std::string
string_printf (const char* fmt, ...)
{
va_list vp;
- int size;
va_start (vp, fmt);
- size = vsnprintf (NULL, 0, fmt, vp);
+ int size = vsnprintf (NULL, 0, fmt, vp);
va_end (vp);
std::string str (size, '\0');
@@ -103,7 +102,8 @@ string_printf (const char* fmt, ...)
/* C++11 and later guarantee std::string uses contiguous memory and
always includes the terminating '\0'. */
va_start (vp, fmt);
- vsprintf (&str[0], fmt, vp);
+ int ret = vsnprintf (&str[0], size + 1, fmt, vp);
+ gdb_assert (ret == size);
va_end (vp);
return str;
@@ -115,17 +115,17 @@ std::string
string_vprintf (const char* fmt, va_list args)
{
va_list vp;
- size_t size;
va_copy (vp, args);
- size = vsnprintf (NULL, 0, fmt, vp);
+ int size = vsnprintf (NULL, 0, fmt, vp);
va_end (vp);
std::string str (size, '\0');
/* C++11 and later guarantee std::string uses contiguous memory and
always includes the terminating '\0'. */
- vsprintf (&str[0], fmt, args);
+ int ret = vsnprintf (&str[0], size + 1, fmt, args);
+ gdb_assert (ret == size);
return str;
}
@@ -152,10 +152,9 @@ std::string &
string_vappendf (std::string &str, const char *fmt, va_list args)
{
va_list vp;
- int grow_size;
va_copy (vp, args);
- grow_size = vsnprintf (NULL, 0, fmt, vp);
+ int grow_size = vsnprintf (NULL, 0, fmt, vp);
va_end (vp);
size_t curr_size = str.size ();
@@ -163,7 +162,8 @@ string_vappendf (std::string &str, const char *fmt, va_list args)
/* C++11 and later guarantee std::string uses contiguous memory and
always includes the terminating '\0'. */
- vsprintf (&str[curr_size], fmt, args);
+ int ret = vsnprintf (&str[curr_size], grow_size + 1, fmt, args);
+ gdb_assert (ret == grow_size);
return str;
}
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 01/13] gdbsupport: remove uses of vsprintf
2026-08-17 15:16 ` [PATCH 01/13] gdbsupport: remove uses of vsprintf Simon Marchi
@ 2026-08-17 15:47 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 15:47 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get:
>
> CXX common-utils.o
> /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:106:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 106 | vsprintf (&str[0], fmt, vp);
> | ^
> /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:128:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 128 | vsprintf (&str[0], fmt, args);
> | ^
> /Users/smarchi/src/binutils-gdb/gdbsupport/common-utils.cc:166:3: error: 'vsprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use vsnprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 166 | vsprintf (&str[curr_size], fmt, args);
> | ^
>
> We know that those calls should be safe because we computed the size that
> fmt+args take just before, and allocated that many bytes. But I also
> don't see a real downside in switching those calls to use vsnprintf and
> double check that everything went right.
>
> Change the type of the existing "size" variable in "string_vprintf" to
> "int", since that's what vsnprintf returns.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: I589d9a170fdd15cc31b44b76689c6d8c324e340a
> ---
> gdbsupport/common-utils.cc | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
> index 3ae3afcc380b..f31699be13a1 100644
> --- a/gdbsupport/common-utils.cc
> +++ b/gdbsupport/common-utils.cc
> @@ -92,10 +92,9 @@ std::string
> string_printf (const char* fmt, ...)
> {
> va_list vp;
> - int size;
>
> va_start (vp, fmt);
> - size = vsnprintf (NULL, 0, fmt, vp);
> + int size = vsnprintf (NULL, 0, fmt, vp);
> va_end (vp);
>
> std::string str (size, '\0');
> @@ -103,7 +102,8 @@ string_printf (const char* fmt, ...)
> /* C++11 and later guarantee std::string uses contiguous memory and
> always includes the terminating '\0'. */
> va_start (vp, fmt);
> - vsprintf (&str[0], fmt, vp);
> + int ret = vsnprintf (&str[0], size + 1, fmt, vp);
> + gdb_assert (ret == size);
> va_end (vp);
>
> return str;
> @@ -115,17 +115,17 @@ std::string
> string_vprintf (const char* fmt, va_list args)
> {
> va_list vp;
> - size_t size;
>
> va_copy (vp, args);
> - size = vsnprintf (NULL, 0, fmt, vp);
> + int size = vsnprintf (NULL, 0, fmt, vp);
> va_end (vp);
>
> std::string str (size, '\0');
>
> /* C++11 and later guarantee std::string uses contiguous memory and
> always includes the terminating '\0'. */
> - vsprintf (&str[0], fmt, args);
> + int ret = vsnprintf (&str[0], size + 1, fmt, args);
> + gdb_assert (ret == size);
>
> return str;
> }
> @@ -152,10 +152,9 @@ std::string &
> string_vappendf (std::string &str, const char *fmt, va_list args)
> {
> va_list vp;
> - int grow_size;
>
> va_copy (vp, args);
> - grow_size = vsnprintf (NULL, 0, fmt, vp);
> + int grow_size = vsnprintf (NULL, 0, fmt, vp);
> va_end (vp);
>
> size_t curr_size = str.size ();
> @@ -163,7 +162,8 @@ string_vappendf (std::string &str, const char *fmt, va_list args)
>
> /* C++11 and later guarantee std::string uses contiguous memory and
> always includes the terminating '\0'. */
> - vsprintf (&str[curr_size], fmt, args);
> + int ret = vsnprintf (&str[curr_size], grow_size + 1, fmt, args);
> + gdb_assert (ret == grow_size);
>
> return str;
> }
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 02/13] gdbsupport: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
2026-08-17 15:16 ` [PATCH 01/13] gdbsupport: remove uses of vsprintf Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 15:49 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 03/13] opcodes/z80: remove use " Simon Marchi
` (12 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get some errors about the uses of sprintf:
CXX xml-utils.o
/Users/smarchi/src/binutils-gdb/gdbsupport/xml-utils.cc:91:8: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
91 | sprintf (str, "%d", va_arg (ap, int));
| ^
We know they are safe, because the 32 byte destination buffer is large
enough for all conversions. But I also don't think it's a big deal to
switch to xsnprintf to avoid these errors, and to catch any future
error.
Change-Id: If3531e1916e103dfccd0ec033639b14a2b6df3cf
---
gdbsupport/xml-utils.cc | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/gdbsupport/xml-utils.cc b/gdbsupport/xml-utils.cc
index 13dc27499120..cec7281c9297 100644
--- a/gdbsupport/xml-utils.cc
+++ b/gdbsupport/xml-utils.cc
@@ -88,52 +88,55 @@ string_xml_appendf (std::string &buffer, const char *format, ...)
str = va_arg (ap, char *);
break;
case 'd':
- sprintf (str, "%d", va_arg (ap, int));
+ xsnprintf (buf, sizeof (buf), "%d", va_arg (ap, int));
break;
case 'u':
- sprintf (str, "%u", va_arg (ap, unsigned int));
+ xsnprintf (buf, sizeof (buf), "%u", va_arg (ap, unsigned int));
break;
case 'x':
- sprintf (str, "%x", va_arg (ap, unsigned int));
+ xsnprintf (buf, sizeof (buf), "%x", va_arg (ap, unsigned int));
break;
case 'o':
- sprintf (str, "%o", va_arg (ap, unsigned int));
+ xsnprintf (buf, sizeof (buf), "%o", va_arg (ap, unsigned int));
break;
case 'l':
f++;
switch (*f)
{
case 'd':
- sprintf (str, "%ld", va_arg (ap, long));
+ xsnprintf (buf, sizeof (buf), "%ld", va_arg (ap, long));
break;
case 'u':
- sprintf (str, "%lu", va_arg (ap, unsigned long));
+ xsnprintf (buf, sizeof (buf), "%lu",
+ va_arg (ap, unsigned long));
break;
case 'x':
- sprintf (str, "%lx", va_arg (ap, unsigned long));
+ xsnprintf (buf, sizeof (buf), "%lx",
+ va_arg (ap, unsigned long));
break;
case 'o':
- sprintf (str, "%lo", va_arg (ap, unsigned long));
+ xsnprintf (buf, sizeof (buf), "%lo",
+ va_arg (ap, unsigned long));
break;
case 'l':
f++;
switch (*f)
{
case 'd':
- sprintf (str, "%" PRId64,
- (int64_t) va_arg (ap, long long));
+ xsnprintf (buf, sizeof (buf), "%" PRId64,
+ (int64_t) va_arg (ap, long long));
break;
case 'u':
- sprintf (str, "%" PRIu64,
- (uint64_t) va_arg (ap, unsigned long long));
+ xsnprintf (buf, sizeof (buf), "%" PRIu64,
+ (uint64_t) va_arg (ap, unsigned long long));
break;
case 'x':
- sprintf (str, "%" PRIx64,
- (uint64_t) va_arg (ap, unsigned long long));
+ xsnprintf (buf, sizeof (buf), "%" PRIx64,
+ (uint64_t) va_arg (ap, unsigned long long));
break;
case 'o':
- sprintf (str, "%" PRIo64,
- (uint64_t) va_arg (ap, unsigned long long));
+ xsnprintf (buf, sizeof (buf), "%" PRIo64,
+ (uint64_t) va_arg (ap, unsigned long long));
break;
default:
str = 0;
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 02/13] gdbsupport: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 02/13] gdbsupport: remove uses of sprintf Simon Marchi
@ 2026-08-17 15:49 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 15:49 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get some errors about the uses of sprintf:
>
> CXX xml-utils.o
> /Users/smarchi/src/binutils-gdb/gdbsupport/xml-utils.cc:91:8: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 91 | sprintf (str, "%d", va_arg (ap, int));
> | ^
>
> We know they are safe, because the 32 byte destination buffer is large
> enough for all conversions. But I also don't think it's a big deal to
> switch to xsnprintf to avoid these errors, and to catch any future
> error.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: If3531e1916e103dfccd0ec033639b14a2b6df3cf
> ---
> gdbsupport/xml-utils.cc | 35 +++++++++++++++++++----------------
> 1 file changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/gdbsupport/xml-utils.cc b/gdbsupport/xml-utils.cc
> index 13dc27499120..cec7281c9297 100644
> --- a/gdbsupport/xml-utils.cc
> +++ b/gdbsupport/xml-utils.cc
> @@ -88,52 +88,55 @@ string_xml_appendf (std::string &buffer, const char *format, ...)
> str = va_arg (ap, char *);
> break;
> case 'd':
> - sprintf (str, "%d", va_arg (ap, int));
> + xsnprintf (buf, sizeof (buf), "%d", va_arg (ap, int));
> break;
> case 'u':
> - sprintf (str, "%u", va_arg (ap, unsigned int));
> + xsnprintf (buf, sizeof (buf), "%u", va_arg (ap, unsigned int));
> break;
> case 'x':
> - sprintf (str, "%x", va_arg (ap, unsigned int));
> + xsnprintf (buf, sizeof (buf), "%x", va_arg (ap, unsigned int));
> break;
> case 'o':
> - sprintf (str, "%o", va_arg (ap, unsigned int));
> + xsnprintf (buf, sizeof (buf), "%o", va_arg (ap, unsigned int));
> break;
> case 'l':
> f++;
> switch (*f)
> {
> case 'd':
> - sprintf (str, "%ld", va_arg (ap, long));
> + xsnprintf (buf, sizeof (buf), "%ld", va_arg (ap, long));
> break;
> case 'u':
> - sprintf (str, "%lu", va_arg (ap, unsigned long));
> + xsnprintf (buf, sizeof (buf), "%lu",
> + va_arg (ap, unsigned long));
> break;
> case 'x':
> - sprintf (str, "%lx", va_arg (ap, unsigned long));
> + xsnprintf (buf, sizeof (buf), "%lx",
> + va_arg (ap, unsigned long));
> break;
> case 'o':
> - sprintf (str, "%lo", va_arg (ap, unsigned long));
> + xsnprintf (buf, sizeof (buf), "%lo",
> + va_arg (ap, unsigned long));
> break;
> case 'l':
> f++;
> switch (*f)
> {
> case 'd':
> - sprintf (str, "%" PRId64,
> - (int64_t) va_arg (ap, long long));
> + xsnprintf (buf, sizeof (buf), "%" PRId64,
> + (int64_t) va_arg (ap, long long));
> break;
> case 'u':
> - sprintf (str, "%" PRIu64,
> - (uint64_t) va_arg (ap, unsigned long long));
> + xsnprintf (buf, sizeof (buf), "%" PRIu64,
> + (uint64_t) va_arg (ap, unsigned long long));
> break;
> case 'x':
> - sprintf (str, "%" PRIx64,
> - (uint64_t) va_arg (ap, unsigned long long));
> + xsnprintf (buf, sizeof (buf), "%" PRIx64,
> + (uint64_t) va_arg (ap, unsigned long long));
> break;
> case 'o':
> - sprintf (str, "%" PRIo64,
> - (uint64_t) va_arg (ap, unsigned long long));
> + xsnprintf (buf, sizeof (buf), "%" PRIo64,
> + (uint64_t) va_arg (ap, unsigned long long));
> break;
> default:
> str = 0;
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 03/13] opcodes/z80: remove use of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
2026-08-17 15:16 ` [PATCH 01/13] gdbsupport: remove uses of vsprintf Simon Marchi
2026-08-17 15:16 ` [PATCH 02/13] gdbsupport: remove uses of sprintf Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-18 6:40 ` Jan Beulich
2026-08-17 15:16 ` [PATCH 04/13] sim/ppc: make defines.h sed command portable Simon Marchi
` (11 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get:
CC z80-dis.lo
/Users/smarchi/src/binutils-gdb/opcodes/z80-dis.c:804:41: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
804 | info->fprintf_func = (fprintf_ftype) &sprintf;
| ^
Replace this use of sprintf with the safer snprintf. Add a small
structure and wrappers around snprintf in order to glue everything
together.
When asked to review my patch, Claude Code mentioned that the existing
code had a latent bug: while info->fprintf_func and info->stream get set
temporarily, info->fprintf_styled_func doesn't. If fprintf_styled_func
happened to be called, it would receive a `stream` it doesn't expect.
It's probably not a problem today, if the disassembler doesn't emit
styling, but it seems like a good moment to fix it. Use
the disassemble_set_printf function to set both fprintf functions and
the stream argument at the same time.
Change-Id: I85dee82f3a0c53f38e52ca1158bc605854ab4896
---
opcodes/z80-dis.c | 52 +++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 48 insertions(+), 4 deletions(-)
diff --git a/opcodes/z80-dis.c b/opcodes/z80-dis.c
index d5b4c4210d0c..fd446d3f128d 100644
--- a/opcodes/z80-dis.c
+++ b/opcodes/z80-dis.c
@@ -768,11 +768,55 @@ pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
static int
print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
+struct sized_buf
+{
+ char *buf;
+ size_t size;
+};
+
+/* An fprintf_ftype implementation writing to STREAM, which must point to a
+ struct sized_buf. */
+
+static int ATTRIBUTE_PRINTF_2
+sized_buf_printf (void *stream, const char *format, ...)
+{
+ va_list ap;
+ int ret;
+ struct sized_buf *sbuf = (struct sized_buf *) stream;
+
+ va_start (ap, format);
+ ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
+ va_end (ap);
+
+ return ret;
+}
+
+/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
+ The style is ignored for now. */
+
+static int ATTRIBUTE_PRINTF_3
+sized_buf_styled_printf (void *stream,
+ enum disassembler_style style ATTRIBUTE_UNUSED,
+ const char *format, ...)
+{
+ va_list ap;
+ int ret;
+ struct sized_buf *sbuf = (struct sized_buf *) stream;
+
+ va_start (ap, format);
+ ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
+ va_end (ap);
+
+ return ret;
+}
+
static int
suffix (struct buffer *buf, disassemble_info *info, const char *txt)
{
char mybuf[TXTSIZ*4];
+ struct sized_buf sbuf = { mybuf, sizeof (mybuf) };
fprintf_ftype old_fprintf;
+ fprintf_styled_ftype old_fprintf_styled;
void *old_stream;
char *p;
@@ -800,15 +844,15 @@ suffix (struct buffer *buf, disassemble_info *info, const char *txt)
}
old_fprintf = info->fprintf_func;
+ old_fprintf_styled = info->fprintf_styled_func;
old_stream = info->stream;
- info->fprintf_func = (fprintf_ftype) &sprintf;
- info->stream = mybuf;
+ disassemble_set_printf (info, &sbuf, sized_buf_printf,
+ sized_buf_styled_printf);
mybuf[0] = 0;
buf->base++;
if (print_insn_z80_buf (buf, info) >= 0)
buf->n_used++;
- info->fprintf_func = old_fprintf;
- info->stream = old_stream;
+ disassemble_set_printf (info, old_stream, old_fprintf, old_fprintf_styled);
for (p = mybuf; *p; ++p)
if (*p == ' ')
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 03/13] opcodes/z80: remove use of sprintf
2026-08-17 15:16 ` [PATCH 03/13] opcodes/z80: remove use " Simon Marchi
@ 2026-08-18 6:40 ` Jan Beulich
2026-08-18 16:48 ` Simon Marchi
0 siblings, 1 reply; 35+ messages in thread
From: Jan Beulich @ 2026-08-18 6:40 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, binutils
On 17.08.2026 17:16, Simon Marchi wrote:
> --- a/opcodes/z80-dis.c
> +++ b/opcodes/z80-dis.c
> @@ -768,11 +768,55 @@ pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
> static int
> print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
>
> +struct sized_buf
> +{
> + char *buf;
> + size_t size;
> +};
> +
> +/* An fprintf_ftype implementation writing to STREAM, which must point to a
> + struct sized_buf. */
> +
> +static int ATTRIBUTE_PRINTF_2
> +sized_buf_printf (void *stream, const char *format, ...)
> +{
> + va_list ap;
> + int ret;
> + struct sized_buf *sbuf = (struct sized_buf *) stream;
We're in the (slow going) process of removing such unnecessary casts, in the
interest of getting the amount of casts down in general. Please drop this one
as well as ...
> + va_start (ap, format);
> + ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
> + va_end (ap);
> +
> + return ret;
> +}
> +
> +/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
> + The style is ignored for now. */
> +
> +static int ATTRIBUTE_PRINTF_3
> +sized_buf_styled_printf (void *stream,
> + enum disassembler_style style ATTRIBUTE_UNUSED,
> + const char *format, ...)
> +{
> + va_list ap;
> + int ret;
> + struct sized_buf *sbuf = (struct sized_buf *) stream;
... the one here.
> + va_start (ap, format);
> + ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
> + va_end (ap);
> +
> + return ret;
> +}
> +
> static int
> suffix (struct buffer *buf, disassemble_info *info, const char *txt)
> {
> char mybuf[TXTSIZ*4];
> + struct sized_buf sbuf = { mybuf, sizeof (mybuf) };
Please use ARRAY_SIZE() here. sizeof() happens to be correct for char[], but
wouldn't be correct for e.g. wchar_t[].
Okay with these adjustments.
Jan
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 03/13] opcodes/z80: remove use of sprintf
2026-08-18 6:40 ` Jan Beulich
@ 2026-08-18 16:48 ` Simon Marchi
0 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-18 16:48 UTC (permalink / raw)
To: Jan Beulich, Simon Marchi; +Cc: gdb-patches, binutils
On 8/18/26 2:40 AM, Jan Beulich wrote:
> On 17.08.2026 17:16, Simon Marchi wrote:
>> --- a/opcodes/z80-dis.c
>> +++ b/opcodes/z80-dis.c
>> @@ -768,11 +768,55 @@ pref_ind (struct buffer *buf, disassemble_info *info, const char *txt)
>> static int
>> print_insn_z80_buf (struct buffer *buf, disassemble_info *info);
>>
>> +struct sized_buf
>> +{
>> + char *buf;
>> + size_t size;
>> +};
>> +
>> +/* An fprintf_ftype implementation writing to STREAM, which must point to a
>> + struct sized_buf. */
>> +
>> +static int ATTRIBUTE_PRINTF_2
>> +sized_buf_printf (void *stream, const char *format, ...)
>> +{
>> + va_list ap;
>> + int ret;
>> + struct sized_buf *sbuf = (struct sized_buf *) stream;
>
> We're in the (slow going) process of removing such unnecessary casts, in the
> interest of getting the amount of casts down in general. Please drop this one
> as well as ...
Ah sorry, C++ habit.
>
>> + va_start (ap, format);
>> + ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
>> + va_end (ap);
>> +
>> + return ret;
>> +}
>> +
>> +/* Same as sized_buf_printf, but as an fprintf_styled_ftype implementation.
>> + The style is ignored for now. */
>> +
>> +static int ATTRIBUTE_PRINTF_3
>> +sized_buf_styled_printf (void *stream,
>> + enum disassembler_style style ATTRIBUTE_UNUSED,
>> + const char *format, ...)
>> +{
>> + va_list ap;
>> + int ret;
>> + struct sized_buf *sbuf = (struct sized_buf *) stream;
>
> ... the one here.
Fixed.
>> + va_start (ap, format);
>> + ret = vsnprintf (sbuf->buf, sbuf->size, format, ap);
>> + va_end (ap);
>> +
>> + return ret;
>> +}
>> +
>> static int
>> suffix (struct buffer *buf, disassemble_info *info, const char *txt)
>> {
>> char mybuf[TXTSIZ*4];
>> + struct sized_buf sbuf = { mybuf, sizeof (mybuf) };
>
> Please use ARRAY_SIZE() here. sizeof() happens to be correct for char[], but
> wouldn't be correct for e.g. wchar_t[].
Fixed.
> Okay with these adjustments.
Thanks, pushed with those changes.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 04/13] sim/ppc: make defines.h sed command portable
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (2 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 03/13] opcodes/z80: remove use " Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 15:36 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 05/13] sim/m32r: fix unused variable warning on non-Linux hosts Simon Marchi
` (10 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, whose sed is the BSD one, I get:
GEN ppc/stamp-defines
sed: 1: "/^#define HAVE_.*1$/{ s ...": extra characters at the end of p command
make[1]: *** [ppc/stamp-defines] Error 1
BSD sed apparently does not accept a `}' directly after another command,
it needs a separating semicolon. Add one after the `p'. GNU sed
accepts both forms, and produces the same output either way.
Re-generate sim/Makefile.in.
Change-Id: I0709ad7b0051e08299f2576113b549aba9fc703f
---
sim/Makefile.in | 2 +-
sim/ppc/local.mk | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sim/Makefile.in b/sim/Makefile.in
index 1f9bbfec03f1..2a23b5c2eaa9 100644
--- a/sim/Makefile.in
+++ b/sim/Makefile.in
@@ -5722,7 +5722,7 @@ testsuite/common/bits64m63.c: testsuite/common/bits-gen$(EXEEXT) testsuite/commo
@SIM_ENABLE_ARCH_ppc_TRUE@ppc/defines.h: ppc/stamp-defines ; @true
@SIM_ENABLE_ARCH_ppc_TRUE@ppc/stamp-defines: config.h Makefile
-@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p }' < config.h > ppc/defines.hin
+@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p; }' < config.h > ppc/defines.hin
@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_at)$(SHELL) $(srcroot)/move-if-change ppc/defines.hin ppc/defines.h
@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_at)touch $@
diff --git a/sim/ppc/local.mk b/sim/ppc/local.mk
index f9f134abe3c9..9aca96465acd 100644
--- a/sim/ppc/local.mk
+++ b/sim/ppc/local.mk
@@ -80,7 +80,7 @@ noinst_PROGRAMS += %D%/run
%D%/defines.h: %D%/stamp-defines ; @true
%D%/stamp-defines: config.h Makefile
- $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p }' < config.h > %D%/defines.hin
+ $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p; }' < config.h > %D%/defines.hin
$(AM_V_at)$(SHELL) $(srcroot)/move-if-change %D%/defines.hin %D%/defines.h
$(AM_V_at)touch $@
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 04/13] sim/ppc: make defines.h sed command portable
2026-08-17 15:16 ` [PATCH 04/13] sim/ppc: make defines.h sed command portable Simon Marchi
@ 2026-08-17 15:36 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 15:36 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, whose sed is the BSD one, I get:
>
> GEN ppc/stamp-defines
> sed: 1: "/^#define HAVE_.*1$/{ s ...": extra characters at the end of p command
> make[1]: *** [ppc/stamp-defines] Error 1
>
> BSD sed apparently does not accept a `}' directly after another command,
> it needs a separating semicolon. Add one after the `p'. GNU sed
> accepts both forms, and produces the same output either way.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Re-generate sim/Makefile.in.
>
> Change-Id: I0709ad7b0051e08299f2576113b549aba9fc703f
> ---
> sim/Makefile.in | 2 +-
> sim/ppc/local.mk | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sim/Makefile.in b/sim/Makefile.in
> index 1f9bbfec03f1..2a23b5c2eaa9 100644
> --- a/sim/Makefile.in
> +++ b/sim/Makefile.in
> @@ -5722,7 +5722,7 @@ testsuite/common/bits64m63.c: testsuite/common/bits-gen$(EXEEXT) testsuite/commo
>
> @SIM_ENABLE_ARCH_ppc_TRUE@ppc/defines.h: ppc/stamp-defines ; @true
> @SIM_ENABLE_ARCH_ppc_TRUE@ppc/stamp-defines: config.h Makefile
> -@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p }' < config.h > ppc/defines.hin
> +@SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p; }' < config.h > ppc/defines.hin
> @SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_at)$(SHELL) $(srcroot)/move-if-change ppc/defines.hin ppc/defines.h
> @SIM_ENABLE_ARCH_ppc_TRUE@ $(AM_V_at)touch $@
>
> diff --git a/sim/ppc/local.mk b/sim/ppc/local.mk
> index f9f134abe3c9..9aca96465acd 100644
> --- a/sim/ppc/local.mk
> +++ b/sim/ppc/local.mk
> @@ -80,7 +80,7 @@ noinst_PROGRAMS += %D%/run
>
> %D%/defines.h: %D%/stamp-defines ; @true
> %D%/stamp-defines: config.h Makefile
> - $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p }' < config.h > %D%/defines.hin
> + $(AM_V_GEN)$(SED) -n -e '/^#define HAVE_.*1$$/{ s/ 1$$/",/; s/.* HAVE_/"HAVE_/; p; }' < config.h > %D%/defines.hin
> $(AM_V_at)$(SHELL) $(srcroot)/move-if-change %D%/defines.hin %D%/defines.h
> $(AM_V_at)touch $@
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 05/13] sim/m32r: fix unused variable warning on non-Linux hosts
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (3 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 04/13] sim/ppc: make defines.h sed command portable Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 15:36 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 06/13] sim/m32r: fix unused function warnings " Simon Marchi
` (9 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get:
/Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:191:18: error: unused variable 'cb' [-Werror,-Wunused-variable]
191 | host_callback *cb = STATE_CALLBACK (sd);
| ^~
All the uses of `cb' in m32r_trap are inside the TRAP_LINUX_SYSCALL
case, which is guarded by `#ifdef __linux__'. Move the declaration
inside that case, so that it only exists where it is used.
Change-Id: I609850daf7fa60d92856988dffe7e314eb1d8a30
---
sim/m32r/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sim/m32r/traps.c b/sim/m32r/traps.c
index 7b98b2453972..bb82ae80e2aa 100644
--- a/sim/m32r/traps.c
+++ b/sim/m32r/traps.c
@@ -188,7 +188,6 @@ USI
m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
{
SIM_DESC sd = CPU_STATE (current_cpu);
- host_callback *cb = STATE_CALLBACK (sd);
if (STATE_ENVIRONMENT (sd) == OPERATING_ENVIRONMENT)
goto case_default;
@@ -217,6 +216,7 @@ m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
#ifdef __linux__
case TRAP_LINUX_SYSCALL:
{
+ host_callback *cb = STATE_CALLBACK (sd);
CB_SYSCALL s;
unsigned int func, arg1, arg2, arg3, arg4, arg5, arg6, arg7;
int result, errcode;
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 05/13] sim/m32r: fix unused variable warning on non-Linux hosts
2026-08-17 15:16 ` [PATCH 05/13] sim/m32r: fix unused variable warning on non-Linux hosts Simon Marchi
@ 2026-08-17 15:36 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 15:36 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get:
>
> /Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:191:18: error: unused variable 'cb' [-Werror,-Wunused-variable]
> 191 | host_callback *cb = STATE_CALLBACK (sd);
> | ^~
>
> All the uses of `cb' in m32r_trap are inside the TRAP_LINUX_SYSCALL
> case, which is guarded by `#ifdef __linux__'. Move the declaration
> inside that case, so that it only exists where it is used.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: I609850daf7fa60d92856988dffe7e314eb1d8a30
> ---
> sim/m32r/traps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/sim/m32r/traps.c b/sim/m32r/traps.c
> index 7b98b2453972..bb82ae80e2aa 100644
> --- a/sim/m32r/traps.c
> +++ b/sim/m32r/traps.c
> @@ -188,7 +188,6 @@ USI
> m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
> {
> SIM_DESC sd = CPU_STATE (current_cpu);
> - host_callback *cb = STATE_CALLBACK (sd);
>
> if (STATE_ENVIRONMENT (sd) == OPERATING_ENVIRONMENT)
> goto case_default;
> @@ -217,6 +216,7 @@ m32r_trap (SIM_CPU *current_cpu, PCADDR pc, int num)
> #ifdef __linux__
> case TRAP_LINUX_SYSCALL:
> {
> + host_callback *cb = STATE_CALLBACK (sd);
> CB_SYSCALL s;
> unsigned int func, arg1, arg2, arg3, arg4, arg5, arg6, arg7;
> int result, errcode;
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 06/13] sim/m32r: fix unused function warnings on non-Linux hosts
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (4 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 05/13] sim/m32r: fix unused variable warning on non-Linux hosts Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 15:37 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 07/13] gdb/csky: remove uses of sprintf Simon Marchi
` (8 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get:
/Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:141:1: error: unused function 't2h_addr' [-Werror,-Wunused-function]
141 | t2h_addr (host_callback *cb, struct cb_syscall *sc,
| ^~~~~~~~
/Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:158:1: error: unused function 'translate_endian_h2t' [-Werror,-Wunused-function]
158 | translate_endian_h2t (void *addr, size_t size)
| ^~~~~~~~~~~~~~~~~~~~
/Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:171:1: error: unused function 'translate_endian_t2h' [-Werror,-Wunused-function]
171 | translate_endian_t2h (void *addr, size_t size)
| ^~~~~~~~~~~~~~~~~~~~
These three helpers are only called from the TRAP_LINUX_SYSCALL case,
which is guarded by `#ifdef __linux__'. Put them behind the same guard.
Change-Id: I8c8744727eb27abc08231be2a10f8d0de44d7ea6
---
sim/m32r/traps.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/sim/m32r/traps.c b/sim/m32r/traps.c
index bb82ae80e2aa..43a81c915d1c 100644
--- a/sim/m32r/traps.c
+++ b/sim/m32r/traps.c
@@ -134,7 +134,9 @@ m32r_core_signal (SIM_DESC sd, SIM_CPU *current_cpu, sim_cia cia,
sim_core_signal (sd, current_cpu, cia, map, nr_bytes, addr,
transfer, sig);
}
-\f
+
+#ifdef __linux__
+
/* Translate target's address to host's address. */
static void *
@@ -180,6 +182,8 @@ translate_endian_t2h (void *addr, size_t size)
*((unsigned short *) p) = T2H_2 (*((unsigned short *) p));
}
+#endif /* __linux__ */
+
/* Trap support.
The result is the pc address to continue at.
Preprocessing like saving the various registers has already been done. */
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 06/13] sim/m32r: fix unused function warnings on non-Linux hosts
2026-08-17 15:16 ` [PATCH 06/13] sim/m32r: fix unused function warnings " Simon Marchi
@ 2026-08-17 15:37 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 15:37 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get:
>
> /Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:141:1: error: unused function 't2h_addr' [-Werror,-Wunused-function]
> 141 | t2h_addr (host_callback *cb, struct cb_syscall *sc,
> | ^~~~~~~~
> /Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:158:1: error: unused function 'translate_endian_h2t' [-Werror,-Wunused-function]
> 158 | translate_endian_h2t (void *addr, size_t size)
> | ^~~~~~~~~~~~~~~~~~~~
> /Users/smarchi/src/binutils-gdb/sim/m32r/traps.c:171:1: error: unused function 'translate_endian_t2h' [-Werror,-Wunused-function]
> 171 | translate_endian_t2h (void *addr, size_t size)
> | ^~~~~~~~~~~~~~~~~~~~
>
> These three helpers are only called from the TRAP_LINUX_SYSCALL case,
> which is guarded by `#ifdef __linux__'. Put them behind the same
> guard.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: I8c8744727eb27abc08231be2a10f8d0de44d7ea6
> ---
> sim/m32r/traps.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/sim/m32r/traps.c b/sim/m32r/traps.c
> index bb82ae80e2aa..43a81c915d1c 100644
> --- a/sim/m32r/traps.c
> +++ b/sim/m32r/traps.c
> @@ -134,7 +134,9 @@ m32r_core_signal (SIM_DESC sd, SIM_CPU *current_cpu, sim_cia cia,
> sim_core_signal (sd, current_cpu, cia, map, nr_bytes, addr,
> transfer, sig);
> }
> -\f
> +
> +#ifdef __linux__
> +
> /* Translate target's address to host's address. */
>
> static void *
> @@ -180,6 +182,8 @@ translate_endian_t2h (void *addr, size_t size)
> *((unsigned short *) p) = T2H_2 (*((unsigned short *) p));
> }
>
> +#endif /* __linux__ */
> +
> /* Trap support.
> The result is the pc address to continue at.
> Preprocessing like saving the various registers has already been done. */
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 07/13] gdb/csky: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (5 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 06/13] sim/m32r: fix unused function warnings " Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:26 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 08/13] gdb/dwarf2: " Simon Marchi
` (7 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get a few:
/Users/smarchi/src/binutils-gdb/gdb/csky-tdep.c:434:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
434 | sprintf (tdesc_reg.name, "cp1cr%d", remain);
| ^
Replace these uses with snprintf, via xsnprintf, which asserts that the
destination buffer was large enough for the output string.
Change-Id: Idc5c0c42479f767c63b0d0cece5ab14cacec9a60
---
gdb/csky-tdep.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
index e86f79a42eaf..ad0d50d8218d 100644
--- a/gdb/csky-tdep.c
+++ b/gdb/csky-tdep.c
@@ -431,19 +431,22 @@ csky_get_supported_register_by_index (int index)
{
case 0: /* Bank1. */
{
- sprintf (tdesc_reg.name, "cp1cr%d", remain);
+ xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp1cr%d",
+ remain);
tdesc_reg.num = 189 + remain;
}
break;
case 1: /* Bank2. */
{
- sprintf (tdesc_reg.name, "cp2cr%d", remain);
+ xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp2cr%d",
+ remain);
tdesc_reg.num = 276 + remain;
}
break;
case 2: /* Bank3. */
{
- sprintf (tdesc_reg.name, "cp3cr%d", remain);
+ xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp3cr%d",
+ remain);
tdesc_reg.num = 221 + remain;
}
break;
@@ -460,7 +463,8 @@ csky_get_supported_register_by_index (int index)
case 13: /* Bank14. */
{
/* Regitsers in Bank4~14 have continuous regno with start 308. */
- sprintf (tdesc_reg.name, "cp%dcr%d", (multi + 1), remain);
+ xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp%dcr%d",
+ (multi + 1), remain);
tdesc_reg.num = 308 + ((multi - 3) * 32) + remain;
}
break;
@@ -482,7 +486,8 @@ csky_get_supported_register_by_index (int index)
case 29: /* Bank31. */
{
/* Regitsers in Bank16~31 have continuous regno with start 660. */
- sprintf (tdesc_reg.name, "cp%dcr%d", (multi + 2), remain);
+ xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp%dcr%d",
+ (multi + 2), remain);
tdesc_reg.num = 660 + ((multi - 14) * 32) + remain;
}
break;
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 07/13] gdb/csky: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 07/13] gdb/csky: remove uses of sprintf Simon Marchi
@ 2026-08-17 16:26 ` Andrew Burgess
2026-08-17 17:03 ` Simon Marchi
2026-08-17 20:50 ` Tom Tromey
0 siblings, 2 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:26 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get a few:
>
> /Users/smarchi/src/binutils-gdb/gdb/csky-tdep.c:434:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 434 | sprintf (tdesc_reg.name, "cp1cr%d", remain);
> | ^
>
> Replace these uses with snprintf, via xsnprintf, which asserts that the
> destination buffer was large enough for the output string.
>
> Change-Id: Idc5c0c42479f767c63b0d0cece5ab14cacec9a60
> ---
> gdb/csky-tdep.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
> index e86f79a42eaf..ad0d50d8218d 100644
> --- a/gdb/csky-tdep.c
> +++ b/gdb/csky-tdep.c
> @@ -431,19 +431,22 @@ csky_get_supported_register_by_index (int index)
> {
> case 0: /* Bank1. */
> {
> - sprintf (tdesc_reg.name, "cp1cr%d", remain);
> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp1cr%d",
> + remain);
Rather than having to include the size of all these buffers, where the
size is known at compile time, I wondered if we could add something
like:
template<size_t N, typename... Args>
int xsnprintf (char (&buf)[N], const char *format, Args &&...args)
{
return xsnprintf (buf, N, format, std::forward<Args> (args)...);
}
to gdbsupport/common-utils.h. This is fine except that gcc is unable to
track the format literal through the template call, so I think we'd
actually have to do:
template<size_t N, typename... Args>
int xsnprintf (char (&buf)[N], const char *format, Args &&...args)
{
DIAGNOSTIC_PUSH
DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
return xsnprintf (buf, N, format, std::forward<Args> (args)...);
DIAGNOSTIC_POP
}
Which isn't ideal, though we do already have things like this in
gdb/printcmd.c, so maybe it's OK.
The other option would be C varargs style handling:
template<size_t N>
int ATTRIBUTE_PRINTF (2, 3)
xsnprintf (char (&buf)[N], const char *format, ...)
{
va_list args;
va_start (args, format);
int ret = vsnprintf (buf, N, format, args);
gdb_assert (ret < static_cast<int> (N));
va_end (args);
return ret;
}
Or similar. The benefit of this would be that you could then write:
xsnprintf (tdesc_reg.name, "cp1cr%d", remain);
And you'd still get the buffer length check.
Anyway, it was just a thought, not a requirement. The patch as it is
looks fine.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
> tdesc_reg.num = 189 + remain;
> }
> break;
> case 1: /* Bank2. */
> {
> - sprintf (tdesc_reg.name, "cp2cr%d", remain);
> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp2cr%d",
> + remain);
> tdesc_reg.num = 276 + remain;
> }
> break;
> case 2: /* Bank3. */
> {
> - sprintf (tdesc_reg.name, "cp3cr%d", remain);
> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp3cr%d",
> + remain);
> tdesc_reg.num = 221 + remain;
> }
> break;
> @@ -460,7 +463,8 @@ csky_get_supported_register_by_index (int index)
> case 13: /* Bank14. */
> {
> /* Regitsers in Bank4~14 have continuous regno with start 308. */
> - sprintf (tdesc_reg.name, "cp%dcr%d", (multi + 1), remain);
> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp%dcr%d",
> + (multi + 1), remain);
> tdesc_reg.num = 308 + ((multi - 3) * 32) + remain;
> }
> break;
> @@ -482,7 +486,8 @@ csky_get_supported_register_by_index (int index)
> case 29: /* Bank31. */
> {
> /* Regitsers in Bank16~31 have continuous regno with start 660. */
> - sprintf (tdesc_reg.name, "cp%dcr%d", (multi + 2), remain);
> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp%dcr%d",
> + (multi + 2), remain);
> tdesc_reg.num = 660 + ((multi - 14) * 32) + remain;
> }
> break;
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 07/13] gdb/csky: remove uses of sprintf
2026-08-17 16:26 ` Andrew Burgess
@ 2026-08-17 17:03 ` Simon Marchi
2026-08-17 20:50 ` Tom Tromey
1 sibling, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 17:03 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches, binutils
On 8/17/26 12:26 PM, Andrew Burgess wrote:
> Simon Marchi <simon.marchi@efficios.com> writes:
>
>> When building on macOS, I get a few:
>>
>> /Users/smarchi/src/binutils-gdb/gdb/csky-tdep.c:434:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
>> 434 | sprintf (tdesc_reg.name, "cp1cr%d", remain);
>> | ^
>>
>> Replace these uses with snprintf, via xsnprintf, which asserts that the
>> destination buffer was large enough for the output string.
>>
>> Change-Id: Idc5c0c42479f767c63b0d0cece5ab14cacec9a60
>> ---
>> gdb/csky-tdep.c | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/gdb/csky-tdep.c b/gdb/csky-tdep.c
>> index e86f79a42eaf..ad0d50d8218d 100644
>> --- a/gdb/csky-tdep.c
>> +++ b/gdb/csky-tdep.c
>> @@ -431,19 +431,22 @@ csky_get_supported_register_by_index (int index)
>> {
>> case 0: /* Bank1. */
>> {
>> - sprintf (tdesc_reg.name, "cp1cr%d", remain);
>> + xsnprintf (tdesc_reg.name, sizeof (tdesc_reg.name), "cp1cr%d",
>> + remain);
>
> Rather than having to include the size of all these buffers, where the
> size is known at compile time, I wondered if we could add something
> like:
>
> template<size_t N, typename... Args>
> int xsnprintf (char (&buf)[N], const char *format, Args &&...args)
> {
> return xsnprintf (buf, N, format, std::forward<Args> (args)...);
> }
>
> to gdbsupport/common-utils.h. This is fine except that gcc is unable to
> track the format literal through the template call, so I think we'd
> actually have to do:
>
> template<size_t N, typename... Args>
> int xsnprintf (char (&buf)[N], const char *format, Args &&...args)
> {
> DIAGNOSTIC_PUSH
> DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL
> return xsnprintf (buf, N, format, std::forward<Args> (args)...);
> DIAGNOSTIC_POP
> }
>
> Which isn't ideal, though we do already have things like this in
> gdb/printcmd.c, so maybe it's OK.
>
> The other option would be C varargs style handling:
>
> template<size_t N>
> int ATTRIBUTE_PRINTF (2, 3)
> xsnprintf (char (&buf)[N], const char *format, ...)
> {
> va_list args;
> va_start (args, format);
> int ret = vsnprintf (buf, N, format, args);
> gdb_assert (ret < static_cast<int> (N));
> va_end (args);
> return ret;
> }
>
> Or similar. The benefit of this would be that you could then write:
>
> xsnprintf (tdesc_reg.name, "cp1cr%d", remain);
>
> And you'd still get the buffer length check.
>
> Anyway, it was just a thought, not a requirement. The patch as it is
> looks fine.
>
> Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks the the suggestion, I will attempt to do this on top of the
current series.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 07/13] gdb/csky: remove uses of sprintf
2026-08-17 16:26 ` Andrew Burgess
2026-08-17 17:03 ` Simon Marchi
@ 2026-08-17 20:50 ` Tom Tromey
2026-08-18 18:26 ` Simon Marchi
1 sibling, 1 reply; 35+ messages in thread
From: Tom Tromey @ 2026-08-17 20:50 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Simon Marchi, gdb-patches, binutils
>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
Andrew> to gdbsupport/common-utils.h. This is fine except that gcc is unable to
Andrew> track the format literal through the template call, so I think we'd
Andrew> actually have to do:
It might work if instead the 'buf' argument were a gdb::array_view,
which I think can be implicitly constructed from an array.
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 07/13] gdb/csky: remove uses of sprintf
2026-08-17 20:50 ` Tom Tromey
@ 2026-08-18 18:26 ` Simon Marchi
0 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-18 18:26 UTC (permalink / raw)
To: Tom Tromey, Andrew Burgess; +Cc: Simon Marchi, gdb-patches, binutils
On 8/17/26 4:50 PM, Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> to gdbsupport/common-utils.h. This is fine except that gcc is unable to
> Andrew> track the format literal through the template call, so I think we'd
> Andrew> actually have to do:
>
> It might work if instead the 'buf' argument were a gdb::array_view,
> which I think can be implicitly constructed from an array.
I tried changing xsnprintf to use array_view, but it was too big of a
change. I did not think about adding a new overload though, I think
that could work.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 08/13] gdb/dwarf2: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (6 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 07/13] gdb/csky: remove uses of sprintf Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:35 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 09/13] gdb/elfread: remove use " Simon Marchi
` (6 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get some:
/Users/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3773:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
3773 | sprintf (buf, "TU %s at offset %s", hex_string (sig_type->signature),
| ^
Replace them with xsnprintf, which takes the destination size and asserts
that the output was not truncated.
Change-Id: Ie0324f75e5d4aad9b647007848459bf4af5998a6
---
gdb/dwarf2/read.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index ca475f53745d..a8b99425554a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -3770,15 +3770,16 @@ process_queue (dwarf2_per_objfile *per_objfile)
if (signatured_type *sig_type = per_cu->as_signatured_type ();
sig_type != nullptr)
{
- sprintf (buf, "TU %s at offset %s", hex_string (sig_type->signature),
- sect_offset_str (per_cu->sect_off ()));
+ xsnprintf (buf, sizeof (buf), "TU %s at offset %s",
+ hex_string (sig_type->signature),
+ sect_offset_str (per_cu->sect_off ()));
/* There can be 100s of TUs. Only print them in verbose mode. */
debug_print_threshold = 2;
}
else
{
- sprintf (buf, "CU at offset %s",
- sect_offset_str (per_cu->sect_off ()));
+ xsnprintf (buf, sizeof (buf), "CU at offset %s",
+ sect_offset_str (per_cu->sect_off ()));
debug_print_threshold = 1;
}
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 08/13] gdb/dwarf2: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 08/13] gdb/dwarf2: " Simon Marchi
@ 2026-08-17 16:35 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:35 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get some:
>
> /Users/smarchi/src/binutils-gdb/gdb/dwarf2/read.c:3773:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 3773 | sprintf (buf, "TU %s at offset %s", hex_string (sig_type->signature),
> | ^
>
> Replace them with xsnprintf, which takes the destination size and asserts
> that the output was not truncated.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: Ie0324f75e5d4aad9b647007848459bf4af5998a6
> ---
> gdb/dwarf2/read.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index ca475f53745d..a8b99425554a 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -3770,15 +3770,16 @@ process_queue (dwarf2_per_objfile *per_objfile)
> if (signatured_type *sig_type = per_cu->as_signatured_type ();
> sig_type != nullptr)
> {
> - sprintf (buf, "TU %s at offset %s", hex_string (sig_type->signature),
> - sect_offset_str (per_cu->sect_off ()));
> + xsnprintf (buf, sizeof (buf), "TU %s at offset %s",
> + hex_string (sig_type->signature),
> + sect_offset_str (per_cu->sect_off ()));
> /* There can be 100s of TUs. Only print them in verbose mode. */
> debug_print_threshold = 2;
> }
> else
> {
> - sprintf (buf, "CU at offset %s",
> - sect_offset_str (per_cu->sect_off ()));
> + xsnprintf (buf, sizeof (buf), "CU at offset %s",
> + sect_offset_str (per_cu->sect_off ()));
> debug_print_threshold = 1;
> }
>
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 09/13] gdb/elfread: remove use of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (7 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 08/13] gdb/dwarf2: " Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:38 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 10/13] gdbsupport: add xstrcpy Simon Marchi
` (5 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get:
/Users/smarchi/src/binutils-gdb/gdb/elfread.c:813:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
813 | sprintf (name_got_plt, "%s" SYMBOL_GOT_PLT_SUFFIX, name);
| ^
Change this use of sprintf with an std::string, which also allows
getting rid of a use of alloca.
Change-Id: I296e25c863463ef05eca582c8303557570d63cf0
---
gdb/elfread.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/gdb/elfread.c b/gdb/elfread.c
index e3890ae0270c..fcbd0176d08f 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -804,20 +804,17 @@ static int
elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
{
gnu_ifunc_debug_printf ("resolving \"%s\" by GOT", name);
- char *name_got_plt;
- const size_t got_suffix_len = strlen (SYMBOL_GOT_PLT_SUFFIX);
int found = 0;
const char *func = __func__;
- name_got_plt = (char *) alloca (strlen (name) + got_suffix_len + 1);
- sprintf (name_got_plt, "%s" SYMBOL_GOT_PLT_SUFFIX, name);
+ std::string name_got_plt = std::string (name) + SYMBOL_GOT_PLT_SUFFIX;
/* FIXME: we only search the initial namespace.
To search other namespaces, we would need to provide context, e.g. in
form of an objfile in that namespace. */
current_program_space->iterate_over_objfiles_in_search_order
- ([name, name_got_plt, &addr_p, &found, func] (struct objfile *objfile)
+ ([name, &name_got_plt, &addr_p, &found, func] (struct objfile *objfile)
{
bfd *obfd = objfile->obfd.get ();
struct gdbarch *gdbarch = objfile->arch ();
@@ -828,8 +825,8 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
gdb_byte *buf = (gdb_byte *) alloca (ptr_size);
bound_minimal_symbol msym
- = lookup_minimal_symbol (current_program_space, name_got_plt,
- objfile);
+ = lookup_minimal_symbol (current_program_space,
+ name_got_plt.c_str (), objfile);
if (msym.minsym == NULL)
return 0;
if (msym.minsym->type () != mst_slot_got_plt)
@@ -850,7 +847,8 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
addr = gdbarch_addr_bits_remove (gdbarch, addr);
gnu_ifunc_debug_printf_func (func, "GOT entry \"%s\" points to %s",
- name_got_plt, paddress (gdbarch, addr));
+ name_got_plt.c_str (),
+ paddress (gdbarch, addr));
if (elf_gnu_ifunc_record_cache (name, addr))
{
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 09/13] gdb/elfread: remove use of sprintf
2026-08-17 15:16 ` [PATCH 09/13] gdb/elfread: remove use " Simon Marchi
@ 2026-08-17 16:38 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:38 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get:
>
> /Users/smarchi/src/binutils-gdb/gdb/elfread.c:813:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 813 | sprintf (name_got_plt, "%s" SYMBOL_GOT_PLT_SUFFIX, name);
> | ^
>
> Change this use of sprintf with an std::string, which also allows
> getting rid of a use of alloca.
+1 for alloca removal!
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: I296e25c863463ef05eca582c8303557570d63cf0
> ---
> gdb/elfread.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/elfread.c b/gdb/elfread.c
> index e3890ae0270c..fcbd0176d08f 100644
> --- a/gdb/elfread.c
> +++ b/gdb/elfread.c
> @@ -804,20 +804,17 @@ static int
> elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
> {
> gnu_ifunc_debug_printf ("resolving \"%s\" by GOT", name);
> - char *name_got_plt;
> - const size_t got_suffix_len = strlen (SYMBOL_GOT_PLT_SUFFIX);
> int found = 0;
> const char *func = __func__;
>
> - name_got_plt = (char *) alloca (strlen (name) + got_suffix_len + 1);
> - sprintf (name_got_plt, "%s" SYMBOL_GOT_PLT_SUFFIX, name);
> + std::string name_got_plt = std::string (name) + SYMBOL_GOT_PLT_SUFFIX;
>
> /* FIXME: we only search the initial namespace.
>
> To search other namespaces, we would need to provide context, e.g. in
> form of an objfile in that namespace. */
> current_program_space->iterate_over_objfiles_in_search_order
> - ([name, name_got_plt, &addr_p, &found, func] (struct objfile *objfile)
> + ([name, &name_got_plt, &addr_p, &found, func] (struct objfile *objfile)
> {
> bfd *obfd = objfile->obfd.get ();
> struct gdbarch *gdbarch = objfile->arch ();
> @@ -828,8 +825,8 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
> gdb_byte *buf = (gdb_byte *) alloca (ptr_size);
>
> bound_minimal_symbol msym
> - = lookup_minimal_symbol (current_program_space, name_got_plt,
> - objfile);
> + = lookup_minimal_symbol (current_program_space,
> + name_got_plt.c_str (), objfile);
> if (msym.minsym == NULL)
> return 0;
> if (msym.minsym->type () != mst_slot_got_plt)
> @@ -850,7 +847,8 @@ elf_gnu_ifunc_resolve_by_got (const char *name, CORE_ADDR *addr_p)
> addr = gdbarch_addr_bits_remove (gdbarch, addr);
>
> gnu_ifunc_debug_printf_func (func, "GOT entry \"%s\" points to %s",
> - name_got_plt, paddress (gdbarch, addr));
> + name_got_plt.c_str (),
> + paddress (gdbarch, addr));
>
> if (elf_gnu_ifunc_record_cache (name, addr))
> {
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 10/13] gdbsupport: add xstrcpy
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (8 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 09/13] gdb/elfread: remove use " Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:45 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 11/13] gdb/remote-fileio: remove uses of sprintf Simon Marchi
` (4 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
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<char> 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
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 10/13] gdbsupport: add xstrcpy
2026-08-17 15:16 ` [PATCH 10/13] gdbsupport: add xstrcpy Simon Marchi
@ 2026-08-17 16:45 ` Andrew Burgess
2026-08-17 17:30 ` Simon Marchi
0 siblings, 1 reply; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:45 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> 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");
Don't functions that take a buffer size usually include an 'n' in the
name. That seems to be true for libc, but also throughout GDB. Would
it not be a good idea to adopt that here too.
This isn't exactly strncpy, but it seems similar. While strncpy can
result in a non-null terminated output string, this "xstrncpy" asserts
that the source fits into the output buffer without being truncated.
But otherwise, it's the same function I think?
Thanks,
Andrew
>
> 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<char> 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
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 10/13] gdbsupport: add xstrcpy
2026-08-17 16:45 ` Andrew Burgess
@ 2026-08-17 17:30 ` Simon Marchi
0 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 17:30 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches, binutils
On 8/17/26 12:45 PM, Andrew Burgess wrote:
> Simon Marchi <simon.marchi@efficios.com> writes:
>
>> 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");
>
> Don't functions that take a buffer size usually include an 'n' in the
> name. That seems to be true for libc, but also throughout GDB. Would
> it not be a good idea to adopt that here too.
>
> This isn't exactly strncpy, but it seems similar. While strncpy can
> result in a non-null terminated output string, this "xstrncpy" asserts
> that the source fits into the output buffer without being truncated.
> But otherwise, it's the same function I think?
I hesitated about naming it strncpy. I decided against it because the
new function doesn't behave exactly like strcnpy on one specific point:
if the source is smaller than destination, strncpy fills the remainder
of the destination buffer with zeroes. Not sure if that matters in
practice, but it could be the source of a subtle bug if one blindly
switches strncpy for xstrncpy.
Both strcpy and strncpy return a pointer to the beginning of the
destination buffer, which is different than my xstrcpy, which returns
the number of bytes written. The latter seems more useful to me, as
it makes it possible to easily chain the calls.
It seems like xstrcpy is more like a wrapper for strlcpy, so we could
always call it xstrlcpy. The only thing is that the argument order is
not the same:
int xstrcpy (char *dst, size_t size, const char *src);
size_t strlcpy (char *dst, const char *src, size_t size);
So if we named it xstrlcpy, I would want to match xstrlcpy's argument
order, otherwise it's just confusing. Personally, I don't strlcpy's
argument order as much, because `size` describes `dst`, so I like having
it right next to it. But I could live with it.
I just noticed that strcpy_s exists in C11, and it is in the same order
as xstrcpy:
errno_t strcpy_s (char* restrict dest, rsize_t destsz, const char* restrict src);
It looks like we could use that, but we also want to return the number
of bytes written, which this does not provide.
I also considered accepting `dst` as a `gdb::array_view<char>`, which
would side-step the argument order problem, but the caller's are not
really ready for that, so their would look look awkward.
In any case, I still believe that the xstrcpy makes sense, because it is
really like "strcpy, but safe".
Given all this, what would be your choice?
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 11/13] gdb/remote-fileio: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (9 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 10/13] gdbsupport: add xstrcpy Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:53 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 12/13] gdb/remote: " Simon Marchi
` (3 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get some:
/Users/smarchi/src/binutils-gdb/gdb/remote-fileio.c:264:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
264 | sprintf (buf + strlen (buf), "%x", retcode);
| ^
The reply built in remote_fileio_reply is made by appending to a fixed
size buffer, using a mix of strcpy, strcat and sprintf. Replace them
with the safer xsnprintf and xstrcpy. This way, every write is bounds
checked.
Change-Id: I8446a98be5c5fc0eda79ccbc4858d9dddaf2d4d5
---
gdb/remote-fileio.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
index a151161371da..297e3337e2fe 100644
--- a/gdb/remote-fileio.c
+++ b/gdb/remote-fileio.c
@@ -253,28 +253,37 @@ static void
remote_fileio_reply (remote_target *remote, int retcode, int error)
{
char buf[32];
+ char *p = buf;
+ char *const end = buf + sizeof (buf);
bool ctrl_c = check_quit_flag ();
- strcpy (buf, "F");
+ p += xstrcpy (p, end - p, "F");
+
if (retcode < 0)
{
- strcat (buf, "-");
+ p += xstrcpy (p, end - p, "-");
retcode = -retcode;
}
- sprintf (buf + strlen (buf), "%x", retcode);
+
+ p += xsnprintf (p, end - p, "%x", retcode);
+
if (error || ctrl_c)
{
if (error && ctrl_c)
error = FILEIO_EINTR;
+
if (error < 0)
{
- strcat (buf, "-");
+ p += xstrcpy (p, end - p, "-");
error = -error;
}
- sprintf (buf + strlen (buf), ",%x", error);
+
+ p += xsnprintf (p, end - p, ",%x", error);
+
if (ctrl_c)
- strcat (buf, ",C");
+ p += xstrcpy (p, end - p, ",C");
}
+
quit_handler = remote_fileio_o_quit_handler;
putpkt (remote, buf);
}
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 11/13] gdb/remote-fileio: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 11/13] gdb/remote-fileio: remove uses of sprintf Simon Marchi
@ 2026-08-17 16:53 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:53 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get some:
>
> /Users/smarchi/src/binutils-gdb/gdb/remote-fileio.c:264:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 264 | sprintf (buf + strlen (buf), "%x", retcode);
> | ^
>
> The reply built in remote_fileio_reply is made by appending to a fixed
> size buffer, using a mix of strcpy, strcat and sprintf. Replace them
> with the safer xsnprintf and xstrcpy. This way, every write is bounds
> checked.
See previous commit for thoughts on xstrcpy. But otherwise, this looks
fine.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: I8446a98be5c5fc0eda79ccbc4858d9dddaf2d4d5
> ---
> gdb/remote-fileio.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/remote-fileio.c b/gdb/remote-fileio.c
> index a151161371da..297e3337e2fe 100644
> --- a/gdb/remote-fileio.c
> +++ b/gdb/remote-fileio.c
> @@ -253,28 +253,37 @@ static void
> remote_fileio_reply (remote_target *remote, int retcode, int error)
> {
> char buf[32];
> + char *p = buf;
> + char *const end = buf + sizeof (buf);
> bool ctrl_c = check_quit_flag ();
>
> - strcpy (buf, "F");
> + p += xstrcpy (p, end - p, "F");
> +
> if (retcode < 0)
> {
> - strcat (buf, "-");
> + p += xstrcpy (p, end - p, "-");
> retcode = -retcode;
> }
> - sprintf (buf + strlen (buf), "%x", retcode);
> +
> + p += xsnprintf (p, end - p, "%x", retcode);
> +
> if (error || ctrl_c)
> {
> if (error && ctrl_c)
> error = FILEIO_EINTR;
> +
> if (error < 0)
> {
> - strcat (buf, "-");
> + p += xstrcpy (p, end - p, "-");
> error = -error;
> }
> - sprintf (buf + strlen (buf), ",%x", error);
> +
> + p += xsnprintf (p, end - p, ",%x", error);
> +
> if (ctrl_c)
> - strcat (buf, ",C");
> + p += xstrcpy (p, end - p, ",C");
> }
> +
> quit_handler = remote_fileio_o_quit_handler;
> putpkt (remote, buf);
> }
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 12/13] gdb/remote: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (10 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 11/13] gdb/remote-fileio: remove uses of sprintf Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:51 ` Andrew Burgess
2026-08-17 15:16 ` [PATCH 13/13] gdb/tracepoint: " Simon Marchi
` (2 subsequent siblings)
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get some:
/Users/smarchi/src/binutils-gdb/gdb/remote.c:11556:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
11556 | sprintf (buf, ";cmds:%x,", bp_tgt->persist);
| ^
Both are in remote_add_target_side_commands, which unlike the similar
remote_add_target_side_condition, does not receive the end of the
packet, and does not bound its writes. Give it a BUF_END parameter,
and use xsnprintf.
The edits to remote_add_target_side_condition are to keep the two
functions in sync.
Ideally, the pack_hex_byte calls and the `*buf = '\0'` assignments
should also have some bound checks, but that is outside the scope of
this patch.
Change-Id: Ib2e9849d89ebcc9e6275297138a3deb8bf03a7c3
---
gdb/remote.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/gdb/remote.c b/gdb/remote.c
index 3d38a9c7c8c9..fe898013a394 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -11513,10 +11513,11 @@ Remote replied unexpectedly while setting startup-with-shell: %s"),
}
\f
-/* Given a location's target info BP_TGT and the packet buffer BUF, output
- the list of conditions (in agent expression bytecode format), if any, the
- target needs to evaluate. The output is placed into the packet buffer
- started from BUF and ended at BUF_END. */
+/* Given a location's target info BP_TGT and the packet buffer BUF, output the
+ list of conditions (in agent expression bytecode format), if any, the
+ target needs to evaluate.
+
+ The output is appended to the existing content of BUF. */
static int
remote_add_target_side_condition (struct gdbarch *gdbarch,
@@ -11532,35 +11533,42 @@ remote_add_target_side_condition (struct gdbarch *gdbarch,
/* Send conditions to the target. */
for (agent_expr *aexpr : bp_tgt->conditions)
{
- xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
- buf += strlen (buf);
+ buf += xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
+
for (int i = 0; i < aexpr->buf.size (); ++i)
buf = pack_hex_byte (buf, aexpr->buf[i]);
+
*buf = '\0';
}
return 0;
}
+/* Given a location's target info BP_TGT and the packet buffer BUF, output the
+ list of commands (in agent expression bytecode format), if any, the target
+ needs to run when the breakpoint is hit.
+
+ The output is appended to the existing content of BUF. */
+
static void
remote_add_target_side_commands (struct gdbarch *gdbarch,
- struct bp_target_info *bp_tgt, char *buf)
+ struct bp_target_info *bp_tgt, char *buf,
+ char *buf_end)
{
if (bp_tgt->tcommands.empty ())
return;
buf += strlen (buf);
-
- sprintf (buf, ";cmds:%x,", bp_tgt->persist);
- buf += strlen (buf);
+ buf += xsnprintf (buf, buf_end - buf, ";cmds:%x,", bp_tgt->persist);
/* Concatenate all the agent expressions that are commands into the
cmds parameter. */
for (agent_expr *aexpr : bp_tgt->tcommands)
{
- sprintf (buf, "X%x,", (int) aexpr->buf.size ());
- buf += strlen (buf);
+ buf += xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
+
for (int i = 0; i < aexpr->buf.size (); ++i)
buf = pack_hex_byte (buf, aexpr->buf[i]);
+
*buf = '\0';
}
}
@@ -11604,7 +11612,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
if (can_run_breakpoint_commands ())
- remote_add_target_side_commands (gdbarch, bp_tgt, p);
+ remote_add_target_side_commands (gdbarch, bp_tgt, p, endbuf);
putpkt (rs->buf);
getpkt (&rs->buf);
@@ -11912,7 +11920,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
if (can_run_breakpoint_commands ())
- remote_add_target_side_commands (gdbarch, bp_tgt, p);
+ remote_add_target_side_commands (gdbarch, bp_tgt, p, endbuf);
putpkt (rs->buf);
getpkt (&rs->buf);
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 12/13] gdb/remote: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 12/13] gdb/remote: " Simon Marchi
@ 2026-08-17 16:51 ` Andrew Burgess
2026-08-17 17:34 ` Simon Marchi
0 siblings, 1 reply; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:51 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get some:
>
> /Users/smarchi/src/binutils-gdb/gdb/remote.c:11556:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 11556 | sprintf (buf, ";cmds:%x,", bp_tgt->persist);
> | ^
>
> Both are in remote_add_target_side_commands, which unlike the similar
> remote_add_target_side_condition, does not receive the end of the
> packet, and does not bound its writes. Give it a BUF_END parameter,
> and use xsnprintf.
>
> The edits to remote_add_target_side_condition are to keep the two
> functions in sync.
>
> Ideally, the pack_hex_byte calls and the `*buf = '\0'` assignments
> should also have some bound checks, but that is outside the scope of
> this patch.
>
> Change-Id: Ib2e9849d89ebcc9e6275297138a3deb8bf03a7c3
> ---
> gdb/remote.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 3d38a9c7c8c9..fe898013a394 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -11513,10 +11513,11 @@ Remote replied unexpectedly while setting startup-with-shell: %s"),
> }
> \f
>
> -/* Given a location's target info BP_TGT and the packet buffer BUF, output
> - the list of conditions (in agent expression bytecode format), if any, the
> - target needs to evaluate. The output is placed into the packet buffer
> - started from BUF and ended at BUF_END. */
> +/* Given a location's target info BP_TGT and the packet buffer BUF, output the
> + list of conditions (in agent expression bytecode format), if any, the
> + target needs to evaluate.
> +
> + The output is appended to the existing content of BUF. */
>
> static int
> remote_add_target_side_condition (struct gdbarch *gdbarch,
> @@ -11532,35 +11533,42 @@ remote_add_target_side_condition (struct gdbarch *gdbarch,
> /* Send conditions to the target. */
> for (agent_expr *aexpr : bp_tgt->conditions)
> {
> - xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
> - buf += strlen (buf);
> + buf += xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
> +
> for (int i = 0; i < aexpr->buf.size (); ++i)
> buf = pack_hex_byte (buf, aexpr->buf[i]);
> +
I wonder if we should add an assert here either inside, or just after,
the loop, to check that we've not blown past BUF_END? Unless I'm
misunderstanding this, these PACK_HEX_BYTE calls could overrun the
buffer, right?
> *buf = '\0';
> }
> return 0;
> }
>
> +/* Given a location's target info BP_TGT and the packet buffer BUF, output the
> + list of commands (in agent expression bytecode format), if any, the target
> + needs to run when the breakpoint is hit.
> +
> + The output is appended to the existing content of BUF. */
> +
> static void
> remote_add_target_side_commands (struct gdbarch *gdbarch,
> - struct bp_target_info *bp_tgt, char *buf)
> + struct bp_target_info *bp_tgt, char *buf,
> + char *buf_end)
> {
> if (bp_tgt->tcommands.empty ())
> return;
>
> buf += strlen (buf);
> -
> - sprintf (buf, ";cmds:%x,", bp_tgt->persist);
> - buf += strlen (buf);
> + buf += xsnprintf (buf, buf_end - buf, ";cmds:%x,", bp_tgt->persist);
>
> /* Concatenate all the agent expressions that are commands into the
> cmds parameter. */
> for (agent_expr *aexpr : bp_tgt->tcommands)
> {
> - sprintf (buf, "X%x,", (int) aexpr->buf.size ());
> - buf += strlen (buf);
> + buf += xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
> +
> for (int i = 0; i < aexpr->buf.size (); ++i)
> buf = pack_hex_byte (buf, aexpr->buf[i]);
> +
As above for buffer overrun maybe?
Thanks,
Andrew
> *buf = '\0';
> }
> }
> @@ -11604,7 +11612,7 @@ remote_target::insert_breakpoint (struct gdbarch *gdbarch,
> remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
>
> if (can_run_breakpoint_commands ())
> - remote_add_target_side_commands (gdbarch, bp_tgt, p);
> + remote_add_target_side_commands (gdbarch, bp_tgt, p, endbuf);
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> @@ -11912,7 +11920,7 @@ remote_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
> remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
>
> if (can_run_breakpoint_commands ())
> - remote_add_target_side_commands (gdbarch, bp_tgt, p);
> + remote_add_target_side_commands (gdbarch, bp_tgt, p, endbuf);
>
> putpkt (rs->buf);
> getpkt (&rs->buf);
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 12/13] gdb/remote: remove uses of sprintf
2026-08-17 16:51 ` Andrew Burgess
@ 2026-08-17 17:34 ` Simon Marchi
0 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 17:34 UTC (permalink / raw)
To: Andrew Burgess, gdb-patches, binutils
On 8/17/26 12:51 PM, Andrew Burgess wrote:
>> @@ -11532,35 +11533,42 @@ remote_add_target_side_condition (struct gdbarch *gdbarch,
>> /* Send conditions to the target. */
>> for (agent_expr *aexpr : bp_tgt->conditions)
>> {
>> - xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
>> - buf += strlen (buf);
>> + buf += xsnprintf (buf, buf_end - buf, "X%x,", (int) aexpr->buf.size ());
>> +
>> for (int i = 0; i < aexpr->buf.size (); ++i)
>> buf = pack_hex_byte (buf, aexpr->buf[i]);
>> +
>
> I wonder if we should add an assert here either inside, or just after,
> the loop, to check that we've not blown past BUF_END? Unless I'm
> misunderstanding this, these PACK_HEX_BYTE calls could overrun the
> buffer, right?
I noted in the commit message that I didn't do it, because I wanted to
focus just on the *printf calls, but I can always do it as a follow-up.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 13/13] gdb/tracepoint: remove uses of sprintf
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (11 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 12/13] gdb/remote: " Simon Marchi
@ 2026-08-17 15:16 ` Simon Marchi
2026-08-17 16:51 ` Andrew Burgess
2026-08-17 20:52 ` [PATCH 00/13] Fix various warnings when building on macOS Tom Tromey
2026-08-18 18:10 ` Simon Marchi
14 siblings, 1 reply; 35+ messages in thread
From: Simon Marchi @ 2026-08-17 15:16 UTC (permalink / raw)
To: gdb-patches, binutils; +Cc: Simon Marchi
When building on macOS, I get some:
/Users/smarchi/src/binutils-gdb/gdb/tracepoint.c:1196:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
1196 | sprintf (end, "M-1,%s,%lX", phex_nz (m_memranges[i].start, 0),
| ^
Replace them with xsnprintf.
Change-Id: Id3ec76c47e5c0091fa3a028e36063b2115378e7e
---
gdb/tracepoint.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 798bb9a552d8..932a8f557e44 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1164,6 +1164,9 @@ collection_list::stringify ()
gdb_printf ("\n");
if (!m_memranges.empty () && info_verbose)
gdb_printf ("Collecting memranges: \n");
+
+ char *buf_end = temp_buf.data () + temp_buf.size ();
+
for (i = 0, count = 0, end = temp_buf.data ();
i < m_memranges.size (); i++)
{
@@ -1193,11 +1196,12 @@ collection_list::stringify ()
"FFFFFFFF" (or more, depending on sizeof (unsigned)).
Special-case it. */
if (m_memranges[i].type == memrange_absolute)
- sprintf (end, "M-1,%s,%lX", phex_nz (m_memranges[i].start, 0),
- (long) length);
+ xsnprintf (end, buf_end - end, "M-1,%s,%lX",
+ phex_nz (m_memranges[i].start, 0), (long) length);
else
- sprintf (end, "M%X,%s,%lX", m_memranges[i].type,
- phex_nz (m_memranges[i].start, 0), (long) length);
+ xsnprintf (end, buf_end - end, "M%X,%s,%lX",
+ m_memranges[i].type, phex_nz (m_memranges[i].start, 0),
+ (long) length);
}
count += strlen (end);
@@ -1213,7 +1217,9 @@ collection_list::stringify ()
count = 0;
end = temp_buf.data ();
}
- sprintf (end, "X%08X,", (int) m_aexprs[i]->buf.size ());
+
+ xsnprintf (end, buf_end - end, "X%08X,",
+ (int) m_aexprs[i]->buf.size ());
end += 10; /* 'X' + 8 hex digits + ',' */
count += 10;
@@ -2816,11 +2822,14 @@ encode_source_string (int tpnum, ULONGEST addr,
{
if (80 + strlen (srctype) > buf_size)
error (_("Buffer too small for source encoding"));
- sprintf (buf, "%x:%s:%s:%x:%x:",
- tpnum, phex_nz (addr),
- srctype, 0, (int) strlen (src));
+
+ xsnprintf (buf, buf_size, "%x:%s:%s:%x:%x:",
+ tpnum, phex_nz (addr),
+ srctype, 0, (int) strlen (src));
+
if (strlen (buf) + strlen (src) * 2 >= buf_size)
error (_("Source string too long for buffer"));
+
bin2hex ((gdb_byte *) src, buf + strlen (buf), strlen (src));
return -1;
}
--
2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 13/13] gdb/tracepoint: remove uses of sprintf
2026-08-17 15:16 ` [PATCH 13/13] gdb/tracepoint: " Simon Marchi
@ 2026-08-17 16:51 ` Andrew Burgess
0 siblings, 0 replies; 35+ messages in thread
From: Andrew Burgess @ 2026-08-17 16:51 UTC (permalink / raw)
To: Simon Marchi, gdb-patches, binutils; +Cc: Simon Marchi
Simon Marchi <simon.marchi@efficios.com> writes:
> When building on macOS, I get some:
>
> /Users/smarchi/src/binutils-gdb/gdb/tracepoint.c:1196:4: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only. Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
> 1196 | sprintf (end, "M-1,%s,%lX", phex_nz (m_memranges[i].start, 0),
> | ^
>
> Replace them with xsnprintf.
LGTM.
Approved-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: Id3ec76c47e5c0091fa3a028e36063b2115378e7e
> ---
> gdb/tracepoint.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 798bb9a552d8..932a8f557e44 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -1164,6 +1164,9 @@ collection_list::stringify ()
> gdb_printf ("\n");
> if (!m_memranges.empty () && info_verbose)
> gdb_printf ("Collecting memranges: \n");
> +
> + char *buf_end = temp_buf.data () + temp_buf.size ();
> +
> for (i = 0, count = 0, end = temp_buf.data ();
> i < m_memranges.size (); i++)
> {
> @@ -1193,11 +1196,12 @@ collection_list::stringify ()
> "FFFFFFFF" (or more, depending on sizeof (unsigned)).
> Special-case it. */
> if (m_memranges[i].type == memrange_absolute)
> - sprintf (end, "M-1,%s,%lX", phex_nz (m_memranges[i].start, 0),
> - (long) length);
> + xsnprintf (end, buf_end - end, "M-1,%s,%lX",
> + phex_nz (m_memranges[i].start, 0), (long) length);
> else
> - sprintf (end, "M%X,%s,%lX", m_memranges[i].type,
> - phex_nz (m_memranges[i].start, 0), (long) length);
> + xsnprintf (end, buf_end - end, "M%X,%s,%lX",
> + m_memranges[i].type, phex_nz (m_memranges[i].start, 0),
> + (long) length);
> }
>
> count += strlen (end);
> @@ -1213,7 +1217,9 @@ collection_list::stringify ()
> count = 0;
> end = temp_buf.data ();
> }
> - sprintf (end, "X%08X,", (int) m_aexprs[i]->buf.size ());
> +
> + xsnprintf (end, buf_end - end, "X%08X,",
> + (int) m_aexprs[i]->buf.size ());
> end += 10; /* 'X' + 8 hex digits + ',' */
> count += 10;
>
> @@ -2816,11 +2822,14 @@ encode_source_string (int tpnum, ULONGEST addr,
> {
> if (80 + strlen (srctype) > buf_size)
> error (_("Buffer too small for source encoding"));
> - sprintf (buf, "%x:%s:%s:%x:%x:",
> - tpnum, phex_nz (addr),
> - srctype, 0, (int) strlen (src));
> +
> + xsnprintf (buf, buf_size, "%x:%s:%s:%x:%x:",
> + tpnum, phex_nz (addr),
> + srctype, 0, (int) strlen (src));
> +
> if (strlen (buf) + strlen (src) * 2 >= buf_size)
> error (_("Source string too long for buffer"));
> +
> bin2hex ((gdb_byte *) src, buf + strlen (buf), strlen (src));
> return -1;
> }
> --
> 2.55.0
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/13] Fix various warnings when building on macOS
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (12 preceding siblings ...)
2026-08-17 15:16 ` [PATCH 13/13] gdb/tracepoint: " Simon Marchi
@ 2026-08-17 20:52 ` Tom Tromey
2026-08-18 18:10 ` Simon Marchi
14 siblings, 0 replies; 35+ messages in thread
From: Tom Tromey @ 2026-08-17 20:52 UTC (permalink / raw)
To: Simon Marchi; +Cc: gdb-patches, binutils
>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
Simon> I tried a build on macOS with the following configure flags (there is no
Simon> native configuration for macOS/AArch64,
FWIW there was a port, but AFAIK we never heard back about copyright
assignment.
https://inbox.sourceware.org/gdb-patches/20251029200227.91464-1-canacar@imcan.dev/
Tom
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH 00/13] Fix various warnings when building on macOS
2026-08-17 15:16 [PATCH 00/13] Fix various warnings when building on macOS Simon Marchi
` (13 preceding siblings ...)
2026-08-17 20:52 ` [PATCH 00/13] Fix various warnings when building on macOS Tom Tromey
@ 2026-08-18 18:10 ` Simon Marchi
14 siblings, 0 replies; 35+ messages in thread
From: Simon Marchi @ 2026-08-18 18:10 UTC (permalink / raw)
To: gdb-patches, binutils
On 8/17/26 11:16 AM, Simon Marchi wrote:
> I tried a build on macOS with the following configure flags (there is no
> native configuration for macOS/AArch64, so I used an alternative target
> to at least build the generic parts of GDB):
>
> --enable-targets=all --target=x86_64-pc-linux-gnu
>
> This series fixes most of the build errors I encountered, mostly about
> the uses of "dangerous" functions like sprintf.
>
> There one patch in opcodes, which will require the approval from a
> binutils maintainer.
>
> There are 3 patches in sim, which will require the apprival from a sim
> maintainer.
>
> The rest is in GDB, and I would of course appreciate some reviews for
> those too.
>
> There are still some warnings about vfork being deprecated, like:
>
> CXX cli/cli-cmds.o
> /Users/smarchi/src/binutils-gdb/gdb/cli/cli-cmds.c:917:9: error: 'vfork' is deprecated: Use posix_spawn or fork [-Werror,-Wdeprecated-declarations]
> 917 | pid = vfork ();
> | ^
>
> I have not fixed those, as it requires more thought about the
> consequences of changing this on all supported platforms.
I pushed the patches that were Approved-By and that don't involve
xstrcpy, I will then work on addressing the comments on the rest.
Simon
^ permalink raw reply [flat|nested] 35+ messages in thread