public inbox for linux-s390@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
@ 2026-01-21  8:24 Thomas Richter
  2026-01-21 16:12 ` Ian Rogers
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Richter @ 2026-01-21  8:24 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	irogers
  Cc: agordeev, gor, sumanthk, hca, japo, Thomas Richter

V1 --> V2: Add linux next repository
           s/needed/unneeded/ in commit message

The perf test case 'libsubcmd help tests', subtest
'Exclude disjoint subcmd names' fails all the time.

Root case is a special case of sorted input which the exclude_cmds()
in libsubcmd can not handle. Assume the following inputs:
cmds = { X, Y, Z } and excludes = { A, B }.

This leads to
 ci  cj  ei   cmds-name  excludes
 ----------|--------------------
 0   0   0 |     X         A       :    cmp > 0, ei++
 0   0   1 |     X         B       :    cmp > 0, ei++

At this point, the loop is terminated due to ei == excludes->cnt.
The for-loop now checks for trailing names which had to be deleted.
But the first entry points to a name: cmds->names[0].name == "X"
and this is a valid entry.

This is the case when all commands listed in excludes are less than
all commands listed in cmds.
Only check for existing names when cmds list was changed, that is ci != cj.

Also remove an unneeded if (cmp > 0).

-
Output before:
 # ./perf test -F 68
 68.1: Load subcmd names                           : Ok
 68.2: Uniquify subcmd names                       : Ok
 68.3: Exclude duplicate subcmd names              : Ok
 perf: help.c:112: exclude_cmds: Assertion `cmds->names[ci] == NULL' \
	failed.
 Aborted                    ./perf test -F 68

Output after:
 # ./perf test -F 68
 68.1: Load subcmd names                           : Ok
 68.2: Uniquify subcmd names                       : Ok
 68.3: Exclude duplicate subcmd names              : Ok
 68.4: Exclude disjoint subcmd names               : Ok

Fixes: 1fdf938168c4 ("perf tools: Fix use-after-free in help_unknown_cmd()")
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
---
 tools/lib/subcmd/help.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
index ddaeb4eb3e24..1ce5fe507687 100644
--- a/tools/lib/subcmd/help.c
+++ b/tools/lib/subcmd/help.c
@@ -93,19 +93,19 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
 			zfree(&cmds->names[ci]);
 			ci++;
 			ei++;
-		} else if (cmp > 0) {
+		} else {
 			ei++;
 		}
 	}
-	if (ci != cj) {
+	if (ci != cj) {		/* Verify cmds list only if it changed */
 		while (ci < cmds->cnt) {
 			cmds->names[cj++] = cmds->names[ci];
 			cmds->names[ci++] = NULL;
 		}
+		for (ci = cj; ci < cmds->cnt; ci++)
+			assert(!cmds->names[ci]);
+		cmds->cnt = cj;
 	}
-	for (ci = cj; ci < cmds->cnt; ci++)
-		assert(cmds->names[ci] == NULL);
-	cmds->cnt = cj;
 }
 
 static void get_term_dimensions(struct winsize *ws)
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
  2026-01-21  8:24 [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails Thomas Richter
@ 2026-01-21 16:12 ` Ian Rogers
  2026-01-21 16:14   ` Ian Rogers
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2026-01-21 16:12 UTC (permalink / raw)
  To: Thomas Richter, jayaram
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	agordeev, gor, sumanthk, hca, japo

On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>
> V1 --> V2: Add linux next repository
>            s/needed/unneeded/ in commit message
>
> The perf test case 'libsubcmd help tests', subtest
> 'Exclude disjoint subcmd names' fails all the time.
>
> Root case is a special case of sorted input which the exclude_cmds()
> in libsubcmd can not handle. Assume the following inputs:
> cmds = { X, Y, Z } and excludes = { A, B }.
>
> This leads to
>  ci  cj  ei   cmds-name  excludes
>  ----------|--------------------
>  0   0   0 |     X         A       :    cmp > 0, ei++
>  0   0   1 |     X         B       :    cmp > 0, ei++
>
> At this point, the loop is terminated due to ei == excludes->cnt.
> The for-loop now checks for trailing names which had to be deleted.
> But the first entry points to a name: cmds->names[0].name == "X"
> and this is a valid entry.
>
> This is the case when all commands listed in excludes are less than
> all commands listed in cmds.
> Only check for existing names when cmds list was changed, that is ci != cj.
>
> Also remove an unneeded if (cmp > 0).
>
> -
> Output before:
>  # ./perf test -F 68
>  68.1: Load subcmd names                           : Ok
>  68.2: Uniquify subcmd names                       : Ok
>  68.3: Exclude duplicate subcmd names              : Ok
>  perf: help.c:112: exclude_cmds: Assertion `cmds->names[ci] == NULL' \
>         failed.
>  Aborted                    ./perf test -F 68
>
> Output after:
>  # ./perf test -F 68
>  68.1: Load subcmd names                           : Ok
>  68.2: Uniquify subcmd names                       : Ok
>  68.3: Exclude duplicate subcmd names              : Ok
>  68.4: Exclude disjoint subcmd names               : Ok
>
> Fixes: 1fdf938168c4 ("perf tools: Fix use-after-free in help_unknown_cmd()")
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Ian Rogers <irogers@google.com>
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>

Thanks Thomas!

I tried to apply this on a perf-tools-next tree but it fails. Looking
into the git logs I see on linux-next:
https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/tools/lib/subcmd/help.c
the last patch is:
2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
In perf-tools-next:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/lib/subcmd/help.c?h=tmp.perf-tools-next
I see:
8 days libsubcmd: Fix null intersection case in exclude_cmds()
2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty

The test I wrote was to give coverage for  Sri Jayaramappa's fix:
https://lore.kernel.org/r/20251202213632.2873731-1-sjayaram@akamai.com

I wonder if we've put the test into linux-next but not Sri's fix, well
that's what it looks like to me.

Now that we have both your fix and Sri's fix, and they differ :-) I'm
wondering how to resolve the differences.

Thanks,
Ian

> ---
>  tools/lib/subcmd/help.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> index ddaeb4eb3e24..1ce5fe507687 100644
> --- a/tools/lib/subcmd/help.c
> +++ b/tools/lib/subcmd/help.c
> @@ -93,19 +93,19 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
>                         zfree(&cmds->names[ci]);
>                         ci++;
>                         ei++;
> -               } else if (cmp > 0) {
> +               } else {
>                         ei++;
>                 }
>         }
> -       if (ci != cj) {
> +       if (ci != cj) {         /* Verify cmds list only if it changed */
>                 while (ci < cmds->cnt) {
>                         cmds->names[cj++] = cmds->names[ci];
>                         cmds->names[ci++] = NULL;
>                 }
> +               for (ci = cj; ci < cmds->cnt; ci++)
> +                       assert(!cmds->names[ci]);
> +               cmds->cnt = cj;
>         }
> -       for (ci = cj; ci < cmds->cnt; ci++)
> -               assert(cmds->names[ci] == NULL);
> -       cmds->cnt = cj;
>  }
>
>  static void get_term_dimensions(struct winsize *ws)
> --
> 2.52.0
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
  2026-01-21 16:12 ` Ian Rogers
@ 2026-01-21 16:14   ` Ian Rogers
  2026-01-22 10:48     ` Thomas Richter
  0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2026-01-21 16:14 UTC (permalink / raw)
  To: Thomas Richter, Jayaramappa, Srilakshmi
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	agordeev, gor, sumanthk, hca, japo

On Wed, Jan 21, 2026 at 8:12 AM Ian Rogers <irogers@google.com> wrote:
>
> On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
> >
> > V1 --> V2: Add linux next repository
> >            s/needed/unneeded/ in commit message
> >
> > The perf test case 'libsubcmd help tests', subtest
> > 'Exclude disjoint subcmd names' fails all the time.
> >
> > Root case is a special case of sorted input which the exclude_cmds()
> > in libsubcmd can not handle. Assume the following inputs:
> > cmds = { X, Y, Z } and excludes = { A, B }.
> >
> > This leads to
> >  ci  cj  ei   cmds-name  excludes
> >  ----------|--------------------
> >  0   0   0 |     X         A       :    cmp > 0, ei++
> >  0   0   1 |     X         B       :    cmp > 0, ei++
> >
> > At this point, the loop is terminated due to ei == excludes->cnt.
> > The for-loop now checks for trailing names which had to be deleted.
> > But the first entry points to a name: cmds->names[0].name == "X"
> > and this is a valid entry.
> >
> > This is the case when all commands listed in excludes are less than
> > all commands listed in cmds.
> > Only check for existing names when cmds list was changed, that is ci != cj.
> >
> > Also remove an unneeded if (cmp > 0).
> >
> > -
> > Output before:
> >  # ./perf test -F 68
> >  68.1: Load subcmd names                           : Ok
> >  68.2: Uniquify subcmd names                       : Ok
> >  68.3: Exclude duplicate subcmd names              : Ok
> >  perf: help.c:112: exclude_cmds: Assertion `cmds->names[ci] == NULL' \
> >         failed.
> >  Aborted                    ./perf test -F 68
> >
> > Output after:
> >  # ./perf test -F 68
> >  68.1: Load subcmd names                           : Ok
> >  68.2: Uniquify subcmd names                       : Ok
> >  68.3: Exclude duplicate subcmd names              : Ok
> >  68.4: Exclude disjoint subcmd names               : Ok
> >
> > Fixes: 1fdf938168c4 ("perf tools: Fix use-after-free in help_unknown_cmd()")
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Ian Rogers <irogers@google.com>
> > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>
> Thanks Thomas!
>
> I tried to apply this on a perf-tools-next tree but it fails. Looking
> into the git logs I see on linux-next:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/tools/lib/subcmd/help.c
> the last patch is:
> 2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
> In perf-tools-next:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/lib/subcmd/help.c?h=tmp.perf-tools-next
> I see:
> 8 days libsubcmd: Fix null intersection case in exclude_cmds()
> 2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
>
> The test I wrote was to give coverage for  Sri Jayaramappa's fix:
> https://lore.kernel.org/r/20251202213632.2873731-1-sjayaram@akamai.com
>
> I wonder if we've put the test into linux-next but not Sri's fix, well
> that's what it looks like to me.
>
> Now that we have both your fix and Sri's fix, and they differ :-) I'm
> wondering how to resolve the differences.

Sorry, resending as I got Sri's email address wrong.
Ian

> Thanks,
> Ian
>
> > ---
> >  tools/lib/subcmd/help.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/lib/subcmd/help.c b/tools/lib/subcmd/help.c
> > index ddaeb4eb3e24..1ce5fe507687 100644
> > --- a/tools/lib/subcmd/help.c
> > +++ b/tools/lib/subcmd/help.c
> > @@ -93,19 +93,19 @@ void exclude_cmds(struct cmdnames *cmds, struct cmdnames *excludes)
> >                         zfree(&cmds->names[ci]);
> >                         ci++;
> >                         ei++;
> > -               } else if (cmp > 0) {
> > +               } else {
> >                         ei++;
> >                 }
> >         }
> > -       if (ci != cj) {
> > +       if (ci != cj) {         /* Verify cmds list only if it changed */
> >                 while (ci < cmds->cnt) {
> >                         cmds->names[cj++] = cmds->names[ci];
> >                         cmds->names[ci++] = NULL;
> >                 }
> > +               for (ci = cj; ci < cmds->cnt; ci++)
> > +                       assert(!cmds->names[ci]);
> > +               cmds->cnt = cj;
> >         }
> > -       for (ci = cj; ci < cmds->cnt; ci++)
> > -               assert(cmds->names[ci] == NULL);
> > -       cmds->cnt = cj;
> >  }
> >
> >  static void get_term_dimensions(struct winsize *ws)
> > --
> > 2.52.0
> >

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails
  2026-01-21 16:14   ` Ian Rogers
@ 2026-01-22 10:48     ` Thomas Richter
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Richter @ 2026-01-22 10:48 UTC (permalink / raw)
  To: Ian Rogers, Jayaramappa, Srilakshmi
  Cc: linux-kernel, linux-s390, linux-perf-users, acme, namhyung,
	agordeev, gor, sumanthk, hca, japo

On 1/21/26 17:14, Ian Rogers wrote:
> On Wed, Jan 21, 2026 at 8:12 AM Ian Rogers <irogers@google.com> wrote:
>>
>> On Wed, Jan 21, 2026 at 12:24 AM Thomas Richter <tmricht@linux.ibm.com> wrote:
>>>
>>> V1 --> V2: Add linux next repository
>>>            s/needed/unneeded/ in commit message
>>>
>>> The perf test case 'libsubcmd help tests', subtest
>>> 'Exclude disjoint subcmd names' fails all the time.
>>>
>>> Root case is a special case of sorted input which the exclude_cmds()
>>> in libsubcmd can not handle. Assume the following inputs:
>>> cmds = { X, Y, Z } and excludes = { A, B }.
>>>
>>> This leads to
>>>  ci  cj  ei   cmds-name  excludes
>>>  ----------|--------------------
>>>  0   0   0 |     X         A       :    cmp > 0, ei++
>>>  0   0   1 |     X         B       :    cmp > 0, ei++
>>>
>>> At this point, the loop is terminated due to ei == excludes->cnt.
>>> The for-loop now checks for trailing names which had to be deleted.
>>> But the first entry points to a name: cmds->names[0].name == "X"
>>> and this is a valid entry.
>>>
>>> This is the case when all commands listed in excludes are less than
>>> all commands listed in cmds.
>>> Only check for existing names when cmds list was changed, that is ci != cj.
>>>
>>> Also remove an unneeded if (cmp > 0).
>>>
>>> -
>>> Output before:
>>>  # ./perf test -F 68
>>>  68.1: Load subcmd names                           : Ok
>>>  68.2: Uniquify subcmd names                       : Ok
>>>  68.3: Exclude duplicate subcmd names              : Ok
>>>  perf: help.c:112: exclude_cmds: Assertion `cmds->names[ci] == NULL' \
>>>         failed.
>>>  Aborted                    ./perf test -F 68
>>>
>>> Output after:
>>>  # ./perf test -F 68
>>>  68.1: Load subcmd names                           : Ok
>>>  68.2: Uniquify subcmd names                       : Ok
>>>  68.3: Exclude duplicate subcmd names              : Ok
>>>  68.4: Exclude disjoint subcmd names               : Ok
>>>
>>> Fixes: 1fdf938168c4 ("perf tools: Fix use-after-free in help_unknown_cmd()")
>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>> Cc: Ian Rogers <irogers@google.com>
>>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
>>
>> Thanks Thomas!
>>
>> I tried to apply this on a perf-tools-next tree but it fails. Looking
>> into the git logs I see on linux-next:
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/tools/lib/subcmd/help.c
>> the last patch is:
>> 2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
>> In perf-tools-next:
>> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/log/tools/lib/subcmd/help.c?h=tmp.perf-tools-next
>> I see:
>> 8 days libsubcmd: Fix null intersection case in exclude_cmds()
>> 2025-09-12 perf subcmd: avoid crash in exclude_cmds when excludes is empty
>>
>> The test I wrote was to give coverage for  Sri Jayaramappa's fix:
>> https://lore.kernel.org/r/20251202213632.2873731-1-sjayaram@akamai.com
>>
>> I wonder if we've put the test into linux-next but not Sri's fix, well
>> that's what it looks like to me.
>>
>> Now that we have both your fix and Sri's fix, and they differ :-) I'm
>> wondering how to resolve the differences.
> 
> Sorry, resending as I got Sri's email address wrong.
> Ian
> 
Hi Ian, Jayaramappa,

I have looked at both patches and Jayaramappa's patch is already
in perf-tool-next and solves the issue too. I am fine with this one.
So lets simply drop mine.

Thanks Thomas
-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-01-22 10:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-21  8:24 [PATCH V2 linux-next] perf test: Subtest Exclude disjoint subcmd names fails Thomas Richter
2026-01-21 16:12 ` Ian Rogers
2026-01-21 16:14   ` Ian Rogers
2026-01-22 10:48     ` Thomas Richter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox