Re: [PATCH] selftests/damon: Fix unnecessary compilation warnings

Re: [PATCH] selftests/damon: Fix unnecessary compilation warnings

[SeongJae Park]

Cc-ing Yuanchu Xie.

Hi Rong,

On Thu, 10 Nov 2022 21:34:18 +0800 Rong Tao <rtoax@foxmail.com> wrote:

> From: Rong Tao <rongtao@cestc.cn>
> 
> When testing overflow and overread, there is no need to keep unnecessary
> compilation warnings, we should simply ignore them.
> 
> How to reproduce the problem:
> 
> $ make -C tools/testing/selftests/
> gcc     huge_count_read_write.c  -o /home/sd/Git/linux/tools/testing/selftests/damon/huge_count_read_write

'checkpatch.pl' complains:

    WARNING: use relative pathname instead of absolute in changelog text
    #20:
    gcc     huge_count_read_write.c  -o /home/sd/Git/linux/tools/testing/selftests/damon/huge_count_read_write

Also, could we add four spaces indent for code snippet/command outputs like
above?

> huge_count_read_write.c: In function ‘write_read_with_huge_count’:
> huge_count_read_write.c:23:9: warning: ‘write’ reading 4294967295 bytes from a region of size 1 [-Wstringop-overread]
>    23 |         write(filedesc, "", 0xfffffffful);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from huge_count_read_write.c:8:
> /usr/include/unistd.h:378:16: note: in a call to function ‘write’ declared with attribute ‘access (read_only, 2, 3)’
>   378 | extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
>       |                ^~~~~
> huge_count_read_write.c:25:15: warning: ‘read’ writing 4294967295 bytes into a region of size 25 overflows the destination [-Wstringop-overflow=]
>    25 |         ret = read(filedesc, buf, 0xfffffffful);
>       |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> huge_count_read_write.c:14:14: note: destination object ‘buf’ of size 25
>    14 |         char buf[25];
>       |              ^~~
> In file included from huge_count_read_write.c:8:
> /usr/include/unistd.h:371:16: note: in a call to function ‘read’ declared with attribute ‘access (write_only, 2, 3)’
>   371 | extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
>       |                ^~~~
> 
> Signed-off-by: Rong Tao <rongtao@cestc.cn>
> ---
>  tools/testing/selftests/damon/huge_count_read_write.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/testing/selftests/damon/huge_count_read_write.c b/tools/testing/selftests/damon/huge_count_read_write.c
> index ad7a6b4cf338..8fbe276870e7 100644
> --- a/tools/testing/selftests/damon/huge_count_read_write.c
> +++ b/tools/testing/selftests/damon/huge_count_read_write.c
> @@ -8,6 +8,11 @@
>  #include <unistd.h>
>  #include <stdio.h>
>  
> +#pragma GCC diagnostic push
> +/* Ignore read(2) overflow and write(2) overread compile warnings */
> +#pragma GCC diagnostic ignored "-Wstringop-overread"
> +#pragma GCC diagnostic ignored "-Wstringop-overflow"
> +

Thank you for sending this patch!

However, there was a similar patch from Yuanchu[1], and this causes another
warning for old gcc[2] that I use (9.4.0), like below.

    gcc -Wno-stringop-overread -Wno-stringop-overflow    huge_count_read_write.c  -o /home/sjpark/linux/tools/testing/selftests/damon/huge_count_read_write
    huge_count_read_write.c:13:32: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas]
       13 | #pragma GCC diagnostic ignored "-Wstringop-overread"
          |                                ^~~~~~~~~~~~~~~~~~~~~
    cc1: warning: unrecognized command line option ‘-Wno-stringop-overread’

As mentioned as a reply to Yuanchu's patch, I'd slightly prefer making it
silent for both new and old compilers than this approach, but no strong opinion
from my side.  Yuanchu and Shuah, do you have some opinion?

[1] https://lore.kernel.org/lkml/CAJj2-QE4ee=N9wYXVQc6gyZYC3zgAsWVwWJ7DMaS2B9q2WqBHw@mail.gmail.com/
[2] https://lore.kernel.org/lkml/20220504184537.130085-1-sj@kernel.org/
[3] https://lore.kernel.org/lkml/20220517160417.1096-1-sj@kernel.org/


Thanks,
SJ

>  void write_read_with_huge_count(char *file)
>  {
>  	int filedesc = open(file, O_RDWR);
> @@ -27,6 +32,8 @@ void write_read_with_huge_count(char *file)
>  	close(filedesc);
>  }
>  
> +#pragma GCC diagnostic pop
> +
>  int main(int argc, char *argv[])
>  {
>  	if (argc != 2) {
> -- 
> 2.31.1
> 
> 

[PATCH] selftests/damon: Fix unnecessary compilation warnings

[Rong Tao]

From: Rong Tao <rongtao@cestc.cn>

When testing overflow and overread, there is no need to keep unnecessary
compilation warnings, we should simply ignore them.

How to reproduce the problem:

    $ make -C tools/testing/selftests/
    ...
    warning: ‘write’ reading 4294967295 bytes from a region of size 1
    [-Wstringop-overread]
    warning: ‘read’ writing 4294967295 bytes into a region of size 25
    overflows the destination [-Wstringop-overflow=]

Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
 tools/testing/selftests/damon/huge_count_read_write.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/testing/selftests/damon/huge_count_read_write.c b/tools/testing/selftests/damon/huge_count_read_write.c
index ad7a6b4cf338..8fbe276870e7 100644
--- a/tools/testing/selftests/damon/huge_count_read_write.c
+++ b/tools/testing/selftests/damon/huge_count_read_write.c
@@ -8,6 +8,11 @@
 #include <unistd.h>
 #include <stdio.h>
 
+#pragma GCC diagnostic push
+/* Ignore read(2) overflow and write(2) overread compile warnings */
+#pragma GCC diagnostic ignored "-Wstringop-overread"
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+
 void write_read_with_huge_count(char *file)
 {
 	int filedesc = open(file, O_RDWR);
@@ -27,6 +32,8 @@ void write_read_with_huge_count(char *file)
 	close(filedesc);
 }
 
+#pragma GCC diagnostic pop
+
 int main(int argc, char *argv[])
 {
 	if (argc != 2) {
-- 
2.31.1

Re: [PATCH] selftests/damon: Fix unnecessary compilation warnings

[SeongJae Park]

Hi Rong,


It would be better if you could notice the version of this patch using 'PATCH
v2' like subject prefix from next time.

On Fri, 11 Nov 2022 10:37:20 +0800 Rong Tao <rtoax@foxmail.com> wrote:

> From: Rong Tao <rongtao@cestc.cn>
> 
> When testing overflow and overread, there is no need to keep unnecessary
> compilation warnings, we should simply ignore them.
> 
> How to reproduce the problem:
> 
>     $ make -C tools/testing/selftests/
>     ...
>     warning: ‘write’ reading 4294967295 bytes from a region of size 1
>     [-Wstringop-overread]
>     warning: ‘read’ writing 4294967295 bytes into a region of size 25
>     overflows the destination [-Wstringop-overflow=]

Thank you for indenting as I suggested!  BTW, I'm ok to violate the line length
limit for quoting commands outputs.

> 
> Signed-off-by: Rong Tao <rongtao@cestc.cn>
> ---
>  tools/testing/selftests/damon/huge_count_read_write.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tools/testing/selftests/damon/huge_count_read_write.c b/tools/testing/selftests/damon/huge_count_read_write.c
> index ad7a6b4cf338..8fbe276870e7 100644
> --- a/tools/testing/selftests/damon/huge_count_read_write.c
> +++ b/tools/testing/selftests/damon/huge_count_read_write.c
> @@ -8,6 +8,11 @@
>  #include <unistd.h>
>  #include <stdio.h>
>  
> +#pragma GCC diagnostic push
> +/* Ignore read(2) overflow and write(2) overread compile warnings */
> +#pragma GCC diagnostic ignored "-Wstringop-overread"

This still trigger below error on my old gcc.

    gcc -Wno-stringop-overread -Wno-stringop-overflow    huge_count_read_write.c  -o /home/sjpark/linux/tools/testing/selftests/damon/huge_count_read_write
    huge_count_read_write.c:13:32: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas]
       13 | #pragma GCC diagnostic ignored "-Wstringop-overread"
          |                                ^~~~~~~~~~~~~~~~~~~~~
    cc1: warning: unrecognized command line option ‘-Wno-stringop-overread’

I don't think that's a blocker of this patch, but hope to hear your opinion.


Thanks,
SJ

> +#pragma GCC diagnostic ignored "-Wstringop-overflow"
> +
>  void write_read_with_huge_count(char *file)
>  {
>  	int filedesc = open(file, O_RDWR);
> @@ -27,6 +32,8 @@ void write_read_with_huge_count(char *file)
>  	close(filedesc);
>  }
>  
> +#pragma GCC diagnostic pop
> +
>  int main(int argc, char *argv[])
>  {
>  	if (argc != 2) {
> -- 
> 2.31.1
> 

Re: Re: [PATCH] selftests/damon: Fix unnecessary compilation warnings

[Rong Tao]

Hi, Park, I just search on GCC source code, found GCC support
"-Wstringop-overread" at least gcc-11.1.0, commit d14c547abd48("Add
-Wstringop-overread for reading past the end by string functions.").

AND found a testsuite gcc/gcc/testsuite/gcc.dg/pragma-diag-10.c

 10 #pragma GCC diagnostic push
 11 #pragma GCC diagnostic ignored "-Wstringop-overflow"
 12 #pragma GCC diagnostic ignored "-Wstringop-overread"
 13   if (c != 0)
 14     return __builtin_memchr (s, c, (unsigned long)-1);
 15 #pragma GCC diagnostic pop

it's totally same as this PATCH.

I think the motivation for this patch is to eliminate the compilation
warning, maybe one day we will compile the kernel with "-Werror -Wall",
at which point this compilation warning will turn into a compilation
error, and in case we already know it, we should fix this error in
advance.

For old gcc, we can add this?

 #pragma GCC diagnostic push
+#if __GNUC__ >= 11 && __GNUC_MINOR__ >= 1
 /* Ignore read(2) overflow and write(2) overread compile warnings */
 #pragma GCC diagnostic ignored "-Wstringop-overread"
 #pragma GCC diagnostic ignored "-Wstringop-overflow"
+#endif

What do you think?

Good day!
Rong Tao

Re: Re: [PATCH] selftests/damon: Fix unnecessary compilation warnings

[SeongJae Park]

Hi Rong,

On Sat, 12 Nov 2022 13:01:04 +0800 Rong Tao <rtoax@foxmail.com> wrote:

> Hi, Park, I just search on GCC source code, found GCC support
> "-Wstringop-overread" at least gcc-11.1.0, commit d14c547abd48("Add
> -Wstringop-overread for reading past the end by string functions.").
> 
> AND found a testsuite gcc/gcc/testsuite/gcc.dg/pragma-diag-10.c
> 
>  10 #pragma GCC diagnostic push
>  11 #pragma GCC diagnostic ignored "-Wstringop-overflow"
>  12 #pragma GCC diagnostic ignored "-Wstringop-overread"
>  13   if (c != 0)
>  14     return __builtin_memchr (s, c, (unsigned long)-1);
>  15 #pragma GCC diagnostic pop
> 
> it's totally same as this PATCH.
> 
> I think the motivation for this patch is to eliminate the compilation
> warning, maybe one day we will compile the kernel with "-Werror -Wall",
> at which point this compilation warning will turn into a compilation
> error, and in case we already know it, we should fix this error in
> advance.
> 
> For old gcc, we can add this?
> 
>  #pragma GCC diagnostic push
> +#if __GNUC__ >= 11 && __GNUC_MINOR__ >= 1
>  /* Ignore read(2) overflow and write(2) overread compile warnings */
>  #pragma GCC diagnostic ignored "-Wstringop-overread"
>  #pragma GCC diagnostic ignored "-Wstringop-overflow"
> +#endif
> 
> What do you think?

I think it looks great!  Looking forward to your v3 patch!


Thanks,
SJ

> 
> Good day!
> Rong Tao

[PATCH v3] selftests/damon: Fix unnecessary compilation warnings

[Rong Tao]

From: Rong Tao <rongtao@cestc.cn>

When testing overflow and overread, there is no need to keep unnecessary
compilation warnings, we should simply ignore them.

The motivation for this patch is to eliminate the compilation warning,
maybe one day we will compile the kernel with "-Werror -Wall", at which
point this compilation warning will turn into a compilation error, we
should fix this error in advance.

How to reproduce the problem (with gcc-11.3.1):

    $ make -C tools/testing/selftests/
    ...
    warning: ‘write’ reading 4294967295 bytes from a region of size 1
    [-Wstringop-overread]
    warning: ‘read’ writing 4294967295 bytes into a region of size 25
    overflows the destination [-Wstringop-overflow=]

"-Wno-stringop-overread" is supported at least in gcc-11.1.0.

Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=d14c547abd484d3540b692bb8048c4a6efe92c8b
Signed-off-by: Rong Tao <rongtao@cestc.cn>
---
 tools/testing/selftests/damon/huge_count_read_write.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/damon/huge_count_read_write.c b/tools/testing/selftests/damon/huge_count_read_write.c
index ad7a6b4cf338..a6fe0689f88d 100644
--- a/tools/testing/selftests/damon/huge_count_read_write.c
+++ b/tools/testing/selftests/damon/huge_count_read_write.c
@@ -8,6 +8,13 @@
 #include <unistd.h>
 #include <stdio.h>
 
+#pragma GCC diagnostic push
+#if __GNUC__ >= 11 && __GNUC_MINOR__ >= 1
+/* Ignore read(2) overflow and write(2) overread compile warnings */
+#pragma GCC diagnostic ignored "-Wstringop-overread"
+#pragma GCC diagnostic ignored "-Wstringop-overflow"
+#endif
+
 void write_read_with_huge_count(char *file)
 {
 	int filedesc = open(file, O_RDWR);
@@ -27,6 +34,8 @@ void write_read_with_huge_count(char *file)
 	close(filedesc);
 }
 
+#pragma GCC diagnostic pop
+
 int main(int argc, char *argv[])
 {
 	if (argc != 2) {
-- 
2.31.1

Re: [PATCH v3] selftests/damon: Fix unnecessary compilation warnings

[SeongJae Park]

On Sun, 13 Nov 2022 08:38:45 +0800 Rong Tao <rtoax@foxmail.com> wrote:

> From: Rong Tao <rongtao@cestc.cn>
> 
> When testing overflow and overread, there is no need to keep unnecessary
> compilation warnings, we should simply ignore them.
> 
> The motivation for this patch is to eliminate the compilation warning,
> maybe one day we will compile the kernel with "-Werror -Wall", at which
> point this compilation warning will turn into a compilation error, we
> should fix this error in advance.
> 
> How to reproduce the problem (with gcc-11.3.1):
> 
>     $ make -C tools/testing/selftests/
>     ...
>     warning: ‘write’ reading 4294967295 bytes from a region of size 1
>     [-Wstringop-overread]
>     warning: ‘read’ writing 4294967295 bytes into a region of size 25
>     overflows the destination [-Wstringop-overflow=]
> 
> "-Wno-stringop-overread" is supported at least in gcc-11.1.0.
> 
> Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=d14c547abd484d3540b692bb8048c4a6efe92c8b
> Signed-off-by: Rong Tao <rongtao@cestc.cn>

Reviewed-by: SeongJae Park <sj@kernel.org>

Thank you for fixing this old problem!


Thanks,
SJ

> ---
>  tools/testing/selftests/damon/huge_count_read_write.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/testing/selftests/damon/huge_count_read_write.c b/tools/testing/selftests/damon/huge_count_read_write.c
> index ad7a6b4cf338..a6fe0689f88d 100644
> --- a/tools/testing/selftests/damon/huge_count_read_write.c
> +++ b/tools/testing/selftests/damon/huge_count_read_write.c
> @@ -8,6 +8,13 @@
>  #include <unistd.h>
>  #include <stdio.h>
>  
> +#pragma GCC diagnostic push
> +#if __GNUC__ >= 11 && __GNUC_MINOR__ >= 1
> +/* Ignore read(2) overflow and write(2) overread compile warnings */
> +#pragma GCC diagnostic ignored "-Wstringop-overread"
> +#pragma GCC diagnostic ignored "-Wstringop-overflow"
> +#endif
> +
>  void write_read_with_huge_count(char *file)
>  {
>  	int filedesc = open(file, O_RDWR);
> @@ -27,6 +34,8 @@ void write_read_with_huge_count(char *file)
>  	close(filedesc);
>  }
>  
> +#pragma GCC diagnostic pop
> +
>  int main(int argc, char *argv[])
>  {
>  	if (argc != 2) {
> -- 
> 2.31.1
> 
>