Bug 466105 - aligned_alloc problems, part 2
Summary: aligned_alloc problems, part 2
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.21 GIT
Platform: Other All
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-19 21:27 UTC by Paul Floyd
Modified: 2023-09-11 09:52 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2023-02-19 21:27:42 UTC
We don't warn when invalid arguments are used for alligned allocation functions.

For instance

memcheck/tests/memalign2_asan
=================================================================
==54416==ERROR: AddressSanitizer: invalid alignment requested in posix_memalign: -1, alignment must be a power of two and a multiple of sizeof(void*) == 8 (thread T0)
    #0 0x28e397 in posix_memalign /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:210:3
    #1 0x2b7a46 in main /home/paulf/scratch/valgrind/memcheck/tests/memalign2.c:123:10
    #2 0x23533f in _start /usr/src/lib/csu/amd64/crt1_c.c:75:7
    #3 0x8002dd007  (<unknown module>)

==54416==HINT: if you don't care about these errors you may set allocator_may_return_null=1
SUMMARY: AddressSanitizer: invalid-posix-memalign-alignment /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:210:3 in posix_memalign
==54416==ABORTING
Comment 1 Paul Floyd 2023-03-10 14:39:00 UTC
Were next for this and for https://bugs.kde.org/show_bug.cgi?id=433857 ?

I think that I'd like to avoid moving all of the aligment checking code into all of the replacement functions (unless I can refactor everything into one place). The advantage of that, though, would be checking directly within memcheck.

The alternative is to do something like the memcheck overlap mechanism. Roughly this is to wrap vg_replace_strmem.c with mc_replace_strmem.c with the one difference that the mc_ version defines the RECORD_OVERLAP_ERROR macro before including vg_replace_strmem.c

So if I do that I'd need a mc_replace_malloc.c that defines a macro like

#define VERIFY_ALIGNMENT(alignment)                  \
  VALGRIND_DO_CLIENT_REQUEST_STMT(                              \
                  _VG_USERREQ__MEMCHECK_VERIFY_ALIGNMENT   \
                  alignment, 0, 0, 0, 0)

back in vg_replace_malloc.c

#if !defined(VERIFY_ALIGNMENT)
#define VERIFY_ALIGNMENT(alignment) do { } while (0)
#endif

and then

#define MEMALIGN(soname, fnname) \
   \
   void* VG_REPLACE_FUNCTION_EZU(10110,soname,fnname) \
            ( SizeT alignment, SizeT n ); \
   void* VG_REPLACE_FUNCTION_EZU(10110,soname,fnname) \
            ( SizeT alignment, SizeT n )  \
   { \
      void* v; \
      \
      DO_INIT; \
      TRIGGER_MEMCHECK_ERROR_IF_UNDEFINED(n); \
      VERIFY_ALIGNMENT(alignment); \

and then of course also add the check, error output, suppression output, suppression reader, manual change ...
Comment 2 Paul Floyd 2023-09-11 09:52:05 UTC
Forgot to close this.