Adapt malloc.c to croak/warn now being defined under multiplicity #20286

[Leont]

This was broken by 13e5ba4

This should fix #20286

[jkeenan]

On FreeBSD-12, using clang10 as the compiler, I built blead with -Dusethreads -Dusemymalloc as suggested. I got 2 -Wmacro-redefined warnings.

$ report-build-warnings a62862a6bb.freebsd.threaded.mymalloc.maketp.output.txt.gz 
File:  a62862a6bb.freebsd.threaded.mymalloc.maketp.output.txt.gz

  Wmacro-redefined                           2
  Wunused-variable                           1

(The -Wunused-variable warning came from a different commit, for which I will open a ticket.)

I then built with the same configuration on the leont/malloc-croak branch.

$ report-build-warnings 7cf9d8b7fa.freebsd.threaded.mymalloc.leont.maketp.output.txt.gz 
File:  7cf9d8b7fa.freebsd.threaded.mymalloc.leont.maketp.output.txt.gz

  Wunused-variable                           1

Branch clears build-time warnings. LGTM.

[tonycoz]

Interestingly (and not related to this commit) with gcc I get this warning:

malloc.c: In function ‘Perl_malloc’:
malloc.c:463:57: warning: array subscript 23 is above array bounds of ‘const short unsigned int[14]’ [-Warray-bounds]
  463 | #  define BUCKET_SIZE_NO_SURPLUS(i) ((i) % 2 ? buck_size[i] : (1 << ((i) >> BUCKET_POW2_SHIFT)))
      |                                                ~~~~~~~~~^~~
malloc.c:1454:32: note: in expansion of macro ‘BUCKET_SIZE_NO_SURPLUS’
 1454 |             add_to_chain(ret, (BUCKET_SIZE_NO_SURPLUS(bucket) +
      |                                ^~~~~~~~~~~~~~~~~~~~~~
malloc.c:459:22: note: while referencing ‘buck_size’
  459 | static const u_short buck_size[MAX_BUCKET_BY_TABLE + 1] =
      |                      ^~~~~~~~~

Configured with ./Configure -des -Dusedevel -Dusemymalloc on x86_64 Debian

[jkeenan]

Interestingly (and not related to this commit) with gcc I get this warning:

Me too.

On blead:

$ uname -mrs
Linux 5.10.0-16-amd64 x86_64

$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110

$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dusethreads -Dusemymalloc';

$ report-build-warnings 658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz 
File:  658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz

  Warray-bounds                              1
  Wunused-result                             2
  Wunused-variable                           1

$ parse-build-warnings 658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz 
File:  658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz

[
  {
    char   => 31,
    group  => "Wunused-result",
    line   => 277,
    source => "malloc.c",
    text   => "ignoring return value of \xE2\x80\x98write\xE2\x80\x99 declared with attribute \xE2\x80\x98warn_unused_result\xE2\x80\x99",
  },
  {
    char   => 31,
    group  => "Wunused-result",
    line   => 277,
    source => "malloc.c",
    text   => "ignoring return value of \xE2\x80\x98write\xE2\x80\x99 declared with attribute \xE2\x80\x98warn_unused_result\xE2\x80\x99",
  },
  {
    char   => 57,
    group  => "Warray-bounds",
    line   => 467,
    source => "malloc.c",
    text   => "array subscript 23 is above array bounds of \xE2\x80\x98const short unsigned int[14]\xE2\x80\x99",
  },
  {
    char   => 14,
    group  => "Wunused-variable",
    line   => 12662,
    source => "toke.c",
    text   => "unused variable \xE2\x80\x98syntax_error\xE2\x80\x99",
  },
]

For the -Wunused-variable warning, see #20302.

But, much to my surprise, I get exactly the same warnings with the same configuration switches in the leont/malloc-croak branch as in blead!

Since, up until now, I have never captured or analyzed the output of -Dusemymalloc builds for build-time warnings, I don't have a point of comparison. More research is needed -- but we don't yet have any evidence that Leon's branch should not be merged.

[jkeenan]

Interestingly (and not related to this commit) with gcc I get this warning:

Me too.

On blead:

$ uname -mrs
Linux 5.10.0-16-amd64 x86_64

$ gcc --version
gcc (Debian 10.2.1-6) 10.2.1 20210110

$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dusethreads -Dusemymalloc';

$ report-build-warnings 658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz 
File:  658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz

  Warray-bounds                              1
  Wunused-result                             2
  Wunused-variable                           1

$ parse-build-warnings 658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz 
File:  658a5bedcf.linux.threaded.mymalloc.maketp.output.txt.gz

[
  {
    char   => 31,
    group  => "Wunused-result",
    line   => 277,
    source => "malloc.c",
    text   => "ignoring return value of \xE2\x80\x98write\xE2\x80\x99 declared with attribute \xE2\x80\x98warn_unused_result\xE2\x80\x99",
  },
  {
    char   => 31,
    group  => "Wunused-result",
    line   => 277,
    source => "malloc.c",
    text   => "ignoring return value of \xE2\x80\x98write\xE2\x80\x99 declared with attribute \xE2\x80\x98warn_unused_result\xE2\x80\x99",
  },
  {
    char   => 57,
    group  => "Warray-bounds",
    line   => 467,
    source => "malloc.c",
    text   => "array subscript 23 is above array bounds of \xE2\x80\x98const short unsigned int[14]\xE2\x80\x99",
  },
  {
    char   => 14,
    group  => "Wunused-variable",
    line   => 12662,
    source => "toke.c",
    text   => "unused variable \xE2\x80\x98syntax_error\xE2\x80\x99",
  },
]

For the -Wunused-variable warning, see #20302.

But, much to my surprise, I get exactly the same warnings with the same configuration switches in the leont/malloc-croak branch as in blead!

Since, up until now, I have never captured or analyzed the output of -Dusemymalloc builds for build-time warnings, I don't have a point of comparison. More research is needed -- but we don't yet have any evidence that Leon's branch should not be merged.

Research (all on Linux with gcc-10, -Dusethreads -Dusemymalloc):

• At v5.30.0: build fails.
• At v5.32.0: build succeeds, but with the 3 build-time warnings in malloc.c reported previously. During the 5.31 dev cycle there was a lot of work done on malloc.c by Ryan Voots and Karl Williamson. This got that build working (again?), but the warnings were probably introduced around then as well.

[Leont]

Interestingly (and not related to this commit) with gcc I get this warning

Yeah, I was planning to look at that later (at the moment I'm seeing quite a few warnings in blead with gcc 12.1.1)

[demerphq]

@Leont any reason not to merge this? It seems stalled.

[jkeenan]

Interestingly (and not related to this commit) with gcc I get this warning

Yeah, I was planning to look at that later (at the moment I'm seeing quite a few warnings in blead with gcc 12.1.1)

@Leont could we get an update on the status of this merge request? Thanks.

[jkeenan]

Interestingly (and not related to this commit) with gcc I get this warning

Yeah, I was planning to look at that later (at the moment I'm seeing quite a few warnings in blead with gcc 12.1.1)

@Leont could we get an update on the status of this merge request? Thanks.

@Leont, do you still want to proceed with this merge request? Thanks.

[Leont]

@Leont, do you still want to proceed with this merge request? Thanks.

I forgot about this.

I've rebased and compiled using clang-16, and seeing now warnings so I'm going ahead and merge this.