2023-03-03 14:28:52

by Alexander Potapenko

[permalink] [raw]
Subject: Infinite loop in checkpatch.pl

Hi folks,

I've noticed that checkpatch.pl chokes on the following file (also attached):

==================================
$ cat test-checkpatch.txt
diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
@@ -504,6 +504,25 @@ static void
test_memcpy_aligned_to_unaligned2(struct kunit *test)
+ EXPECTATION_NO_REPORT(expect); \
+ volatile uint##size##_t uninit; \
==================================


, getting into an infinite loop in annotate_values().
The following patch helps it to proceed:

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 78cc595b98ce1..01d998b416a51 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2240,8 +2240,13 @@ sub annotate_values {
print "C($1)\n" if ($dbg_values > 1);
}
if (defined $1) {
- $cur = substr($cur, length($1));
- $res .= $type x length($1);
+ if (length($1)) {
+ $cur = substr($cur, length($1));
+ $res .= $type x length($1);
+ } else {
+ $res .= $cur;
+ $cur = "";
+ }
}
}

, but I have no idea how to test it properly.

Could you please take a look?


Thanks,
Alex

--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg


Attachments:
test-checkpatch.txt (276.00 B)

2023-03-03 17:03:13

by Joe Perches

[permalink] [raw]
Subject: Re: Infinite loop in checkpatch.pl

On Fri, 2023-03-03 at 15:28 +0100, Alexander Potapenko wrote:
> Hi folks,
>
> I've noticed that checkpatch.pl chokes on the following file (also attached):
>
> ==================================
> $ cat test-checkpatch.txt
> diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> @@ -504,6 +504,25 @@ static void
> test_memcpy_aligned_to_unaligned2(struct kunit *test)
> + EXPECTATION_NO_REPORT(expect); \
> + volatile uint##size##_t uninit; \

checkpatch isn't a syntax complete c parser. Don't expect to be.


2023-03-03 20:54:30

by Alexander Potapenko

[permalink] [raw]
Subject: Re: Infinite loop in checkpatch.pl

On Fri, Mar 3, 2023 at 5:53 PM Joe Perches <[email protected]> wrote:
>
> On Fri, 2023-03-03 at 15:28 +0100, Alexander Potapenko wrote:
> > Hi folks,
> >
> > I've noticed that checkpatch.pl chokes on the following file (also attached):
> >
> > ==================================
> > $ cat test-checkpatch.txt
> > diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> > @@ -504,6 +504,25 @@ static void
> > test_memcpy_aligned_to_unaligned2(struct kunit *test)
> > + EXPECTATION_NO_REPORT(expect); \
> > + volatile uint##size##_t uninit; \
>
> checkpatch isn't a syntax complete c parser. Don't expect to be.

That's understandable, and I sure don't. But as a user I expect it to
not loop infinitely, and I think that's also reasonable.
The example I gave is not a randomly generated code snippet, but an
excerpt from a valid patch that I sent earlier today:
https://lore.kernel.org/lkml/[email protected]/,
which checkpatch cannot process.


--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Liana Sebastian
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

2023-03-06 00:05:28

by Joe Perches

[permalink] [raw]
Subject: Re: Infinite loop in checkpatch.pl

On Fri, 2023-03-03 at 21:53 +0100, Alexander Potapenko wrote:
> On Fri, Mar 3, 2023 at 5:53 PM Joe Perches <[email protected]> wrote:
> >
> > On Fri, 2023-03-03 at 15:28 +0100, Alexander Potapenko wrote:
> > > Hi folks,
> > >
> > > I've noticed that checkpatch.pl chokes on the following file (also attached):
> > >
> > > ==================================
> > > $ cat test-checkpatch.txt
> > > diff --git a/mm/kmsan/kmsan_test.c b/mm/kmsan/kmsan_test.c
> > > @@ -504,6 +504,25 @@ static void
> > > test_memcpy_aligned_to_unaligned2(struct kunit *test)
> > > + EXPECTATION_NO_REPORT(expect); \
> > > + volatile uint##size##_t uninit; \
> >
> > checkpatch isn't a syntax complete c parser. Don't expect to be.
>
> That's understandable, and I sure don't. But as a user I expect it to
> not loop infinitely, and I think that's also reasonable.
> The example I gave is not a randomly generated code snippet, but an
> excerpt from a valid patch that I sent earlier today:
> https://lore.kernel.org/lkml/[email protected]/,
> which checkpatch cannot process.

I think this may be appropriate instead. Andy W?
---
scripts/checkpatch.pl | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd44d12965c98..3328fb9f6d048 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -999,7 +999,9 @@ if (defined($typedefsfile)) {
}

sub build_types {
- my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)";
+ my $mods = "(?x: \n" . join("|\n ", @modifierList);
+ $mods .= "|(?^: \n" . join("|\n ", @modifierListFile) . "\n)" if ($#modifierListFile >= 0);
+ $mods .= ')';
my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)";
my $Misordered = "(?x: \n" . join("|\n ", @typeListMisordered) . "\n)";
my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)";