Bug 32366 - malloc test failures with GCC 15
Summary: malloc test failures with GCC 15
Status: RESOLVED FIXED
Alias: None
Product: glibc
Classification: Unclassified
Component: malloc (show other bugs)
Version: 2.41
: P2 normal
Target Milestone: 2.41
Assignee: Sam James
URL:
Keywords:
: 32425 (view as bug list)
Depends on:
Blocks:
 
Reported: 2024-11-14 22:28 UTC by Sam James
Modified: 2024-12-17 12:52 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
0001-malloc-pass-fno-malloc-dce-for-certain-tests-BZ-3236.patch (2.13 KB, patch)
2024-12-09 03:44 UTC, Sam James
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sam James 2024-11-14 22:28:54 UTC
With gcc trunk that now optimises out more dead mallocs where they're only used w/ a test for NULL:
```
FAIL: malloc/tst-aligned-alloc
FAIL: malloc/tst-aligned-alloc-malloc-check
FAIL: malloc/tst-aligned-alloc-malloc-hugetlb1
FAIL: malloc/tst-aligned-alloc-malloc-hugetlb2
FAIL: malloc/tst-aligned-alloc-mcheck
FAIL: malloc/tst-aligned-alloc-static
FAIL: malloc/tst-aligned-alloc-static-malloc-hugetlb1
FAIL: malloc/tst-aligned-alloc-static-malloc-hugetlb2
FAIL: malloc/tst-compathooks-on
FAIL: malloc/tst-malloc
FAIL: malloc/tst-malloc-check
FAIL: malloc/tst-malloc-check-malloc-hugetlb1
FAIL: malloc/tst-malloc-check-malloc-hugetlb2
FAIL: malloc/tst-malloc-check-mcheck
FAIL: malloc/tst-malloc-malloc-check
FAIL: malloc/tst-malloc-malloc-hugetlb1
FAIL: malloc/tst-malloc-malloc-hugetlb2
FAIL: malloc/tst-malloc-mcheck
FAIL: malloc/tst-malloc-random
FAIL: malloc/tst-malloc-random-malloc-check
FAIL: malloc/tst-malloc-random-malloc-hugetlb1
FAIL: malloc/tst-malloc-random-malloc-hugetlb2
FAIL: malloc/tst-malloc-random-mcheck
FAIL: malloc/tst-malloc-too-large
FAIL: malloc/tst-malloc-too-large-malloc-check
FAIL: malloc/tst-malloc-too-large-malloc-hugetlb1
FAIL: malloc/tst-malloc-too-large-malloc-hugetlb2
FAIL: malloc/tst-malloc-too-large-mcheck
FAIL: malloc/tst-realloc
FAIL: malloc/tst-realloc-malloc-check
FAIL: malloc/tst-realloc-malloc-hugetlb1
FAIL: malloc/tst-realloc-malloc-hugetlb2
FAIL: malloc/tst-realloc-mcheck
```

(This is with a slightly earlier version of the patch which got committed, I'll update the bug if the results are different, but I don't expect them to be.)

See e.g. https://inbox.sourceware.org/gcc-patches/Zyo-CFHI22yX81nv@kam.mff.cuni.cz/ for discussion.
Comment 1 Sam James 2024-11-14 22:30:51 UTC
honza's suggestion over there:
"""
For things like glibc testuiste there is -fno-builtin-malloc and I think
it should use it (also for other functions we recognize as biultins),
since the tests are very likely not expecting compiler to change function calls
to different one.  Such as printf ("Hello %i",1) to puts ("Hello 1")
"""

We can also do -fno-malloc-dce (new option for this optimisation).
Comment 2 Jan Hubicka 2024-11-15 12:38:22 UTC
I think it would be nice if glibc used -fno-builtin-XXX when testing behaviour of library call XXX and this library call is also a GCC built-in. This would prevent unwanted optimizations at GCC side and will ensure that the test will remain working as intended (testing library call behaviour).

There are also useful tests which can find bugs in GCC optimizations. I know I had bugs in memset/memcpy in inline expansions which were caught by glibc testsuite. So even better would be to separate tests that do test glibc behaviour to overall test that given function call has intended effect.

Honza
Comment 3 Yury Khrustalev 2024-11-29 12:44:44 UTC
Using "-fno-builtin-malloc" doesn't fix all tests thought.

Using "-fno-malloc-dce" results in more compilation errors for some of the Glibc tests:

```
cd ../glibc/malloc/; /path/to/gcc/bin/aarch64-none-linux-gnu-gcc tst-calloc.c \
  -c -std=gnu11 -fgnu89-inline \
  -fno-malloc-dce \
  -g -O2 -Wall -Wwrite-strings -Wundef -Wimplicit-fallthrough -Werror \
  -fmerge-all-constants -frounding-math -fno-stack-protector -fno-common \
  -U_FORTIFY_SOURCE -Wstrict-prototypes -Wold-style-definition -fmath-errno \
  -fPIE -I../include -I/path/to/glibc-build/malloc \
  -I/path/to/glibc-build  -I../sysdeps/unix/sysv/linux/aarch64 \
  -I../sysdeps/aarch64/nptl  -I../sysdeps/unix/sysv/linux/wordsize-64 \
  -I../sysdeps/unix/sysv/linux/include -I../sysdeps/unix/sysv/linux \
  -I../sysdeps/nptl  -I../sysdeps/pthread  -I../sysdeps/gnu \
  -I../sysdeps/unix/inet \
  -I../sysdeps/unix/sysv  -I../sysdeps/unix  -I../sysdeps/posix \
  -I../sysdeps/aarch64/fpu \
  -I../sysdeps/aarch64/multiarch  -I../sysdeps/aarch64 \
  -I../sysdeps/wordsize-64 \
  -I../sysdeps/ieee754/ldbl-128  -I../sysdeps/ieee754/dbl-64  \
  -I../sysdeps/ieee754/flt-32 \
  -I../sysdeps/ieee754  -I../sysdeps/generic \
  -I.. -I../libio -I. -nostdinc \
  -isystem /path/to/gcc/bin/../lib/gcc/aarch64-none-linux-gnu/15.0.0/include \
  -isystem /path/to/gcc/bin/../lib/gcc/aarch64-none-linux-gnu/15.0.0/include-fixed \
  -isystem /path/to/gcc/aarch64-none-linux-gnu/usr/include \
  -D_LIBC_REENTRANT -include /path/to/glibc-build/libc-modules.h \
  -DMODULE_NAME=testsuite -include ../include/libc-symbols.h  -DPIC \
  -DTOP_NAMESPACE=glibc -DTEST_NO_MALLOPT \
  -o /path/to/glibc-build/malloc/tst-calloc.o \
  -MD -MP -MF /path/to/glibc-build/malloc/tst-calloc.o.dt \
  -MT /path/to/glibc-build/malloc/tst-calloc.o; cd -
In function ‘null_test’,
    inlined from ‘do_test’ at tst-calloc.c:126:3,
    inlined from ‘legacy_test_function’ at ../test-skeleton.c:55:10:
tst-calloc.c:105:3: error: argument 2 value ‘18446744073709551615’ 
exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
  105 |   calloc (0, ~((size_t) 0));
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../include/stdlib.h:16,
                 from tst-calloc.c:22:
../stdlib/stdlib.h: In function ‘legacy_test_function’:
../stdlib/stdlib.h:675:14: note: in a call to allocation function ‘calloc’ declared here
  675 | extern void *calloc (size_t __nmemb, size_t __size)
      |              ^~~~~~
In function ‘null_test’,
    inlined from ‘do_test’ at tst-calloc.c:126:3,
    inlined from ‘legacy_test_function’ at ../test-skeleton.c:55:10:
tst-calloc.c:106:3: error: argument 1 value ‘18446744073709551615’
exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
  106 |   calloc (~((size_t) 0), 0);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~
```
Comment 4 Florian Weimer 2024-11-29 15:13:43 UTC
Please try with -fno-builtin-calloc as well.
Comment 5 hubicka 2024-12-01 10:32:39 UTC
note that we also recognize strdup and strndup.  so probably disabling
all those allocation builtins.
Comment 6 Yury Khrustalev 2024-12-02 11:46:03 UTC
> Please try with -fno-builtin-calloc as well.

This breaks compilation of the "malloc/tst-calloc" test in the same way as the "-fno-malloc-dce" option does (see error above).

> so probably disabling all those allocation builtins

As of now I struggle to find the right combination of options that would allow Glibc tests to both compile and run successfully. I have to rollback the 7828dc07051 commit in GCC to make things work.
Comment 7 Andreas Schwab 2024-12-06 22:49:34 UTC
*** Bug 32425 has been marked as a duplicate of this bug. ***
Comment 8 Sam James 2024-12-09 03:26:55 UTC
Mine.
Comment 9 Sam James 2024-12-09 03:44:11 UTC
Created attachment 15826 [details]
0001-malloc-pass-fno-malloc-dce-for-certain-tests-BZ-3236.patch

Attached an untested patch based on top of H.J.'s patch for testing with a different compiler.
Comment 10 Sam James 2024-12-09 23:45:44 UTC
Patch posted: https://inbox.sourceware.org/libc-alpha/8a9a794bdf1fd6435764f3f836b4b3adce36bcfc.1733787863.git.sam@gentoo.org/.

Note that it uses a different approach.
Comment 11 Sam James 2024-12-10 01:54:57 UTC
commit a9944a52c967ce76a5894c30d0274b824df43c7a (HEAD -> master, upstream/master, malloc-tests-3)
Author: Sam James <sam@gentoo.org>
Date:   Mon Dec 9 23:11:25 2024 +0000

    malloc: add indirection for malloc(-like) functions in tests [BZ #32366]

    GCC 15 introduces allocation dead code removal (DCE) for PR117370 in
    r15-5255-g7828dc070510f8. This breaks various glibc tests which want
    to assert various properties of the allocator without doing anything
    obviously useful with the allocated memory.

    Alexander Monakov rightly pointed out that we can and should do better
    than passing -fno-malloc-dce to paper over the problem. Not least because
    GCC 14 already does such DCE where there's no testing of malloc's return
    value against NULL, and LLVM has such optimisations too.

    Handle this by providing malloc (and friends) wrappers with a volatile
    function pointer to obscure that we're calling malloc (et. al) from the
    compiler.

    Reviewed-by: Paul Eggert <eggert@cs.ucla.edu>


Fixed for 2.41. We should probably backport it too. CCing Linaro CI folks as an FYI as well.
Comment 12 Sam James 2024-12-10 03:24:01 UTC
Backported to 2.39+2.40. Others are free to do further back.
Comment 13 Sourceware Commits 2024-12-17 12:52:05 UTC
The master branch has been updated by H.J. Lu <hjl@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=f9493a15ea9cfb63a815c00c23142369ec09d8ce

commit f9493a15ea9cfb63a815c00c23142369ec09d8ce
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Tue Dec 17 18:41:45 2024 +0800

    Hide all malloc functions from compiler [BZ #32366]
    
    Since -1 isn't a power of two, compiler may reject it, hide memalign from
    Clang 19 which issues an error:
    
    tst-memalign.c:86:31: error: requested alignment is not a power of 2 [-Werror,-Wnon-power-of-two-alignment]
       86 |   p = memalign (-1, pagesize);
          |                 ^~
    tst-memalign.c:86:31: error: requested alignment must be 4294967296 bytes or smaller; maximum alignment assumed [-Werror,-Wbuiltin-assume-aligned-alignment]
       86 |   p = memalign (-1, pagesize);
          |                 ^~
    
    Update tst-malloc-aux.h to hide all malloc functions and include it in
    all malloc tests to prevent compiler from optimizing out any malloc
    functions.
    
    Tested with Clang 19.1.5 and GCC 15 20241206 for BZ #32366.
    
    Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
    Reviewed-by: Sam James <sam@gentoo.org>