Bug 286849 - [PATCH] Interceptors for new/delete on Darwin were erroneously commented out in r12043
Summary: [PATCH] Interceptors for new/delete on Darwin were erroneously commented out ...
Status: CONFIRMED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.7 SVN
Platform: Unlisted Binaries macOS
: NOR normal
Target Milestone: ---
Assignee: Rhys Kidd
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-11-17 14:19 UTC by Alexander Potapenko
Modified: 2015-10-03 07:24 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Proposed patch, re-enabling intercepts (7.82 KB, patch)
2015-10-03 07:23 UTC, Rhys Kidd
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Potapenko 2011-11-17 14:19:42 UTC
r12043 commit message contains the following point:

* comment out many intercepts in mc_replace_strmem.c and
 vg_replace_malloc.c that are apparently unnecessary for Darwin

In this revision a number of interceptors was indeed commented out for OS X (see http://code.google.com/p/valgrind-variant/source/diff?spec=svn122&r=110&format=side&path=/trunk/valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c&old_path=/trunk/valgrind/coregrind/m_replacemalloc/vg_replace_malloc.c&old=88 for the diff):

291     #elif defined(VGO_darwin)
292	 // operator new(unsigned int), GNU mangling
293	 #if VG_WORDSIZE == 4
294	  //ALLOC_or_BOMB(VG_Z_LIBSTDCXX_SONAME, _Znwj,          __builtin_new);
295	  //ALLOC_or_BOMB(VG_Z_LIBC_SONAME,      _Znwj,          __builtin_new);
296	 #endif
297	 // operator new(unsigned long), GNU mangling
298	 #if 1 // FIXME: is this right?
299	  //ALLOC_or_BOMB(VG_Z_LIBSTDCXX_SONAME, _Znwm,          __builtin_new);
300	  //ALLOC_or_BOMB(VG_Z_LIBC_SONAME,      _Znwm,          __builtin_new);
301	 #endif

Before r12043 the leak reports on Chromium looked like this:
 operator new(unsigned long) (m_replacemalloc/vg_replace_malloc.c:299)
 base::StatisticsRecorder::RegisterOrDeleteDuplicateRanges(base::Histogram*)
 base::StatisticsRecorder::RegisterOrDeleteDuplicate(base::Histogram*)
 base::CustomHistogram::FactoryGet(std::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&,
std::vector<int, std::allocator<int> > const&, base::Histogram::Flags)
 base::(anonymous
namespace)::HistogramTest_RecordedStartupTest_Test::TestBody()

After it the leak reports look like:
 malloc (m_replacemalloc/vg_replace_malloc.c:266)
 operator new(unsigned long)
 base::StatisticsRecorder::RegisterOrDeleteDuplicateRanges(base::Histogram*)
 base::StatisticsRecorder::RegisterOrDeleteDuplicate(base::Histogram*)
 base::LinearHistogram::FactoryGet(std::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&, int, int,
unsigned long, base::Histogram::Flags)
 base::(anonymous namespace)::HistogramTest_RangeTest_Test::TestBody()

This might be still correct because operator new on Darwin really
calls malloc(), but this makes the suppressions incompatible with
those used for Linux.
Comment 1 Rhys Kidd 2015-10-03 07:23:05 UTC
Created attachment 94823 [details]
Proposed patch, re-enabling intercepts
Comment 2 Rhys Kidd 2015-10-03 07:24:28 UTC
Attached patch re-enables and cleans up this section of the Darwin-specific code.

Before committing, I want to test this further on the various OS X flavours and across 64bit and 32bit for regressions. But the approach itself should be sound.