All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kris Van Hees <kris.van.hees@oracle.com>
To: Kris Van Hees <kris.van.hees@oracle.com>
Cc: dtrace@lists.linux.dev, dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH v2 5/7] string: ensure string size is handled correctly
Date: Thu, 14 Aug 2025 16:37:53 -0400	[thread overview]
Message-ID: <aJ5JIVdM3rPvtN70@oracle.com> (raw)
In-Reply-To: <SJ0PR10MB56727D85238D96378BB2542EC235A@SJ0PR10MB5672.namprd10.prod.outlook.com>

Apologies - this is v3

On Thu, Aug 14, 2025 at 04:33:37PM -0400, Kris Van Hees via DTrace-devel wrote:
> A string is defined as a character array of size strsize.  Strings
> that are less that strsize in length are terminated by a NUL byte.
> This implies that the NUL byte is part of the array and therefore,
> strsize is indeed the size of the array and not the maximum number
> of characters in the string.
> 
> Tests are adjusted to reflect this behaviour.
> 
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
>  libdtrace/dt_bpf.c                                    |  4 ++--
>  libdtrace/dt_cg.c                                     | 11 +++++------
>  test/unittest/codegen/tst.str_const_length.d          |  2 +-
>  test/unittest/codegen/tst.str_data_size.d             |  2 +-
>  test/unittest/codegen/tst.str_store_var.d             |  2 +-
>  .../funcs/inet_ntoa6/tst.inet_ntoa6.strsize_10.r      |  6 +++---
>  test/unittest/funcs/strjoin/tst.strjoin-bordercases.d |  2 +-
>  .../funcs/strjoin/tst.strjoin-capped-size-2.d         |  2 +-
>  .../funcs/strjoin/tst.strjoin-capped-size-3.d         |  2 +-
>  test/unittest/funcs/strjoin/tst.strjoin-capped-size.d |  2 +-
>  test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d     |  2 +-
>  .../strlen/{tst.capped-sizw.d => tst.capped-size.d}   |  2 +-
>  test/unittest/funcs/strtok/tst.strtok_long.d          |  4 ++--
>  test/unittest/funcs/strtok/tst.strtok_long.r          |  4 ++--
>  test/unittest/funcs/substr/tst.substr-stored-len.d    |  2 +-
>  test/unittest/funcs/substr/tst.substr-strsize.d       |  2 +-
>  test/unittest/funcs/substr/tst.substr_nonDPTR.d       |  2 +-
>  test/unittest/funcs/tst.basename_nonDPTR.d            |  2 +-
>  test/unittest/funcs/tst.inet_ntoa_nonDPTR.d           |  2 +-
>  test/unittest/funcs/tst.lltostr-short.d               |  2 +-
>  test/unittest/variables/tvar/tst.str-size.d           |  2 +-
>  21 files changed, 30 insertions(+), 31 deletions(-)
>  rename test/unittest/funcs/strlen/{tst.capped-sizw.d => tst.capped-size.d} (94%)
> 
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index ddf703ddd..31781ac9f 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -910,8 +910,8 @@ gmap_create_strtab(dtrace_hdl_t *dtp)
>  	while (buf < end) {
>  		uint_t	len = strlen((char *)buf);
>  
> -		if (len > strsize)
> -			buf[strsize] = '\0';
> +		if (len >= strsize)
> +			buf[strsize - 1] = '\0';
>  
>  		buf += len + 1;
>  	}
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 88dddb087..cd9e7f4e9 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1741,11 +1741,10 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>  			dt_cg_check_ptr_arg(dlp, drp, dnp, NULL);
>  
>  		TRACE_REGSET("store_val(): Begin ");
> -		off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size + 1, 1, pfp,
> -				 arg);
> +		off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, 1, pfp, arg);
>  
>  		/*
> -		 * Copy the string data (no more than STRSIZE + 1 bytes) to the
> +		 * Copy the string data (no more than STRSIZE bytes) to the
>  		 * buffer at (%r9 + off).  We depend on the fact that
>  		 * probe_read_str() stops at the terminating NUL byte.
>  		 */
> @@ -1754,7 +1753,7 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>  
>  		emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
>  		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off));
> -		emit(dlp, BPF_MOV_IMM(BPF_REG_2, strsize + 1));
> +		emit(dlp, BPF_MOV_IMM(BPF_REG_2, strsize));
>  		emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
>  		dt_regset_free(drp, dnp->dn_reg);
>  		dt_cg_tstring_free(pcb, dnp);
> @@ -1765,13 +1764,13 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
>  		/*
>  		 * Pad the rest with zeroes, if necessary.
>  		 */
> -		emit(dlp,  BPF_BRANCH_IMM(BPF_JGE, BPF_REG_0, strsize + 1, lbl_ok));
> +		emit(dlp,  BPF_BRANCH_IMM(BPF_JGE, BPF_REG_0, strsize, lbl_ok));
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  		emit(dlp,  BPF_MOV_REG(BPF_REG_1, BPF_REG_9));
>  		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, off));
>  		emit(dlp,  BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_0));
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, strsize + 1));
> +		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, strsize));
>  		emit(dlp,  BPF_ALU64_REG(BPF_SUB, BPF_REG_2, BPF_REG_0));
>  		dt_cg_zerosptr(BPF_REG_3, dlp, drp);
>  		emit(dlp,  BPF_CALL_HELPER(dtp->dt_bpfhelper[BPF_FUNC_probe_read_kernel]));
> diff --git a/test/unittest/codegen/tst.str_const_length.d b/test/unittest/codegen/tst.str_const_length.d
> index 1c58ba13d..bfdd00a2e 100644
> --- a/test/unittest/codegen/tst.str_const_length.d
> +++ b/test/unittest/codegen/tst.str_const_length.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=5
> +#pragma D option strsize=6
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/codegen/tst.str_data_size.d b/test/unittest/codegen/tst.str_data_size.d
> index a928e8299..c93174a4f 100644
> --- a/test/unittest/codegen/tst.str_data_size.d
> +++ b/test/unittest/codegen/tst.str_data_size.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=5
> +#pragma D option strsize=6
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/codegen/tst.str_store_var.d b/test/unittest/codegen/tst.str_store_var.d
> index c14714c01..cfd37404f 100644
> --- a/test/unittest/codegen/tst.str_store_var.d
> +++ b/test/unittest/codegen/tst.str_store_var.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=6
> +#pragma D option strsize=7
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/inet_ntoa6/tst.inet_ntoa6.strsize_10.r b/test/unittest/funcs/inet_ntoa6/tst.inet_ntoa6.strsize_10.r
> index 5e7d49b03..8090daad6 100644
> --- a/test/unittest/funcs/inet_ntoa6/tst.inet_ntoa6.strsize_10.r
> +++ b/test/unittest/funcs/inet_ntoa6/tst.inet_ntoa6.strsize_10.r
> @@ -1,8 +1,8 @@
> -fe80:7060:
> -1080::808:
> +fe80:7060
> +1080::808
>  ::1
>  ::
>  127.0.0.1
>  127.0.0.1
> -::fffe:7f0
> +::fffe:7f
>  
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin-bordercases.d b/test/unittest/funcs/strjoin/tst.strjoin-bordercases.d
> index 253b2d6ef..1ac5ffd12 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin-bordercases.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin-bordercases.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=5
> +#pragma D option strsize=6
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin-capped-size-2.d b/test/unittest/funcs/strjoin/tst.strjoin-capped-size-2.d
> index 625c786b6..44a6acd88 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin-capped-size-2.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin-capped-size-2.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=6
> +#pragma D option strsize=7
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin-capped-size-3.d b/test/unittest/funcs/strjoin/tst.strjoin-capped-size-3.d
> index 52718325a..d2982a70b 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin-capped-size-3.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin-capped-size-3.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=6
> +#pragma D option strsize=7
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin-capped-size.d b/test/unittest/funcs/strjoin/tst.strjoin-capped-size.d
> index fe5b323f7..b54fd2f29 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin-capped-size.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin-capped-size.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option rawbytes
> -#pragma D option strsize=10
> +#pragma D option strsize=11
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d b/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> index a1816f975..2b700a5a8 100644
> --- a/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> +++ b/test/unittest/funcs/strjoin/tst.strjoin_nonDPTR.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=14
> +#pragma D option strsize=15
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/strlen/tst.capped-sizw.d b/test/unittest/funcs/strlen/tst.capped-size.d
> similarity index 94%
> rename from test/unittest/funcs/strlen/tst.capped-sizw.d
> rename to test/unittest/funcs/strlen/tst.capped-size.d
> index 4e473236c..676ed185e 100644
> --- a/test/unittest/funcs/strlen/tst.capped-sizw.d
> +++ b/test/unittest/funcs/strlen/tst.capped-size.d
> @@ -11,7 +11,7 @@
>   * SECTION: Actions and Subroutines/strlen()
>   */
>  
> -#pragma D option strsize=5
> +#pragma D option strsize=6
>  #pragma D option quiet
>  
>  BEGIN
> diff --git a/test/unittest/funcs/strtok/tst.strtok_long.d b/test/unittest/funcs/strtok/tst.strtok_long.d
> index 1fbe415dc..254a65485 100644
> --- a/test/unittest/funcs/strtok/tst.strtok_long.d
> +++ b/test/unittest/funcs/strtok/tst.strtok_long.d
> @@ -9,8 +9,8 @@
>  
>  BEGIN
>  {
> -	/* 256-char string ending in "XYZ" */
> -	x = "_____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XYZ";
> +	/* 256-char string ending in "XYZ" (255 chars + NUL byte)*/
> +	x = "____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XYZ";
>  
>  	/* check whether the last char of a long string is seen */
>  	y = "a";
> diff --git a/test/unittest/funcs/strtok/tst.strtok_long.r b/test/unittest/funcs/strtok/tst.strtok_long.r
> index a752554e8..4b172ca1b 100644
> --- a/test/unittest/funcs/strtok/tst.strtok_long.r
> +++ b/test/unittest/funcs/strtok/tst.strtok_long.r
> @@ -1,5 +1,5 @@
> -_____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XYZ
> +____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XYZ
>  zyxwvutsrqponmlkjihgfedcba
> -_____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XY
> +____________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________XY
>  Z
>  
> diff --git a/test/unittest/funcs/substr/tst.substr-stored-len.d b/test/unittest/funcs/substr/tst.substr-stored-len.d
> index 97d2b0f61..fa3e5337c 100644
> --- a/test/unittest/funcs/substr/tst.substr-stored-len.d
> +++ b/test/unittest/funcs/substr/tst.substr-stored-len.d
> @@ -13,7 +13,7 @@
>  
>  #pragma D option quiet
>  #pragma D option rawbytes
> -#pragma D option strsize=13
> +#pragma D option strsize=14
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/substr/tst.substr-strsize.d b/test/unittest/funcs/substr/tst.substr-strsize.d
> index b4d0cc63e..04eab4297 100644
> --- a/test/unittest/funcs/substr/tst.substr-strsize.d
> +++ b/test/unittest/funcs/substr/tst.substr-strsize.d
> @@ -15,7 +15,7 @@
>  
>  #pragma D option quiet
>  #pragma D option rawbytes
> -#pragma D option strsize=13
> +#pragma D option strsize=14
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/substr/tst.substr_nonDPTR.d b/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> index 9d847ab6f..3e10af08e 100644
> --- a/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> +++ b/test/unittest/funcs/substr/tst.substr_nonDPTR.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=13
> +#pragma D option strsize=14
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/tst.basename_nonDPTR.d b/test/unittest/funcs/tst.basename_nonDPTR.d
> index 4f93685f7..cecef827b 100644
> --- a/test/unittest/funcs/tst.basename_nonDPTR.d
> +++ b/test/unittest/funcs/tst.basename_nonDPTR.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=14
> +#pragma D option strsize=15
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> index 500c53e86..f079c7ef4 100644
> --- a/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> +++ b/test/unittest/funcs/tst.inet_ntoa_nonDPTR.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=14
> +#pragma D option strsize=15
>  
>  BEGIN
>  {
> diff --git a/test/unittest/funcs/tst.lltostr-short.d b/test/unittest/funcs/tst.lltostr-short.d
> index 969045667..65a5714c2 100644
> --- a/test/unittest/funcs/tst.lltostr-short.d
> +++ b/test/unittest/funcs/tst.lltostr-short.d
> @@ -6,7 +6,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=7
> +#pragma D option strsize=8
>  
>  BEGIN
>  {
> diff --git a/test/unittest/variables/tvar/tst.str-size.d b/test/unittest/variables/tvar/tst.str-size.d
> index b4ab6ea2b..bd8aac132 100644
> --- a/test/unittest/variables/tvar/tst.str-size.d
> +++ b/test/unittest/variables/tvar/tst.str-size.d
> @@ -12,7 +12,7 @@
>   */
>  
>  #pragma D option quiet
> -#pragma D option strsize=4
> +#pragma D option strsize=5
>  
>  BEGIN
>  {
> -- 
> 2.45.2
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

  reply	other threads:[~2025-08-14 20:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-14 20:33 [PATCH v2 5/7] string: ensure string size is handled correctly Kris Van Hees
2025-08-14 20:37 ` Kris Van Hees [this message]
2025-08-14 21:49   ` [DTrace-devel] " Eugene Loh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJ5JIVdM3rPvtN70@oracle.com \
    --to=kris.van.hees@oracle.com \
    --cc=dtrace-devel@oss.oracle.com \
    --cc=dtrace@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.