Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.95k stars 554 forks source link

op.c: recent change triggers two new build-time warnings #22614

Closed jkeenan closed 2 weeks ago

jkeenan commented 1 month ago

When compiling blead (or perl-5.41.4) with clang-14 on linux, I am observing two previously unobserved build-time warnings. Both are coming from a recently modified line in op.c. One occurs during the compilation of opmini.c; the other during compilation of op.c. From output of make test_prep:

   1 echo @`sh  cflags "optimize='-O2'" opmini.o`  -DPERL_IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   2 @clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/inclu     de -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=     vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -DPERL     _IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   3 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
   4     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
   5                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   6 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
   7 #define MAX(a,b) (((a)>(b))?(a):(b))
   8                     ~ ^ ~
   9 1 warning generated.
...
 498 clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/includ     e -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=v     la -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings op.c
 499 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
 500     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
 501                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 502 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
 503 #define MAX(a,b) (((a)>(b))?(a):(b))
 504                     ~ ^ ~
 505 1 warning generated.

I haven't done a full bisection, but the line in question was most recently modified in 8038d05097.

commit 8038d05097cbe358d957e77fa7fb1c96cdf0a900
Author:     Karl Williamson <khw@cpan.org>
AuthorDate: Fri Aug 16 11:52:49 2024 -0600
Commit:     Karl Williamson <khw@cpan.org>
CommitDate: Mon Sep 2 10:47:07 2024 -0600

    op.c: Improve invmap_dump output

@khwilliamson, can you take a look? Thanks.

Leont commented 1 month ago

I would also like to note that MAX evaluates the higher of its arguments twice, which may be less than ideal for that TOTAL_LEN(array[upper] - 1)

jkeenan commented 3 weeks ago

When compiling blead (or perl-5.41.4) with clang-14 on linux, I am observing two previously unobserved build-time warnings. Both are coming from a recently modified line in op.c. One occurs during the compilation of opmini.c; the other during compilation of op.c. From output of make test_prep:

   1 echo @`sh  cflags "optimize='-O2'" opmini.o`  -DPERL_IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   2 @clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/inclu     de -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=     vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -DPERL     _IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   3 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
   4     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
   5                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   6 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
   7 #define MAX(a,b) (((a)>(b))?(a):(b))
   8                     ~ ^ ~
   9 1 warning generated.
...
 498 clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/includ     e -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=v     la -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings op.c
 499 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
 500     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
 501                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 502 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
 503 #define MAX(a,b) (((a)>(b))?(a):(b))
 504                     ~ ^ ~
 505 1 warning generated.

I haven't done a full bisection, but the line in question was most recently modified in 8038d05.

commit 8038d05097cbe358d957e77fa7fb1c96cdf0a900
Author:     Karl Williamson <khw@cpan.org>
AuthorDate: Fri Aug 16 11:52:49 2024 -0600
Commit:     Karl Williamson <khw@cpan.org>
CommitDate: Mon Sep 2 10:47:07 2024 -0600

    op.c: Improve invmap_dump output

@khwilliamson, can you take a look? Thanks.

@khwilliamson, have you had a chance to look at this? Thx.

sisyphus commented 3 weeks ago

I'm seeing essentially the same warning on MSWindows with perl built using 64-bit gcc-14 - except that the comparison there is between unsigned long long int and int

jkeenan commented 2 weeks ago

This patch clears up the warnings I was seeing on Linux with clang-14. @sisyphus, does it work for you on win32? @Leont, if the patch also meets your concerns, it's good to be applied.

jkeenan commented 2 weeks ago

This patch clears up the warnings I was seeing on Linux with clang-14. @sisyphus, does it work for you on win32? @Leont, if the patch also meets your concerns, it's good to be applied.

Posted in incorrect ticket.

tonycoz commented 2 weeks ago

Fixed by #22637 which lacked the incorrect magic to automatically mark this ticket closed.

Checked by looking at the build logs on macos before and after the commit (which is clang)