On 10/11/21 6:56 AM, Ammar Faizi wrote: > On Mon, Oct 11, 2021 at 7:13 PM Jens Axboe <axboe@xxxxxxxxx> wrote: >> On 10/11/21 12:49 AM, Ammar Faizi wrote: >>> Add `__attribute__((__aligned__))` to the `user_p` to guarantee >>> pointer returned by the `malloc()` is properly aligned for user. >>> >>> This attribute asks the compiler to align a type to the maximum >>> useful alignment for the target machine we are compiling for, >>> which is often, but by no means always, 8 or 16 bytes [1]. >>> >>> Link: https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Common-Variable-Attributes.html#Common-Variable-Attributes [1] >>> Fixes: https://github.com/axboe/liburing/issues/454 >>> Reported-by: Louvian Lyndal <louvianlyndal@xxxxxxxxx> >>> Signed-off-by: Ammar Faizi <ammar.faizi@xxxxxxxxxxxxxxxxxxxxx> >>> --- >>> src/nolibc.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/nolibc.c b/src/nolibc.c >>> index 5582ca0..251780b 100644 >>> --- a/src/nolibc.c >>> +++ b/src/nolibc.c >>> @@ -20,7 +20,7 @@ void *memset(void *s, int c, size_t n) >>> >>> struct uring_heap { >>> size_t len; >>> - char user_p[]; >>> + char user_p[] __attribute__((__aligned__)); >>> }; >> >> This seems to over-align for me, at 16 bytes where 8 bytes would be fine. >> What guarantees does malloc() give? >> > > Section 7.20.3 of C99 states this about `malloc()`: > __The pointer returned if the allocation succeeds is suitably aligned > so that it may be assigned to a pointer to any type of object.__ > > I have just browsed the glibc source code, malloc does give us 16 bytes > alignment guarantee on x86-64. > > https://code.woboq.org/userspace/glibc/sysdeps/generic/malloc-alignment.h.html#_M/MALLOC_ALIGNMENT > > Lookie here on Linux x86-64... > > ``` > ammarfaizi2@integral:/tmp$ cat > test.c > #include <stdio.h> > int main(void) > { > printf("alignof = %zu\n", __alignof__(long double)); > return 0; > } > ammarfaizi2@integral:/tmp$ gcc -o test test.c > ammarfaizi2@integral:/tmp$ ./test > alignof = 16 > ammarfaizi2@integral:/tmp$ > ``` > > We have `long double` which requires 16 byte alignment. So `malloc()` > should cover this. Although we don't use floating point in liburing, > it's probably better to have this guarantee as well? Ah yes, good point. FWIW, I did apply your patch previously, for alignment it's always better to error on the side of caution. -- Jens Axboe
