Bug 433857 - Add validation to C++17 aligned new/delete alignment size
Summary: Add validation to C++17 aligned new/delete alignment size
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Paul Floyd
URL:
Keywords:
Depends on:
Blocks: 467441
  Show dependency treegraph
 
Reported: 2021-03-02 12:53 UTC by Paul Floyd
Modified: 2023-09-11 09:54 UTC (History)
0 users

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 2021-03-02 12:53:26 UTC
According to

https://en.cppreference.com/w/cpp/memory/new/operator_new

a) these operators should only be called when the alignment is greater than __STDCPP_DEFAULT_NEW_ALIGNMENT__ (the contrary would either mean a compiler bug or an explicit call to the operator with an alignment that it too small)

b) "The behavior is undefined if this is not a valid alignment value "

c) I suppose that the alignment size passed to new should match that passed to delete.

For point b) the wording of the C++ 17 standard is

"6.6.5 Alignment
 
4 Alignments are represented as values of the type std::size_t. Valid alignments include only those values
returned by an alignof expression for the fundamental types plus an additional implementation-defined set
of values, which may be empty. Every alignment value shall be a non-negative integral power of two."

For point b) I suggest just checking that the alignment is a power of 2.

Point c) looks more complicated, and I'll look at that another time.
Comment 1 Paul Floyd 2021-03-04 07:33:53 UTC
The value of __STDCPP_DEFAULT_NEW_ALIGNMENT__ to use seems to be platform and compiler dependent.

Some examples

clang 11 x86: 8
clang 11 amd64: 16
gcc 10 x86: 8
gcc 10 amd64: 16
AT 12 PPC: 16 [couldn't check 32bit]
gcc 10 arm7 : 8
gcc 10 arm8 and arm64: 16
Comment 2 Paul Floyd 2023-03-08 22:14:41 UTC
I haven't yet added any memcheck errors, but this first step is done. Valgrind now behaves like stdlibc++
There is a small difference with libc++ which accepts an alignment of zero.

commit d4affb0ab725a59da786fee4b918b338eec615fe (HEAD -> master, origin/master, origin/HEAD)
Author: Paul Floyd <pjfloyd@wanadoo.fr>
Date:   Wed Mar 8 23:10:22 2023 +0100

    Make operator new aligned more like the standalone versions
    
    If the alignment is not a power of two return nullptr for the
    nothrow overload and bomb for the throwing overload.
Comment 3 Paul Floyd 2023-03-16 07:04:45 UTC
ASAN example

Here ASAN is assuming use of new delete expressions whilst I was making raw calls to the operators.

paulf> ./new_delete_mismatch_size.asan
=================================================================
==1505==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x603000000040 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   32 bytes;
  size of the deallocated type: 33 bytes.
    #0 0x2b9452 in operator delete(void*, unsigned long) /usr/src/contrib/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:164:3
    #1 0x2bb021 in main /usr/home/paulf/scratch/valgrind/memcheck/tests/new_delete_mismatch_size.cpp:18:5
    #2 0x23613f in _start /usr/src/lib/csu/amd64/crt1_c.c:75:7
    #3 0x8002e0007  (<unknown module>)

So far I have

==7503== Mismatched new/delete with size 33 
==7503==    at 0x484F133: operator delete(void*, unsigned long) (vg_replace_malloc.c:1036)
==7503==    by 0x201AF1: main (new_delete_mismatch_size.cpp:18)
==7503==  Address 0x55b4040 is 0 bytes inside a block of size 32 alloc'd
==7503==    at 0x484D0D4: operator new(unsigned long) (vg_replace_malloc.c:487)
==7503==    by 0x201AE0: main (new_delete_mismatch_size.cpp:17)
Comment 4 Paul Floyd 2023-09-11 09:54:16 UTC
Fixed.