]> git.ipfire.org Git - thirdparty/valgrind.git/commitdiff
Bug 446754 Improve error codes from alloc functions under memcheck
authorPaul Floyd <pjfloyd@wanadoo.fr>
Mon, 9 May 2022 20:53:04 +0000 (22:53 +0200)
committerPaul Floyd <pjfloyd@wanadoo.fr>
Mon, 9 May 2022 20:57:06 +0000 (22:57 +0200)
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.

.gitignore
NEWS
coregrind/m_replacemalloc/vg_replace_malloc.c
memcheck/tests/freebsd/Makefile.am
memcheck/tests/freebsd/errno_aligned_allocs.c [new file with mode: 0644]
memcheck/tests/freebsd/errno_aligned_allocs.stderr.exp [new file with mode: 0644]
memcheck/tests/freebsd/errno_aligned_allocs.vgtest [new file with mode: 0644]

index 78ea5819a68565eb9d354d721df3069fab4bf51f..c7202fab3a2e495f47ec24596b4bc12099d8b094 100644 (file)
 /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 05eb34d1f6fdaad90195ad336681dc87b8007e18..dac9a1ce2ac6ec995cb5155cd2660d88cc9e6635 100644 (file)
--- 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
index 9711b6573af17cc93e26448c169c4a4f2f3996e3..164c9ed910f84524c4c0d99efb26b1a98e2aa526 100644 (file)
@@ -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);
index f70e822a415888b0612fe013215ab4839a7c6177..b9dc4a6d767ad61b4b0ecf02ff3b24fa2ccb4b4f 100644 (file)
@@ -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 (file)
index 0000000..50c4959
--- /dev/null
@@ -0,0 +1,49 @@
+#include <stdlib.h>
+#if defined(__FreeBSD__)
+#include <malloc_np.h>
+#endif
+#include <assert.h>
+#include <errno.h>
+
+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 (file)
index 0000000..eb42921
--- /dev/null
@@ -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 (file)
index 0000000..c5dc469
--- /dev/null
@@ -0,0 +1,3 @@
+prog: errno_aligned_allocs
+
+