Bug 446754 - Improve error codes from alloc functions under memcheck
Summary: Improve error codes from alloc functions under memcheck
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.19 GIT
Platform: Compiled Sources FreeBSD
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-09 21:15 UTC by Paul Floyd
Modified: 2022-05-09 20:59 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
more errno for allocs (5.83 KB, patch)
2021-12-09 21:15 UTC, Paul Floyd
Details
Updated patch (7.03 KB, patch)
2021-12-11 17:13 UTC, Paul Floyd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2021-12-09 21:15:59 UTC
Created attachment 144408 [details]
more errno for allocs

This commit

commit 1c9a0bf58a47e855e6e5bf78a30bcee0af835804
Author: Mark Wielaard <mark@klomp.org>
Date:   Fri Feb 12 23:29:34 2021 +0100

    PR217695 malloc/calloc/realloc/memalign failure doesn't set errno to ENOMEM

started adding some errno functionality to Linux.

The attached patch
a) covers a couple more functions
b) adds EINVAL for aligned variants
c) adds FreeBSD and Solaris

Not yet tested on Linux or Solaris.
Comment 1 Paul Floyd 2021-12-10 09:18:25 UTC
On Linux aligned_alloc accepts an alignment of 0 and an alignment of 40 (i.e.,  not a power of 2). So the man page is wrong. In fact everything that the man page says about errors concerning alignment/size is wrong.

Looking here

https://elixir.bootlin.com/glibc/latest/source/malloc/malloc.c#L3416

If the alignment is less than MALLOC_ALIGNMENT (which is 16 for i386 and the max(2*sizeof(size_t), sizeof(long double)) on other platforms) then it just calls the malloc implementation, no error.

Not in the man page but in the source there is a check that alignment is not over half the max value of size_t. That causes EINVAL.

Lastly if the alignment is not a power of 2 it gets bumped up to the next power of 2, no error.

There is no check that size is a multiple of alignment.

What a nightmare.
Comment 2 Tom Hughes 2021-12-10 09:38:03 UTC
I think that's only true for memalign (which the manual page says is obsolete) and aligned_alloc (which appears to be an alias for memalign even the manual page says it has an extra restriction) though, and posix_memalign does enforce power of two and a multiple of sizeof(void *) as the manual page says:

https://elixir.bootlin.com/glibc/latest/source/malloc/malloc.c#L5553
Comment 3 Paul Floyd 2021-12-10 10:09:10 UTC
Yes, posix_memalign looks a lot better.

For aligned_alloc I need to dig through the jemalloc code (used on FreeBSD). It's much more factorized which makes it fairly hard to follow.
Comment 4 Paul Floyd 2021-12-11 17:13:50 UTC
Created attachment 144458 [details]
Updated patch

Added as much as possible to the comment and added Solaris.

The big question is should I keep the conditon that size % align == 0 ? The man pages seem to say it is not allowed / UB but none complain or set EINVAL.
Comment 5 Paul Floyd 2021-12-11 17:41:49 UTC
Also either we need to make the same checks in memalign as in aligned_alloc or else disable the aligned_alloc tests for Linux in the testcase
Comment 6 Paul Floyd 2022-05-09 20:59:51 UTC
I pushed the changes, but only for FreeBSD and Solaris.

I'll leave the Linux decision to Mark.