From: Paul Floyd Date: Mon, 9 May 2022 20:53:04 +0000 (+0200) Subject: Bug 446754 Improve error codes from alloc functions under memcheck X-Git-Tag: VALGRIND_3_20_0~85 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=978eb7f1ab4ea5990bf08e68a7a417933271e11b;p=thirdparty%2Fvalgrind.git Bug 446754 Improve error codes from alloc functions under memcheck I've made these changes only for FreeBSD and Solaris for the moment. I don't know what should be done on Linux for aligned_alloc/memalign. The current Valgrind code refects the glibc implementation, but not what the documentation says. --- diff --git a/.gitignore b/.gitignore index 78ea5819a6..c7202fab3a 100644 --- a/.gitignore +++ b/.gitignore @@ -1342,6 +1342,7 @@ /memcheck/tests/freebsd/realpathat /memcheck/tests/freebsd/scalar_13_plus /memcheck/tests/freebsd/452275 +/memcheck/tests/freebsd/errno_aligned_allocs # /memcheck/tests/amd64-freebsd /memcheck/tests/amd64-freebsd/*.stderr.diff diff --git a/NEWS b/NEWS index 05eb34d1f6..dac9a1ce2a 100644 --- a/NEWS +++ b/NEWS @@ -35,6 +35,7 @@ bugzilla (https://bugs.kde.org/enter_bug.cgi?product=valgrind) rather than mailing the developers (or mailing lists) directly -- bugs that are not entered into bugzilla tend to get forgotten about or ignored. +446754 Improve error codes from alloc functions under memcheck 452274 memcheck crashes with Assertion 'sci->status.what == SsIdle' failed 452779 Valgrind fails to build on FreeBSD 13.0 with llvm-devel (15.0.0) 453055 shared_timed_mutex drd test fails with "Lock shared failed" message diff --git a/coregrind/m_replacemalloc/vg_replace_malloc.c b/coregrind/m_replacemalloc/vg_replace_malloc.c index 9711b6573a..164c9ed910 100644 --- a/coregrind/m_replacemalloc/vg_replace_malloc.c +++ b/coregrind/m_replacemalloc/vg_replace_malloc.c @@ -193,15 +193,27 @@ static void init(void); if (info.clo_trace_malloc) { \ VALGRIND_INTERNAL_PRINTF(format, ## args ); } -/* Tries to set ERRNO to ENOMEM if possible. - Only implemented for glibc at the moment. -*/ +/* Tries to set ERRNO to ENOMEM/EINVAL if possible. */ #if defined(VGO_linux) extern int *__errno_location (void) __attribute__((weak)); #define SET_ERRNO_ENOMEM if (__errno_location) \ (*__errno_location ()) = VKI_ENOMEM; +#define SET_ERRNO_EINVAL {} +#elif defined(VGO_freebsd) +extern int *__error (void) __attribute__((weak)); +#define SET_ERRNO_ENOMEM if (__error) \ + (*__error ()) = VKI_ENOMEM; +#define SET_ERRNO_EINVAL if (__error) \ + (*__error ()) = VKI_EINVAL; +#elif defined(VGO_solaris) +extern int *___errno (void) __attribute__((weak)); +#define SET_ERRNO_ENOMEM if (___errno) \ + (*___errno ()) = VKI_ENOMEM; +#define SET_ERRNO_EINVAL if (___errno) \ + (*___errno ()) = VKI_EINVAL; #else #define SET_ERRNO_ENOMEM {} +#define SET_ERRNO_EINVAL {} #endif /* Below are new versions of malloc, __builtin_new, free, @@ -1482,6 +1494,7 @@ extern int *__errno_location (void) __attribute__((weak)); \ v = (void*)VALGRIND_NON_SIMD_CALL2( info.tl_memalign, alignment, n ); \ MALLOC_TRACE(" = %p\n", v ); \ + if (!v) SET_ERRNO_ENOMEM; \ return v; \ } @@ -1703,6 +1716,32 @@ extern int *__errno_location (void) __attribute__((weak)); /*---------------------- aligned_alloc ----------------------*/ + /* + * No OS does things the same way. + * + * Linux, the man page claims that the alignment must be a power + * of two and that size should be a multiple of alignment. + * However the only case that returns EINVAL (glibc 2.34) + * is if the alignement is > SIZE_MAX / 2 + 1 + * Also this is just a weak alias for memalign so this wrapper + * has no effect on Linux. + * + * FreeBSD. the man page claims alignment must be a power of 2. + * UB if size is not an integral multiple of alignment. + * The code checks that the alignment is a power of + * 2 and not less than the minumum alignment (1) + * + * Solaris: doesn't seem to exist on 11.3 + * Illumos: invalid if the size is 0, the alignment is 0, the + * alignment is not a multiple of 4 (no power of 2 + * requirement even though the manpage claims is) or the + * alignment is greater than MAX_ALIGN (whatever that is). + * Wrapper function that just calls memalign + * + */ + +#if defined (VGO_linux) + #define ALIGNED_ALLOC(soname, fnname) \ \ void* VG_REPLACE_FUNCTION_EZU(10170,soname,fnname) \ @@ -1725,6 +1764,33 @@ extern int *__errno_location (void) __attribute__((weak)); return mem; \ } +#else + + #define ALIGNED_ALLOC(soname, fnname) \ + \ + void* VG_REPLACE_FUNCTION_EZU(10170,soname,fnname) \ + ( SizeT alignment, SizeT size ); \ + void* VG_REPLACE_FUNCTION_EZU(10170,soname,fnname) \ + ( SizeT alignment, SizeT size ) \ + { \ + void *mem; \ + \ + if (alignment == 0 \ + || size % alignment != 0 \ + || (alignment & (alignment - 1)) != 0) { \ + SET_ERRNO_EINVAL; \ + return 0; \ + } \ + \ + mem = VG_REPLACE_FUNCTION_EZU(10110,VG_Z_LIBC_SONAME,memalign) \ + (alignment, size); \ + \ + if (!mem) SET_ERRNO_ENOMEM; \ + \ + return mem; \ + } +#endif + #if defined(VGO_linux) ALIGNED_ALLOC(VG_Z_LIBC_SONAME, aligned_alloc); ALIGNED_ALLOC(SO_SYN_MALLOC, aligned_alloc); diff --git a/memcheck/tests/freebsd/Makefile.am b/memcheck/tests/freebsd/Makefile.am index f70e822a41..b9dc4a6d76 100644 --- a/memcheck/tests/freebsd/Makefile.am +++ b/memcheck/tests/freebsd/Makefile.am @@ -79,14 +79,16 @@ EXTRA_DIST = \ eventfd1.vgtest \ eventfd1.stderr.exp eventfd1.stdout.exp \ eventfd2.vgtest \ - eventfd2.stderr.exp eventfd2.stdout.exp + eventfd2.stderr.exp eventfd2.stdout.exp \ + errno_aligned_allocs.vgtest \ + errno_aligned_allocs.stderr.exp check_PROGRAMS = \ statfs pdfork_pdkill getfsstat inlinfo inlinfo_nested.so extattr \ sigwait chflags get_set_login revoke scalar capsicum getfh \ linkat scalar_fork scalar_thr_exit scalar_abort2 scalar_pdfork \ scalar_vfork stat file_locking_wait6 utimens access chmod_chown \ - misc get_set_context utimes static_allocs fexecve + misc get_set_context utimes static_allocs fexecve errno_aligned_allocs AM_CFLAGS += $(AM_FLAG_M3264_PRI) AM_CXXFLAGS += $(AM_FLAG_M3264_PRI) diff --git a/memcheck/tests/freebsd/errno_aligned_allocs.c b/memcheck/tests/freebsd/errno_aligned_allocs.c new file mode 100644 index 0000000000..50c4959af6 --- /dev/null +++ b/memcheck/tests/freebsd/errno_aligned_allocs.c @@ -0,0 +1,49 @@ +#include +#if defined(__FreeBSD__) +#include +#endif +#include +#include + +int main(void) +{ + char* p = NULL; + int res; + + // zero alignment + res = posix_memalign((void**)&p, 0, 8); + assert(p == NULL && res == EINVAL); + // non multiple of alignment passes on FreeBSD + //res = posix_memalign((void**)&p, 8, 25); + //assert(p == NULL && res == EINVAL); + // align not multiple of sizeof(void*) + res = posix_memalign((void**)&p, 2, 32); + assert(p == NULL && res == EINVAL); + // align not power of two + res = posix_memalign((void**)&p, 40, 160); + assert(p == NULL && res == EINVAL); + // too big + res = posix_memalign((void**)&p, 16, 1UL<<48); + assert(p == NULL && res == ENOMEM); + errno = 0; + + + // if ever we make this multi-platform, Solaris doesn't support this + // zero size + p = aligned_alloc(0, 8); + assert(p == NULL && errno == EINVAL); + errno = 0; + // non multiple of alignment passes on FreeBSD + //p = aligned_alloc(8, 25); + //assert(p == NULL && errno == EINVAL); + //errno = 0; + // align not power of 2 + p = aligned_alloc(40, 160); + assert(p == NULL && errno == EINVAL); + errno = 0; + // too big + p = aligned_alloc(16, 1UL<<48); + assert(p == NULL && errno == ENOMEM); +} + + diff --git a/memcheck/tests/freebsd/errno_aligned_allocs.stderr.exp b/memcheck/tests/freebsd/errno_aligned_allocs.stderr.exp new file mode 100644 index 0000000000..eb42921c60 --- /dev/null +++ b/memcheck/tests/freebsd/errno_aligned_allocs.stderr.exp @@ -0,0 +1,10 @@ + + +HEAP SUMMARY: + in use at exit: 0 bytes in 0 blocks + total heap usage: 0 allocs, 0 frees, 0 bytes allocated + +For a detailed leak analysis, rerun with: --leak-check=full + +For lists of detected and suppressed errors, rerun with: -s +ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) diff --git a/memcheck/tests/freebsd/errno_aligned_allocs.vgtest b/memcheck/tests/freebsd/errno_aligned_allocs.vgtest new file mode 100644 index 0000000000..c5dc4696e8 --- /dev/null +++ b/memcheck/tests/freebsd/errno_aligned_allocs.vgtest @@ -0,0 +1,3 @@ +prog: errno_aligned_allocs + +