malloc: Deprecate hook variables, __default_morecore, <mcheck.h>

  The original round of hook variable deprecations missed
__after_morecore_hook and __morecore.  __default_morecore is just
an implementation detail.

The functionality in <mcheck.h> will eventually be replaced with
no-op functions (and a separate, preloadable DSO).

2016-10-26  Florian Weimer  <fweimer@redhat.com>

	* malloc/mcheck.h (__MCHECK_DEPRECATED): Define.
	(mcheck, mcheck_pedantic, mcheck_check_all, mprobe, mtrace)
	(muntrace): Deprecate.
	* malloc/malloc.h (__morecore, __default_morecore)
	(__after_morecore_hook): Likewise.
  

On Wed, 26 Oct 2016, Florian Weimer wrote:

> The functionality in <mcheck.h> will eventually be replaced with
> no-op functions (and a separate, preloadable DSO).

If you declare functionality deprecated then you need to update the manual 
accordingly.  But I don't think the right approach is to declare 
deprecated on an "eventually be replaced" basis.  Rather, the deprecation 
and the new DSO should be in the same patch series, going in the same 
glibc version, so the NEWS file and the main documentation can tell users 
of the mtrace script to preload the DSO and remove calls to the mtrace 
function (if that's the intended way to update code using mtrace and the 
one to be used for all the glibc tests that make use of it).  (And that's 
general for any deprecation: make clear how people should update their 
code and ensure the replacement is actually ready first, don't just say 
something is deprecated in NEWS without the replacement being obvious.)
  

On 10/26/2016 05:55 PM, Joseph Myers wrote:
> On Wed, 26 Oct 2016, Florian Weimer wrote:
>
>> The functionality in <mcheck.h> will eventually be replaced with
>> no-op functions (and a separate, preloadable DSO).
>
> If you declare functionality deprecated then you need to update the manual
> accordingly.  But I don't think the right approach is to declare
> deprecated on an "eventually be replaced" basis.  Rather, the deprecation
> and the new DSO should be in the same patch series, going in the same
> glibc version, so the NEWS file and the main documentation can tell users
> of the mtrace script to preload the DSO and remove calls to the mtrace
> function (if that's the intended way to update code using mtrace and the
> one to be used for all the glibc tests that make use of it).

In my opinion, the larger ecosystem already provides suitable replacements.

* <mcheck.h> and all malloc hook functions are now deprecated.  Future
   implementations of the mcheck- and mtrace-related functions will not
   have any effect, and glibc will stop calling the hook functions from
   its malloc implementation.  Instead of mcheck and mtrace, developers
   should consider using valgrind.  As a replacement for the hook
   functions, developers can interpose their own malloc implementation.

Plus removal of the documentation of the hook functions from the manual 
(if there is anything left).

In any case, I'll submit a separate patch for documenting the malloc 
interposition feature (bug 20424).

Thanks,
Florian
  

On Tue, 15 Nov 2016, Florian Weimer wrote:

> In my opinion, the larger ecosystem already provides suitable replacements.
> 
> * <mcheck.h> and all malloc hook functions are now deprecated.  Future
>   implementations of the mcheck- and mtrace-related functions will not
>   have any effect, and glibc will stop calling the hook functions from
>   its malloc implementation.  Instead of mcheck and mtrace, developers
>   should consider using valgrind.  As a replacement for the hook
>   functions, developers can interpose their own malloc implementation.

I don't consider valgrind suitable for replacing the uses of mtrace in the 
glibc testsuite (or other similar uses elsewhere for lightweight checking 
for leaks).  It's massively more heavyweight, has limited architecture 
support, as far as I know can only be built with an installed glibc and I 
don't know if there are other complications building it in a bootstrap 
environment.  Whereas a simple preloadable interposing malloc 
implementation provided with glibc could support the mtrace functionality 
as well as providing an example for people writing interposing 
implementations to use.

I don't know enough about the functionality GDB expects from linking with 
-lmcheck by default in development to know whether other malloc 
improvements would provide similar checking by default or whether that 
would also need GDB to provide its own malloc (again, I think valgrind is 
vastly too heavyweight for using by default in GDB development).
  

On 11/15/2016 04:39 PM, Joseph Myers wrote:
> On Tue, 15 Nov 2016, Florian Weimer wrote:
>
>> In my opinion, the larger ecosystem already provides suitable replacements.
>>
>> * <mcheck.h> and all malloc hook functions are now deprecated.  Future
>>   implementations of the mcheck- and mtrace-related functions will not
>>   have any effect, and glibc will stop calling the hook functions from
>>   its malloc implementation.  Instead of mcheck and mtrace, developers
>>   should consider using valgrind.  As a replacement for the hook
>>   functions, developers can interpose their own malloc implementation.
>
> I don't consider valgrind suitable for replacing the uses of mtrace in the
> glibc testsuite (or other similar uses elsewhere for lightweight checking
> for leaks).

I completely agree.

The above is for the NEWS file, for external use.  For internal use, we 
still need a solution.  (I have an mtrace-compatible interposition-based 
tracer almost finished, but it may not make the cut for the next 
release, and it may be superseded by DJ's work anyway.)  I think we can 
run internal deprecation at a different pace than external deprecation.

> I don't know enough about the functionality GDB expects from linking with
> -lmcheck by default in development to know whether other malloc
> improvements would provide similar checking by default or whether that
> would also need GDB to provide its own malloc (again, I think valgrind is
> vastly too heavyweight for using by default in GDB development).

I wasn't aware that GDB developers are using this functionality.  I'll 
ask them how critical it is to their needs.  But I expect that there 
will be no glibc release which neither provides mcheck, nor provides an 
alternative.

Thanks,
Florian
  

On Tue, 15 Nov 2016, Florian Weimer wrote:

> > I don't consider valgrind suitable for replacing the uses of mtrace in the
> > glibc testsuite (or other similar uses elsewhere for lightweight checking
> > for leaks).
> 
> I completely agree.
> 
> The above is for the NEWS file, for external use.  For internal use, we still
> need a solution.  (I have an mtrace-compatible interposition-based tracer
> almost finished, but it may not make the cut for the next release, and it may
> be superseded by DJ's work anyway.)  I think we can run internal deprecation
> at a different pace than external deprecation.

Well, I think mtrace (meaning the ability to run with tracing then run the 
mtrace script to process the results, more than the functions to turn 
tracing on and off) is similarly useful externally as a lightweight system 
for tracing allocations and detecting leaks.  And it is of course 
documented in the manual.
  

On Wed, Nov 16, 2016 at 1:36 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 15 Nov 2016, Florian Weimer wrote:
>
>> > I don't consider valgrind suitable for replacing the uses of mtrace in the
>> > glibc testsuite (or other similar uses elsewhere for lightweight checking
>> > for leaks).
>>
>> I completely agree.
>>
>> The above is for the NEWS file, for external use.  For internal use, we still
>> need a solution.  (I have an mtrace-compatible interposition-based tracer
>> almost finished, but it may not make the cut for the next release, and it may
>> be superseded by DJ's work anyway.)  I think we can run internal deprecation
>> at a different pace than external deprecation.
>
> Well, I think mtrace (meaning the ability to run with tracing then run the
> mtrace script to process the results, more than the functions to turn
> tracing on and off) is similarly useful externally as a lightweight system
> for tracing allocations and detecting leaks.  And it is of course
> documented in the manual.

It is a useful feature to use on constrained systems to do basic
allocation tracing. The main deficiency it has is it is not
thread-safe, which makes it rather useless for many modern
applications. In an ideal world glibc malloc would support a wider
range of tracing and profiling features similar to jemalloc/tcmalloc,
e.g. thread-safe allocation tracing, pprof.
  

On 11/16/2016 02:36 AM, Joseph Myers wrote:
> On Tue, 15 Nov 2016, Florian Weimer wrote:
>
>>> I don't consider valgrind suitable for replacing the uses of mtrace in the
>>> glibc testsuite (or other similar uses elsewhere for lightweight checking
>>> for leaks).
>>
>> I completely agree.
>>
>> The above is for the NEWS file, for external use.  For internal use, we still
>> need a solution.  (I have an mtrace-compatible interposition-based tracer
>> almost finished, but it may not make the cut for the next release, and it may
>> be superseded by DJ's work anyway.)  I think we can run internal deprecation
>> at a different pace than external deprecation.
>
> Well, I think mtrace (meaning the ability to run with tracing then run the
> mtrace script to process the results, more than the functions to turn
> tracing on and off) is similarly useful externally as a lightweight system
> for tracing allocations and detecting leaks.  And it is of course
> documented in the manual.

In my experience, it is not useful at all for finding the cause of leaks 
because you only get the address of the immediate caller of malloc, 
which is often a wrapper.  The address is subject to ASLR as well.

Based on the comments in this thread, I'm not sure if people find mtrace 
theoretically useful, or actually use it. :)

In my proposed NEWS entry, I forgot to mention Address Sanitizer (as was 
pointed out in the GDB discussion).  It has leak detection capabilities 
as well:

==7440==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
     #0 0x7fc52f77097a in malloc (/lib64/libasan.so.2+0x9897a)
     #1 0x400703 in main (/tmp/a.out+0x400703)
     #2 0x7fc52f33757f in __libc_start_main (/lib64/libc.so.6+0x2057f)

And it runs significantly faster than valgrind.

Florian
  

On Thu, 17 Nov 2016, Florian Weimer wrote:

> > Well, I think mtrace (meaning the ability to run with tracing then run the
> > mtrace script to process the results, more than the functions to turn
> > tracing on and off) is similarly useful externally as a lightweight system
> > for tracing allocations and detecting leaks.  And it is of course
> > documented in the manual.
> 
> In my experience, it is not useful at all for finding the cause of leaks
> because you only get the address of the immediate caller of malloc, which is
> often a wrapper.  The address is subject to ASLR as well.
> 
> Based on the comments in this thread, I'm not sure if people find mtrace
> theoretically useful, or actually use it. :)

I use it for similar purposes to the glibc testsuite use: compile code 
once then possibly run it with tracing to check for leaks.  (Where the 
expectation is that the cause of the leak is in the code you just 
added/changed that resulted in the check for leaks starting to fail.)

I'd be fine with extending the trace format to allow listing more of a 
backtrace (using glibc's backtrace functions in the preloadable tracing 
malloc implementation).
  

On 11/17/2016 08:00 AM, Florian Weimer wrote:
> In my experience, it is not useful at all for finding the cause of leaks
> because you only get the address of the immediate caller of malloc,
> which is often a wrapper.  The address is subject to ASLR as well.
>
> Based on the comments in this thread, I'm not sure if people find mtrace
> theoretically useful, or actually use it. :)

I make use of mtrace extensively when debugging oomkills on Cray
systems.  Traditional memory leaks are typically not as big a problem as
heap fragmentation.  Special hardware needs require non-default
mallopt settings, notably severly restricting use of mmap, so fragmentation
is a much larger problem for us than for many others.  In my experience,
mtrace is lightweight enough to not noticeably perturb the heap structure
over the course of the program, usually allowing me to pinpoint which allocations
are leading to fragmentation.  Heavier instrumentation, such as valgrind,
messes with it far too much to isolate which allocations from the application
are actually causing fragmentation when the instrumentation is not present,
making them much less useful to me.

> In my proposed NEWS entry, I forgot to mention Address Sanitizer (as was
> pointed out in the GDB discussion).  It has leak detection capabilities
> as well:
>
> ==7440==ERROR: LeakSanitizer: detected memory leaks
>
> Direct leak of 32 byte(s) in 1 object(s) allocated from:
>     #0 0x7fc52f77097a in malloc (/lib64/libasan.so.2+0x9897a)
>     #1 0x400703 in main (/tmp/a.out+0x400703)
>     #2 0x7fc52f33757f in __libc_start_main (/lib64/libc.so.6+0x2057f)
>
> And it runs significantly faster than valgrind.

Does it have a noticeable impact on the layout of the heap compared to
non-instrumented runs?

Steven Vormwald
  

On 17/11/2016 12:50, Steve Vormwald wrote:
> On 11/17/2016 08:00 AM, Florian Weimer wrote:
>> In my experience, it is not useful at all for finding the cause of leaks
>> because you only get the address of the immediate caller of malloc,
>> which is often a wrapper.  The address is subject to ASLR as well.
>>
>> Based on the comments in this thread, I'm not sure if people find mtrace
>> theoretically useful, or actually use it. :)
> 
> I make use of mtrace extensively when debugging oomkills on Cray
> systems.  Traditional memory leaks are typically not as big a problem as
> heap fragmentation.  Special hardware needs require non-default
> mallopt settings, notably severly restricting use of mmap, so fragmentation
> is a much larger problem for us than for many others.  In my experience,
> mtrace is lightweight enough to not noticeably perturb the heap structure
> over the course of the program, usually allowing me to pinpoint which allocations
> are leading to fragmentation.  Heavier instrumentation, such as valgrind,
> messes with it far too much to isolate which allocations from the application
> are actually causing fragmentation when the instrumentation is not present,
> making them much less useful to me.
> 
>> In my proposed NEWS entry, I forgot to mention Address Sanitizer (as was
>> pointed out in the GDB discussion).  It has leak detection capabilities
>> as well:
>>
>> ==7440==ERROR: LeakSanitizer: detected memory leaks
>>
>> Direct leak of 32 byte(s) in 1 object(s) allocated from:
>>     #0 0x7fc52f77097a in malloc (/lib64/libasan.so.2+0x9897a)
>>     #1 0x400703 in main (/tmp/a.out+0x400703)
>>     #2 0x7fc52f33757f in __libc_start_main (/lib64/libc.so.6+0x2057f)
>>
>> And it runs significantly faster than valgrind.
> 
> Does it have a noticeable impact on the layout of the heap compared to
> non-instrumented runs?

Yes, since it is built on ASAN it uses an interposed malloc implementation
and it also uses more VMA (as shadow memory for internal error detection).
  

On 11/17/2016 03:50 PM, Steve Vormwald wrote:
> On 11/17/2016 08:00 AM, Florian Weimer wrote:
>> In my experience, it is not useful at all for finding the cause of leaks
>> because you only get the address of the immediate caller of malloc,
>> which is often a wrapper.  The address is subject to ASLR as well.
>>
>> Based on the comments in this thread, I'm not sure if people find mtrace
>> theoretically useful, or actually use it. :)
>
> I make use of mtrace extensively when debugging oomkills on Cray
> systems.  Traditional memory leaks are typically not as big a problem as
> heap fragmentation.  Special hardware needs require non-default
> mallopt settings, notably severly restricting use of mmap, so fragmentation
> is a much larger problem for us than for many others.

This is very interesting, thanks.

I've been considering for a while to remove the use of sbrk from the 
allocator, both to simplify it and to take away the possibility for 
attacks to tweak the NON_MAIN_ARENA flag to trick the allocator into 
doing something bad.  Some embedded targets appear to strongly prefer 
the sbrk as well, so maybe this approach is doomed.

Would your configuration have problems if we handed all allocations 
which are larger than 1 MiB or so straight to mmap?

> In my experience,
> mtrace is lightweight enough to not noticeably perturb the heap structure
> over the course of the program, usually allowing me to pinpoint which allocations
> are leading to fragmentation.  Heavier instrumentation, such as valgrind,
> messes with it far too much to isolate which allocations from the application
> are actually causing fragmentation when the instrumentation is not present,
> making them much less useful to me.

DJ's tracer should cover that well because it's designed to alter heap 
layout at all (its own allocations use mmap).

We would also be interested in general patterns of fragmentation you 
have encountered, if any.  Maybe there's something we can do about them.

>> In my proposed NEWS entry, I forgot to mention Address Sanitizer (as was
>> pointed out in the GDB discussion).  It has leak detection capabilities
>> as well:
>>
>> ==7440==ERROR: LeakSanitizer: detected memory leaks
>>
>> Direct leak of 32 byte(s) in 1 object(s) allocated from:
>>     #0 0x7fc52f77097a in malloc (/lib64/libasan.so.2+0x9897a)
>>     #1 0x400703 in main (/tmp/a.out+0x400703)
>>     #2 0x7fc52f33757f in __libc_start_main (/lib64/libc.so.6+0x2057f)
>>
>> And it runs significantly faster than valgrind.
>
> Does it have a noticeable impact on the layout of the heap compared to
> non-instrumented runs?

Eh, yes.  It's a totally different allocator.  So I can see why mtrace 
is attractive.

Thanks,
Florian
  

> On 11/18/2016 04:13 AM, Florian Weimer wrote:
> I've been considering for a while to remove the use of sbrk from the
> allocator, both to simplify it and to take away the possibility for
> attacks to tweak the NON_MAIN_ARENA flag to trick the allocator into
> doing something bad.  Some embedded targets appear to strongly prefer
> the sbrk as well, so maybe this approach is doomed.

For us anyway, the use of sbrk isn't as important as its semantics (eg,
heap is contiguous in virtual address space and only grows/shrinks on
one end).  Thus, things like libhugetlbfs that provide the same semantics
via __morecore work just as well.

> Would your configuration have problems if we handed all allocations
> which are larger than 1 MiB or so straight to mmap?

That would probably not work out for us.  The underlying problem we have
is that unmapping pages can be extremely expensive (on the order of tens
of seconds or worse in certain circumstances).  We use segments with
fixed base addresses and maximum length to transparently make portions
of a process's virtual address space (notably including all user allocations)
available for hardware DMA.  Updating page mappings within a segment is
extremely cheap.  Adding a new segment is also relatively cheap, but they
are a limited resource.  Invalidating a segment so it can be reused can get
very expensive since everything that has used it needs to be notified of the
invalidation.  We can take a fast path if the allocator has sbrk-like semantics
as described above because we know it only requires updating page
mappings, assuming the existing segment is large enough.  We have to be
much more conservative for allocations that are satisfied with mmap,
leading to a lot of expensive and potentially unnecessary segment creation
and invalidation.

> We would also be interested in general patterns of fragmentation you
> have encountered, if any.  Maybe there's something we can do about them.

The usual culprit is something "caching" small (<128byte) allocations for
future reuse rather than freeing them when they become unused, which
prevents the allocator from coalescing free regions to satisfy larger allocations.
Changing the code either to allocate in bulk instead of one object at a time or
to simply free them without caching is generally sufficient to fix the
fragmentation issue.

Steven Vormwald
  

Steve Vormwald <sdvormwa@cray.com> writes:
> The usual culprit is something "caching" small (<128byte) allocations for
> future reuse rather than freeing them when they become unused, which

Have you tried disabling fastbins?  I.e. mallopt(M_MXFAST, 0) ?
  

I still feel we have a fairly complete set of replacements available 
today (Address Sanitizer, valgrind for debugging; interposition instead 
of __more_core &c), and that it makes sense to announce our planned 
deprecation early.

It does, however, look rather unlikely I will have concrete to show by 
the end of December 2016.

Final comments?

Thanks,
Florian