| Summary: | vg_replace_malloc DELETE checks size | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | Mark Wielaard <mark> |
| Component: | memcheck | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | pjfloyd |
| Priority: | NOR | ||
| Version First Reported In: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| See Also: | https://bugzilla.redhat.com/show_bug.cgi?id=2246032 | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
commit a8b6ee6b5f5efbd759c87fa987e9149800db2899 Author: Mark Wielaard <mark@klomp.org> Date: Thu Oct 26 12:25:44 2023 +0200 vg_replace_malloc DELETE should not check size The DELETE replacement functions check the size argument, but this doesn't actually exist. Only the DELETE_SIZED replacement functions get a size (and should check it). On i386 (fedora gnu/linux) this causes the following failures: memcheck/tests/cxx17_aligned_new (stderr) memcheck/tests/leak_cpp_interior (stderr) memcheck/tests/mismatches (stderr) memcheck/tests/mismatches_xml (stderr) memcheck/tests/new_aligned_delete_default (stderr) memcheck/tests/new_nothrow (stderr) memcheck/tests/realloc_size_zero_mismatch (stderr) All showing "size" being undefined: +Conditional jump or move depends on uninitialised value(s) + at 0x........: ...operator delete[]... (vg_replace_malloc.c:...) or +Mismatched new/delete size value: 4 + at 0x........: ...operator delete... (vg_replace_malloc.c:...) Oddly no other architecture seems to show issues. Maybe we just got lucky? This patch fixes the issues on i386 (and shows no regressions on x86_64) https://bugs.kde.org/show_bug.cgi?id=476108 Yes, I probably copied DELETE from DELETE_SIZED and didn’t remove all the sized bits. On 64bit the rags are passed in registers so less likely to be uninitialized. The diff looks good. |
The DELETE replacement functions check the size argument, but this doesn't actually exist. Only the DELETE_SIZED replacement functions get a size (and should check it). On i386 (fedora gnu/linux) this causes the following failures: memcheck/tests/cxx17_aligned_new (stderr) memcheck/tests/leak_cpp_interior (stderr) memcheck/tests/mismatches (stderr) memcheck/tests/mismatches_xml (stderr) memcheck/tests/new_aligned_delete_default (stderr) memcheck/tests/new_nothrow (stderr) memcheck/tests/realloc_size_zero_mismatch (stderr) All showing "size" being undefined: +Conditional jump or move depends on uninitialised value(s) + at 0x........: ...operator delete[]... (vg_replace_malloc.c:...) or +Mismatched new/delete size value: 4 + at 0x........: ...operator delete... (vg_replace_malloc.c:...) Oddly no other architecture seems to show issues. Maybe we just got lucky? The following patch fixes the issues on i386 (and shows no regressions on x86_64): diff --git a/coregrind/m_replacemalloc/vg_replace_malloc.c b/coregrind/m_replacemalloc/vg_replace_malloc.c index e238a52f3..7859f5f32 100644 --- a/coregrind/m_replacemalloc/vg_replace_malloc.c +++ b/coregrind/m_replacemalloc/vg_replace_malloc.c @@ -1027,13 +1027,12 @@ extern int * __error(void) __attribute__((weak)); #define DELETE(soname, fnname, vg_replacement, tag) \ \ - void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p, SizeT size); \ - void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p, SizeT size) \ + void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p); \ + void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p) \ { \ struct AlignedAllocInfo aligned_alloc_info = { .mem=p, .alloc_kind=AllocKind##tag }; \ \ DO_INIT; \ - TRIGGER_MEMCHECK_ERROR_IF_UNDEFINED((UWord)size); \ VERIFY_ALIGNMENT(&aligned_alloc_info); \ MALLOC_TRACE(#fnname "(%p)\n", p ); \ if (p == NULL) \