Bug 474332

Summary: aligned_alloc under Valgrind returns nullptr when alignment is not a multiple of sizeof(void *)
Product: [Developer tools] valgrind Reporter: Stefano Bonicatti <smjert>
Component: memcheckAssignee: Julian Seward <jseward>
Status: RESOLVED FIXED    
Severity: normal CC: pjfloyd
Priority: NOR    
Version First Reported In: 3.22 GIT   
Target Milestone: ---   
Platform: Arch Linux   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: Glibc 2.38 nm output

Description Stefano Bonicatti 2023-09-09 12:39:25 UTC
STEPS TO REPRODUCE
1. Compile and run the following C snippet:

#include <stdlib.h>
#include <stdio.h>

int main() {
    char *p = aligned_alloc(4, 4);

    if(p == NULL) {
        printf("Allocation failed!\n");     
        return 1;
    }

    printf("Allocation succeeded!\n");

    return 0;
}

2. Then run it again under Valgrind

OBSERVED RESULT
When running on its own the allocation succeeds, when running under Valgrind it fails

EXPECTED RESULT

Both cases succeed.

ADDITIONAL INFORMATION

Looking at the source code the problem seems to be highlighted in the comment here (and then the subsequent check); https://sourceware.org/git/?p=valgrind.git;a=blob;f=coregrind/m_replacemalloc/vg_replace_malloc.c;hb=23250889de4e2079ad1ede6874cc824bc9dd92db#l2179

Valgrind expects the alignment to be a multiple of sizeof(void *), but this is not true in glibc https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/malloc.c;hb=a43003ebf674f7af8c4b8d6d1b682244f1a28719#l3548, which only checks that it's a power of two (and then internally adjusts if it's smaller than some constant, but it doesn't make it fail).

That limit seems to be for posix_memalign, but as far as I know the C standard says that a valid alignment is implementation dependent, and I would expect for Valgrind on Linux to be the same as for glibc.
Comment 1 Paul Floyd 2023-09-09 19:44:05 UTC
Exactly which version of GNU libc is this with?

At the time that I made these changes aligned_alloc was just an alias for memalign. And memalign basically did no input validation and just allocated something like the next power of 2 sized block of memory.

GNU libc behaviour changes with this:
https://sourceware.org/pipermail/libc-alpha/2023-March/146383.html

I don't yet have a system using that to test.

Getting back to older GNU libc. On a Debian system with

GNU C Library (Debian GLIBC 2.36-9+deb12u1) stable release version 2.36.

Looking to see what aligned_alloc is exactly:
nm -D /lib/x86_64-linux-gnu/libc.so.6 | grep aligned_alloc
0000000000099450 W aligned_alloc@@GLIBC_2.16

Then looking for that offset in the file:

nm -D /lib/x86_64-linux-gnu/libc.so.6 | grep 0000000000099450
0000000000099450 T __libc_memalign@@GLIBC_2.2.5
0000000000099450 W aligned_alloc@@GLIBC_2.16
0000000000099450 W memalign@@GLIBC_2.2.5

Which means that aligned_alloc is really just a call to memalign.

You testcase works fine on that Debian system.

Could you post the output that you get with nm?
Comment 2 Paul Floyd 2023-09-09 19:48:46 UTC
*** Bug 474339 has been marked as a duplicate of this bug. ***
Comment 3 Stefano Bonicatti 2023-09-09 20:21:47 UTC
Created attachment 161528 [details]
Glibc 2.38 nm output

I'm using glibc 2.38, sorry for not specifying!

I've attached the full output.
Comment 4 Paul Floyd 2023-09-10 13:35:44 UTC
Should be fixed with this. Tested in Debian with an older libc, ArchLinux latest and FreeBSD.

commit ae413f79b24de558ac9ab925babd1fb86e0d21c7 (HEAD -> master, origin/users/paulf/try-bug474332, origin/master, origin/HEAD, bug474332)
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Sun Sep 10 15:05:57 2023 +0200

    Bug 474332 - aligned_alloc under Valgrind returns nullptr when alignment is not a multiple of sizeof(void *)
    
    At configure time use glibc version to set a HAVE flag for C17 aligned_alloc.
    The use the HAVE flag to select which redir macro to use.
    Also make the (normally unused) glibc ALIGNED_ALLOC macro
Comment 5 Stefano Bonicatti 2023-09-10 13:53:07 UTC
(In reply to Paul Floyd from comment #4)
> Should be fixed with this. Tested in Debian with an older libc, ArchLinux
> latest and FreeBSD.
> 
> commit ae413f79b24de558ac9ab925babd1fb86e0d21c7 (HEAD -> master,
> origin/users/paulf/try-bug474332, origin/master, origin/HEAD, bug474332)
> Author: Paul Floyd <pjfloyd@wanadoo.fr>
> Date:   Sun Sep 10 15:05:57 2023 +0200
> 
>     Bug 474332 - aligned_alloc under Valgrind returns nullptr when alignment
> is not a multiple of sizeof(void *)
>     
>     At configure time use glibc version to set a HAVE flag for C17
> aligned_alloc.
>     The use the HAVE flag to select which redir macro to use.
>     Also make the (normally unused) glibc ALIGNED_ALLOC macro

Thanks for such quick turnaround!
A question though, I built the new version and now the behavior of aligned_alloc with an alignment that is not a power of 2 is not consistent with glibc, meaning that Valgrind succeeds and glibc fails.

==68409== Memcheck, a memory error detector
==68409== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==68409== Using Valgrind-3.22.0.GIT and LibVEX; rerun with -h for copyright info
==68409== Command: ./test2
==68409== 
==68409== Invalid alignment value: 3 (should be a power of 2)
==68409==    at 0x4848D47: aligned_alloc (vg_replace_malloc.c:2242)
==68409==    by 0x10915F: main (in /home/smjert/Development/test-valgrind/test2)
==68409== 
Allocation succeeded!

While Valgrind does report that an alignment of 3 is incorrect, shouldn't it also fail the allocation, to not change the program behavior (at least in this case where Valgrind can emulate glibc easily)?

Again this is what glibc does: https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/malloc.c;h=e2f1a615a4fc7b036e188a28de9cfb132b2351df;hb=36f2487f13e3540be9ee0fb51876b1da72176d3f#l3548
Comment 6 Paul Floyd 2023-09-10 14:03:29 UTC
Hmm. I need to check that.

There will be cases where the allocation succeeds but you get an error message. The goal is to warn about anything that is unsafe and non-portable.

That would be the case for older glibc memalign/aligned alloc which allow just about any value for the alignment, Solaris which accepts any multiple of 4 in one case and a few more cases.
Comment 7 Paul Floyd 2023-09-10 15:28:13 UTC
With this:

    char *p = aligned_alloc(3, 4);

I get

standalone:
Allocation failed!

Valgrind 3.21:
Allocation failed!

Valgrind git head:
==1279== Invalid alignment value: 3 (should be a power of 2)
==1279==    at 0x4848D82: aligned_alloc (vg_replace_malloc.c:2242)
==1279==    by 0x10915F: main (aa.c:5)
==1279== 
Allocation failed!

Did you re-run autogeh.sh and configure?
Comment 8 Stefano Bonicatti 2023-09-10 15:40:07 UTC
(In reply to Paul Floyd from comment #7)
> With this:
> 
>     char *p = aligned_alloc(3, 4);
> 
> I get
> 
> standalone:
> Allocation failed!
> 
> Valgrind 3.21:
> Allocation failed!
> 
> Valgrind git head:
> ==1279== Invalid alignment value: 3 (should be a power of 2)
> ==1279==    at 0x4848D82: aligned_alloc (vg_replace_malloc.c:2242)
> ==1279==    by 0x10915F: main (aa.c:5)
> ==1279== 
> Allocation failed!
> 
> Did you re-run autogeh.sh and configure?

I'm a dummy, I confused myself looking at the wrong ALIGNED_ALLOC (the one that received the modification) and missed that I had to re-run autogen. Sorry for the time lost.
Comment 9 Paul Floyd 2023-09-10 16:00:07 UTC
> I'm a dummy, I confused myself looking at the wrong ALIGNED_ALLOC (the one
> that received the modification) and missed that I had to re-run autogen.
> Sorry for the time lost.

No worry. I often make mistakes so the more checks and feedback we get the better.