Patchwork [v4] libc: Extend __libc_freeres framework (Bug 23329).

login
register
mail settings
Submitter Carlos O'Donell
Date June 28, 2018, 1:35 p.m.
Message ID <be30b49c-a791-796e-db62-841ed0697184@redhat.com>
Download mbox | patch
Permalink /patch/28099/
State New
Headers show

Comments

Carlos O'Donell - June 28, 2018, 1:35 p.m.
On 06/28/2018 09:22 AM, Florian Weimer wrote:
>> +    [BZ #23329]
>> +    * include/libc-symbols.h: Comment the freeres framework.
>> +    * include/set-hooks.h: Include libc-symbols.h.  Fix comment.
>> +    * dlfcn/Makefile (libdl-routines): Add dlfreeres.
>> +    * dlfcn/Versions (GLIBC_PRIVATE): Add __libdl_freeres.
>> +    * dlfcn/dlerror.c: Include libc-symbols.h
>> +    (__dlerror_main_freeres): New function.
>> +    * dlfcn/dlfreeres.c: New file.
>> +    * dlfcn/sdlfreeres.c: New file.
>> +    * include/dlfcn.h: Declare __dlerror_main_freeres.
>> +    * malloc/set-freeres.c: Declare __libdl_freeres, and
>> +    __libpthread_freeres.
>> +    (__libc_subfreeres): Call __libdl_freeres, and __libpthread_freeres if
>> +    the releavant libraries are loaded.
>> +    * malloc/thread-freeres.c: Add comments.
>> +    * nptl/Makefile (libpthread-routines): Add nptlfreeres.
>> +    * nptl/Version (GLIBC_PRIVATE): Add __libpthread_freeres.
>> +    * nptl/allocatestack.c (__nptl_free_stacks): New function.
>> +    (__free_stacks): Rename to...
>> +    (free_stacks): ...this. Mark static.
>> +    (queue_stack): Call free_stacks.
>> +    * nptl/libc_pthread_init.c [SHARED] (freeres_libpthread): Delete.
>> +    * nptl/nptl-init.c: Delete delcaration of nptl_freeres.
>> +    * sysdeps/nptl/pthread-functions.h (pthread_functions): Remove
>> +    ptr_freeres element from struct.
>> +    (pthread_functions): Remove .ptr_freeres from struct initializer.
>> +    [SHARED] (nptl_freeres): Remove.
>> +    * nptl/nptlfreeres.c: New file.
>> +    * nptl/pthreadP.h
>> +    [IS_IN (libpthread) && SHARED ] (__unwind_freeres): Rename to...
>> +    [IS_IN (libpthread)] (__nptl_unwind_freeres): ...this.  Mark
>> +    attribute_hidden.
>> +    (__free_stacks): Rename to...
>> +    (__nptl_stacks_freeres): ...this.
>> +    (__shm_directory_freeres): Declare.
>> +    * nptl/unwind-forcedunwind.c (__unwind_freeres): Rename to...
>> +    (__nptl_unwind_freeres): ...this.
>> +    * resolv/res-close.c: Add comment.
>> +    * resolv/resolv_conf.c: Include libc-symbols.h.
>> +    * string/strerror_l.c: Include libc-symbols.h.
>> +    * sunrpc/rpc_thread.c: Include libc-symbols.h.
>> +    * sysdeps/mach/strerror_l.c: Inlcude libc-symbols.h
>> +    * sysdeps/unix/sysv/linux/shm-directory.c (freeit): Rename to...
>> +    [IS_IN (libpthread)] (__shm_directory_freeres): ...this.
> 
> ChangeLog looks okay now (but it still does not apply with git am -3 --reject).

Complies and applies fine for me.

This is the only commit I have on top of master.

Where are the conflicts?

>> diff --git a/dlfcn/dlfreeres.c b/dlfcn/dlfreeres.c
>> new file mode 100644
>> index 0000000000..8c18731b5c
> …
>> +void
>> +__libdl_freeres (void)
>> +{
>> +  call_function_static_weak (__dlerror_main_freeres);
>> +}
>> +libc_hidden_def (__libdl_freeres)
> 
> Please remove the last line of the file before committing.  It is a no-op (otherwise it wouldn't compile due to the missing libc_hidden_proto).

Fixed. I missed this, I had a slightly different version before where
I wanted this to be called internally. I don't need hidden def for
a public API.

v4
- Attached. With libc_hidden_def removed for __libdl_freeres.

Cheers,
Carlos.
Florian Weimer - June 28, 2018, 1:44 p.m.
On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
> This is the only commit I have on top of master.
> 
> Where are the conflicts?

git am cannot construct a common ancestor because it doesn't have a copy 
of the original file:

diff --git a/ChangeLog b/ChangeLog
index 1f35f29d77..5f34833c2c 100644

Locally, I do not have a blob starting with 1f35f29d77, with master at 
commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.

Thanks,
Florian
Carlos O'Donell - June 28, 2018, 2:13 p.m.
On 06/28/2018 09:44 AM, Florian Weimer wrote:
> On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
>> This is the only commit I have on top of master.
>>
>> Where are the conflicts?
> 
> git am cannot construct a common ancestor because it doesn't have a copy of the original file:
> 
> diff --git a/ChangeLog b/ChangeLog
> index 1f35f29d77..5f34833c2c 100644
> 
> Locally, I do not have a blob starting with 1f35f29d77, with master at commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.

I don't have it either, but git format-patch HEAD~2 still generates it.

I also have master at 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.

I have two local commits on top of HEAD.

commit ed8f443d30cb0e685a0be38f4587ca84a9e2e846 (HEAD)
commit d55ec04203058b78de125c8fd1f1762b649f228d
commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80 (origin/master, origin/HEAD)

Cheers,
Carlos.
Florian Weimer - June 28, 2018, 2:15 p.m.
On 06/28/2018 04:13 PM, Carlos O'Donell wrote:
> On 06/28/2018 09:44 AM, Florian Weimer wrote:
>> On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
>>> This is the only commit I have on top of master.
>>>
>>> Where are the conflicts?
>>
>> git am cannot construct a common ancestor because it doesn't have a copy of the original file:
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index 1f35f29d77..5f34833c2c 100644
>>
>> Locally, I do not have a blob starting with 1f35f29d77, with master at commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
> 
> I don't have it either, but git format-patch HEAD~2 still generates it.
> 
> I also have master at 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
> 
> I have two local commits on top of HEAD.
> 
> commit ed8f443d30cb0e685a0be38f4587ca84a9e2e846 (HEAD)
> commit d55ec04203058b78de125c8fd1f1762b649f228d
> commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80 (origin/master, origin/HEAD)

Hmm, what do “git whatchanged” or “git ls-tree HEAD^ ChangeLog” show?

Florian
Andreas Schwab - June 28, 2018, 2:23 p.m.
On Jun 28 2018, Carlos O'Donell <carlos@redhat.com> wrote:

> On 06/28/2018 09:44 AM, Florian Weimer wrote:
>> On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
>>> This is the only commit I have on top of master.
>>>
>>> Where are the conflicts?
>> 
>> git am cannot construct a common ancestor because it doesn't have a copy of the original file:
>> 
>> diff --git a/ChangeLog b/ChangeLog
>> index 1f35f29d77..5f34833c2c 100644
>> 
>> Locally, I do not have a blob starting with 1f35f29d77, with master at commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
>
> I don't have it either, but git format-patch HEAD~2 still generates it.

It's a blob, not a commit.  Try git show 1f35f29d77.

Andreas.
Carlos O'Donell - June 28, 2018, 2:26 p.m.
On 06/28/2018 10:15 AM, Florian Weimer wrote:
> On 06/28/2018 04:13 PM, Carlos O'Donell wrote:
>> On 06/28/2018 09:44 AM, Florian Weimer wrote:
>>> On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
>>>> This is the only commit I have on top of master.
>>>>
>>>> Where are the conflicts?
>>>
>>> git am cannot construct a common ancestor because it doesn't have a copy of the original file:
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> index 1f35f29d77..5f34833c2c 100644
>>>
>>> Locally, I do not have a blob starting with 1f35f29d77, with master at commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
>>
>> I don't have it either, but git format-patch HEAD~2 still generates it.
>>
>> I also have master at 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
>>
>> I have two local commits on top of HEAD.
>>
>> commit ed8f443d30cb0e685a0be38f4587ca84a9e2e846 (HEAD)
>> commit d55ec04203058b78de125c8fd1f1762b649f228d
>> commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80 (origin/master, origin/HEAD)
> 
> Hmm, what do “git whatchanged” or “git ls-tree HEAD^ ChangeLog” show?

For the d55ec04203058b78de125c8fd1f1762b649f228d commit, which is the
__libc_freeres cleanup patch (the other patch is the malloc hook removal
sitting on top), we have:

:100644 100644 1f35f29d77... 5f34833c2c... M    ChangeLog
               ^^^^^^^^^^^^^

:100644 100644 56dcae0604... 34f9923334... M    dlfcn/Makefile
:100644 100644 97902f0dfd... 1df6925a92... M    dlfcn/Versions
:100644 100644 04dce9ddc6... 33574faab6... M    dlfcn/dlerror.c
:000000 100644 0000000000... 4004db0edb... A    dlfcn/dlfreeres.c
:000000 100644 0000000000... 7347672990... A    dlfcn/sdlfreeres.c
:100644 100644 c231309083... 0dc57dbe22... M    include/dlfcn.h
:100644 100644 6137304b0b... 8b9273c13a... M    include/libc-symbols.h
:100644 100644 0207656bb9... b3bd8b4092... M    include/set-hooks.h
:100644 100644 f4a0e7bda4... cda368479f... M    malloc/set-freeres.c
:100644 100644 8902c845bc... a63b6c93f3... M    malloc/thread-freeres.c
:100644 100644 0f9c44afa0... 2f2bb0569d... M    nptl/Makefile
:100644 100644 0ae5def464... b1c2da06c0... M    nptl/Versions
:100644 100644 9c10b993fd... f9e053f9e5... M    nptl/allocatestack.c
:100644 100644 f390480d4b... e254ea2552... M    nptl/libc_pthread_init.c
:100644 100644 1d3790f500... 907411d5bc... M    nptl/nptl-init.c
:000000 100644 0000000000... cfa54e1bc7... A    nptl/nptlfreeres.c
:100644 100644 075530c15c... 3cba0b6f9e... M    nptl/pthreadP.h
:100644 100644 38572b1d2f... 1c36dd5f1d... M    resolv/res-close.c
:100644 100644 b4021ab735... 2f0ffbc524... M    resolv/resolv_conf.c
:100644 100644 2a9c3b5e0b... 2a62b1f12c... M    string/strerror_l.c
:100644 100644 a65a90dc15... 0abe6dc172... M    sunrpc/rpc_thread.c
:100644 100644 b9842ccf40... 7111124439... M    sysdeps/mach/strerror_l.c
:100644 100644 53a4b38fbc... fa103695d9... M    sysdeps/nptl/pthread-functions.h
:100644 100644 0621c8002f... 5902fa4be7... M    sysdeps/nptl/unwind-forcedunwind.c
:100644 100644 c494d3fbf6... 6a7c7fd182... M    sysdeps/unix/sysv/linux/shm-directory.c

commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80 (origin/master, origin/HEAD)
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Thu Jun 28 13:27:31 2018 +0100

    Fix documentation build with old makeinfo
    
    With old makeinfo '@code {' fails because of the extra space.
    
            * manual/llio.texi: Remove spurious space.

:100644 100644 25e824faca... 1f35f29d77... M    ChangeLog
                             ^^^^^^^^^^
:100644 100644 e840c55f86... 2733b9cb73... M    manual/llio.texi

So it looks correct.

Cheers,
Carlos.
Florian Weimer - June 28, 2018, 2:50 p.m.
On 06/28/2018 04:26 PM, Carlos O'Donell wrote:
> For the d55ec04203058b78de125c8fd1f1762b649f228d commit, which is the
> __libc_freeres cleanup patch (the other patch is the malloc hook removal
> sitting on top), we have:
> 
> :100644 100644 1f35f29d77... 5f34833c2c... M    ChangeLog
>                 ^^^^^^^^^^^^^

Hmm, looks like I botched my testing after fetching from sourceware.  I 
agree that applying the patch should work now.

Thanks,
Florian
Florian Weimer - June 28, 2018, 7:10 p.m.
On 06/28/2018 04:23 PM, Andreas Schwab wrote:
> On Jun 28 2018, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> On 06/28/2018 09:44 AM, Florian Weimer wrote:
>>> On 06/28/2018 03:35 PM, Carlos O'Donell wrote:
>>>> This is the only commit I have on top of master.
>>>>
>>>> Where are the conflicts?
>>>
>>> git am cannot construct a common ancestor because it doesn't have a copy of the original file:
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> index 1f35f29d77..5f34833c2c 100644
>>>
>>> Locally, I do not have a blob starting with 1f35f29d77, with master at commit 0b11b6491cc01b5ce5fea1302f8591f25e8c4d80.
>>
>> I don't have it either, but git format-patch HEAD~2 still generates it.
> 
> It's a blob, not a commit.  Try git show 1f35f29d77.

I used git cat-file blob.  I'm surprised that git show handles blob 
hashes because doing so increases the chances of collisions

I didn't have it locally at first, but then I fetched, and grepped 
whatchanged output, but rooted that not in the fetched master branch 
(master instead of origin/master).

So I guess a git am-based workflow would have to always git fetch first.

*However*, Carlos' messages are not actually git am-ready.  The commit 
message that git am produces is totally bogus and does not match what's 
intended to be committed at all.  We should probably continue that 
discussion on the other thread, though.

Thanks,
Florian
Andreas Schwab - July 2, 2018, 8:56 a.m.
On Jun 28 2018, Florian Weimer <fweimer@redhat.com> wrote:

> I used git cat-file blob.  I'm surprised that git show handles blob hashes
> because doing so increases the chances of collisions

git cat-file is plumbing, git show is porcelain.

Andreas.

Patch

From d55ec04203058b78de125c8fd1f1762b649f228d Mon Sep 17 00:00:00 2001
From: Carlos O'Donell <carlos@redhat.com>
Date: Fri, 22 Jun 2018 09:28:47 -0400
Subject: [PATCH] libc: Extend __libc_freeres framework (Bug 23329).

The __libc_freeres framework does not extend to non-libc.so objects.
This causes problems in general for valgrind and mtrace detecting
unfreed objects in both libdl.so and libpthread.so.  This change is
a pre-requisite to properly moving the malloc hooks out of malloc
since such a move now requires precise accounting of all allocated
data before destructors are run.

This commit adds a proper hook in libc.so.6 for both libdl.so and
for libpthread.so, this ensures that shm-directory.c which uses
freeit () to free memory is called properly.  We also remove the
nptl_freeres hook and fall back to using weak-ref-and-check idiom
for a loaded libpthread.so, thus making this process similar for
all DSOs.

Lastly we follow best practice and use explicit free calls for
both libdl.so and libpthread.so instead of the generic hook process
which has undefined order.

Tested on x86_64 with no regressions.

Signed-off-by: DJ Delorie <dj@redhat.com>
Signed-off-by: Carlos O'Donell <carlos@redhat.com>
---
 ChangeLog                               | 48 +++++++++++++++++++
 dlfcn/Makefile                          |  2 +-
 dlfcn/Versions                          |  1 +
 dlfcn/dlerror.c                         | 14 ++++++
 dlfcn/dlfreeres.c                       | 29 ++++++++++++
 dlfcn/sdlfreeres.c                      |  1 +
 include/dlfcn.h                         |  4 +-
 include/libc-symbols.h                  | 83 +++++++++++++++++++++++++++++----
 include/set-hooks.h                     |  3 +-
 malloc/set-freeres.c                    | 15 ++++++
 malloc/thread-freeres.c                 |  4 +-
 nptl/Makefile                           |  2 +-
 nptl/Versions                           |  1 +
 nptl/allocatestack.c                    | 12 +++--
 nptl/libc_pthread_init.c                |  8 ----
 nptl/nptl-init.c                        | 15 ------
 nptl/nptlfreeres.c                      | 31 ++++++++++++
 nptl/pthreadP.h                         |  5 +-
 resolv/res-close.c                      |  1 +
 resolv/resolv_conf.c                    |  1 +
 string/strerror_l.c                     |  3 +-
 sunrpc/rpc_thread.c                     |  1 +
 sysdeps/mach/strerror_l.c               |  3 +-
 sysdeps/nptl/pthread-functions.h        |  1 -
 sysdeps/nptl/unwind-forcedunwind.c      |  4 +-
 sysdeps/unix/sysv/linux/shm-directory.c |  6 +--
 26 files changed, 245 insertions(+), 53 deletions(-)
 create mode 100644 dlfcn/dlfreeres.c
 create mode 100644 dlfcn/sdlfreeres.c
 create mode 100644 nptl/nptlfreeres.c

diff --git a/ChangeLog b/ChangeLog
index 1f35f29d77..5f34833c2c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,51 @@ 
+2018-06-28  DJ Delorie  <dj@redhat.com>
+	    Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #23329]
+	* include/libc-symbols.h: Comment the freeres framework.
+	* include/set-hooks.h: Include libc-symbols.h.  Fix comment.
+	* dlfcn/Makefile (libdl-routines): Add dlfreeres.
+	* dlfcn/Versions (GLIBC_PRIVATE): Add __libdl_freeres.
+	* dlfcn/dlerror.c: Include libc-symbols.h
+	(__dlerror_main_freeres): New function.
+	* dlfcn/dlfreeres.c: New file.
+	* dlfcn/sdlfreeres.c: New file.
+	* include/dlfcn.h: Declare __dlerror_main_freeres.
+	* malloc/set-freeres.c: Declare __libdl_freeres, and
+	__libpthread_freeres.
+	(__libc_subfreeres): Call __libdl_freeres, and __libpthread_freeres if
+	the releavant libraries are loaded.
+	* malloc/thread-freeres.c: Add comments.
+	* nptl/Makefile (libpthread-routines): Add nptlfreeres.
+	* nptl/Version (GLIBC_PRIVATE): Add __libpthread_freeres.
+	* nptl/allocatestack.c (__nptl_free_stacks): New function.
+	(__free_stacks): Rename to...
+	(free_stacks): ...this. Mark static.
+	(queue_stack): Call free_stacks.
+	* nptl/libc_pthread_init.c [SHARED] (freeres_libpthread): Delete.
+	* nptl/nptl-init.c: Delete delcaration of nptl_freeres.
+	* sysdeps/nptl/pthread-functions.h (pthread_functions): Remove
+	ptr_freeres element from struct.
+	(pthread_functions): Remove .ptr_freeres from struct initializer.
+	[SHARED] (nptl_freeres): Remove.
+	* nptl/nptlfreeres.c: New file.
+	* nptl/pthreadP.h
+	[IS_IN (libpthread) && SHARED ] (__unwind_freeres): Rename to...
+	[IS_IN (libpthread)] (__nptl_unwind_freeres): ...this.  Mark
+	attribute_hidden.
+	(__free_stacks): Rename to...
+	(__nptl_stacks_freeres): ...this.
+	(__shm_directory_freeres): Declare.
+	* nptl/unwind-forcedunwind.c (__unwind_freeres): Rename to...
+	(__nptl_unwind_freeres): ...this.
+	* resolv/res-close.c: Add comment.
+	* resolv/resolv_conf.c: Include libc-symbols.h.
+	* string/strerror_l.c: Include libc-symbols.h.
+	* sunrpc/rpc_thread.c: Include libc-symbols.h.
+	* sysdeps/mach/strerror_l.c: Inlcude libc-symbols.h
+	* sysdeps/unix/sysv/linux/shm-directory.c (freeit): Rename to...
+	[IS_IN (libpthread)] (__shm_directory_freeres): ...this.
+
 2018-06-28  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
 	* manual/llio.texi: Remove spurious space.
diff --git a/dlfcn/Makefile b/dlfcn/Makefile
index 56dcae0604..34f9923334 100644
--- a/dlfcn/Makefile
+++ b/dlfcn/Makefile
@@ -22,7 +22,7 @@  include ../Makeconfig
 headers		:= bits/dlfcn.h dlfcn.h
 extra-libs	:= libdl
 libdl-routines	:= dlopen dlclose dlsym dlvsym dlerror dladdr dladdr1 dlinfo \
-		   dlmopen dlfcn
+		   dlmopen dlfcn dlfreeres
 routines	:= $(patsubst %,s%,$(filter-out dlfcn,$(libdl-routines)))
 elide-routines.os := $(routines)
 
diff --git a/dlfcn/Versions b/dlfcn/Versions
index 97902f0dfd..1df6925a92 100644
--- a/dlfcn/Versions
+++ b/dlfcn/Versions
@@ -13,5 +13,6 @@  libdl {
   }
   GLIBC_PRIVATE {
     _dlfcn_hook;
+    __libdl_freeres;
   }
 }
diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c
index 04dce9ddc6..33574faab6 100644
--- a/dlfcn/dlerror.c
+++ b/dlfcn/dlerror.c
@@ -24,6 +24,7 @@ 
 #include <string.h>
 #include <libc-lock.h>
 #include <ldsodefs.h>
+#include <libc-symbols.h>
 
 #if !defined SHARED && IS_IN (libdl)
 
@@ -222,6 +223,19 @@  free_key_mem (void *mem)
 
 # ifdef SHARED
 
+/* Free the dlerror-related resources.  */
+void
+__dlerror_main_freeres (void)
+{
+  void *mem;
+  /* Free the global memory if used.  */
+  check_free (&last_result);
+  /* Free the TSD memory if used.  */
+  mem = __libc_getspecific (key);
+  if (mem != NULL)
+    free_key_mem (mem);
+}
+
 struct dlfcn_hook *_dlfcn_hook __attribute__((nocommon));
 libdl_hidden_data_def (_dlfcn_hook)
 
diff --git a/dlfcn/dlfreeres.c b/dlfcn/dlfreeres.c
new file mode 100644
index 0000000000..4004db0edb
--- /dev/null
+++ b/dlfcn/dlfreeres.c
@@ -0,0 +1,29 @@ 
+/* Clean up allocated libdl memory on demand.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <set-hooks.h>
+#include <libc-symbols.h>
+#include <dlfcn.h>
+
+/* Free libdl.so resources.
+   Note: Caller ensures we are called only once.  */
+void
+__libdl_freeres (void)
+{
+  call_function_static_weak (__dlerror_main_freeres);
+}
diff --git a/dlfcn/sdlfreeres.c b/dlfcn/sdlfreeres.c
new file mode 100644
index 0000000000..7347672990
--- /dev/null
+++ b/dlfcn/sdlfreeres.c
@@ -0,0 +1 @@ 
+#include "dlfreeres.c"
diff --git a/include/dlfcn.h b/include/dlfcn.h
index c231309083..0dc57dbe22 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -155,6 +155,8 @@  extern void __libc_register_dl_open_hook (struct link_map *map)
 extern void __libc_register_dlfcn_hook (struct link_map *map)
      attribute_hidden;
 #endif
-#endif
 
+extern void __dlerror_main_freeres (void) attribute_hidden;
+
+#endif
 #endif
diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index 6137304b0b..8b9273c13a 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -217,16 +217,6 @@ 
   static const char __evoke_link_warning_##symbol[]	\
     __attribute__ ((used, section (".gnu.warning." #symbol __sec_comment))) \
     = msg;
-#define libc_freeres_ptr(decl) \
-  __make_section_unallocated ("__libc_freeres_ptrs, \"aw\", %nobits") \
-  decl __attribute__ ((section ("__libc_freeres_ptrs" __sec_comment)))
-#define __libc_freeres_fn_section \
-  __attribute__ ((section ("__libc_freeres_fn")))
-
-#define libc_freeres_fn(name)	\
-  static void name (void) __attribute_used__ __libc_freeres_fn_section;	\
-  text_set_element (__libc_subfreeres, name);				\
-  static void name (void)
 
 /* A canned warning for sysdeps/stub functions.  */
 #define	stub_warning(name) \
@@ -244,6 +234,79 @@  requires at runtime the shared libraries from the glibc version used \
 for linking")
 #endif
 
+/* Resource Freeing Hooks:
+
+   Normally a process exits and the OS cleans up any allocated
+   memory.  However, when tooling like mtrace or valgrind is monitoring
+   the process we need to free all resources that are part of the
+   process in order to provide the consistency required to track
+   memory leaks.
+
+   A single public API exists and is __libc_freeres(), and this is used
+   by applications like valgrind to freee resouces.
+
+   There are 3 cases:
+
+   (a) __libc_freeres
+
+	In this case all you need to do is define the freeing routine:
+
+	foo.c:
+	libfoo_freeres_fn (foo_freeres)
+	{
+	  complex_free (mem);
+	}
+
+	This ensures the function is called at the right point to free
+	resources.
+
+   (b) __libc_freeres_ptr
+
+	The framework for (a) iterates over the list of pointers-to-free
+	in (b) and frees them.
+
+	foo.c:
+	libc_freeres_ptr (static char *foo_buffer);
+
+	Freeing these resources alaways happens last and is equivalent
+	to registering a function that does 'free (foo_buffer)'.
+
+   (c) Explicit lists of free routines to call or objects to free.
+
+	It is the intended goal to remove (a) and (b) which have some
+	non-determinism based on link order, and instead use explicit
+	lists of functions and frees to resolve cleanup ordering issues
+	and make it easy to debug and maintain.
+
+	As of today the following subsystems use (c):
+
+	Per-thread cleanup:
+	* malloc/thread-freeres.c
+
+	libdl cleanup:
+	* dlfcn/dlfreeres.c
+
+	libpthread cleanup:
+	* nptl/nptlfreeres.c
+
+	So if you need any shutdown routines to run you should add them
+	directly to the appropriate subsystem's shutdown list.  */
+
+/* Resource pointers to free in libc.so.  */
+#define libc_freeres_ptr(decl) \
+  __make_section_unallocated ("__libc_freeres_ptrs, \"aw\", %nobits") \
+  decl __attribute__ ((section ("__libc_freeres_ptrs" __sec_comment)))
+
+/* Resource freeing functions from libc.so go in this section.  */
+#define __libc_freeres_fn_section \
+  __attribute__ ((section ("__libc_freeres_fn")))
+
+/* Resource freeing functions for libc.so.  */
+#define libc_freeres_fn(name) \
+  static void name (void) __attribute_used__ __libc_freeres_fn_section;	\
+  text_set_element (__libc_subfreeres, name);				\
+  static void name (void)
+
 /* Declare SYMBOL to be TYPE (`function' or `object') of SIZE bytes
    alias to ORIGINAL, when the assembler supports such declarations
    (such as in ELF).
diff --git a/include/set-hooks.h b/include/set-hooks.h
index 0207656bb9..b3bd8b4092 100644
--- a/include/set-hooks.h
+++ b/include/set-hooks.h
@@ -22,11 +22,12 @@ 
 #define __need_size_t
 #include <stddef.h>
 #include <sys/cdefs.h>
+#include <libc-symbols.h>
 
 #ifdef symbol_set_define
 /* Define a hook variable called NAME.  Functions put on this hook take
    arguments described by PROTO.  Use `text_set_element (NAME, FUNCTION)'
-   from gnu-stabs.h to add a function to the hook.  */
+   from include/libc-symbols.h to add a function to the hook.  */
 
 # define DEFINE_HOOK(NAME, PROTO)		\
   typedef void __##NAME##_hook_function_t PROTO; \
diff --git a/malloc/set-freeres.c b/malloc/set-freeres.c
index f4a0e7bda4..cda368479f 100644
--- a/malloc/set-freeres.c
+++ b/malloc/set-freeres.c
@@ -26,6 +26,10 @@  DEFINE_HOOK (__libc_subfreeres, (void));
 
 symbol_set_define (__libc_freeres_ptrs);
 
+extern __attribute__ ((weak)) void __libdl_freeres (void);
+
+extern __attribute__ ((weak)) void __libpthread_freeres (void);
+
 void __libc_freeres_fn_section
 __libc_freeres (void)
 {
@@ -39,8 +43,19 @@  __libc_freeres (void)
 
       _IO_cleanup ();
 
+      /* We run the resource freeing after IO cleanup.  */
       RUN_HOOK (__libc_subfreeres, ());
 
+      /* Call the libdl list of cleanup functions
+	 (weak-ref-and-check).  */
+      if (&__libdl_freeres != NULL)
+	__libdl_freeres ();
+
+      /* Call the libpthread list of cleanup functions
+	 (weak-ref-and-check).  */
+      if (&__libpthread_freeres != NULL)
+	__libpthread_freeres ();
+
       for (p = symbol_set_first_element (__libc_freeres_ptrs);
            !symbol_set_end_p (__libc_freeres_ptrs, p); ++p)
         free (*p);
diff --git a/malloc/thread-freeres.c b/malloc/thread-freeres.c
index 8902c845bc..a63b6c93f3 100644
--- a/malloc/thread-freeres.c
+++ b/malloc/thread-freeres.c
@@ -24,8 +24,8 @@ 
 
 /* Thread shutdown function.  Note that this function must be called
    for threads during shutdown for correctness reasons.  Unlike
-   __libc_subfreeres, skipping calls to it is not a valid
-   optimization.  */
+   __libc_subfreeres, skipping calls to it is not a valid optimization.
+   This is called directly from pthread_create as the thread exits.  */
 void
 __libc_thread_freeres (void)
 {
diff --git a/nptl/Makefile b/nptl/Makefile
index 0f9c44afa0..2f2bb0569d 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -45,7 +45,7 @@  pthread-compat-wrappers = \
 		      sigwait sigsuspend \
 		      recvmsg sendmsg
 
-libpthread-routines = nptl-init vars events version pt-interp \
+libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pthread_create pthread_exit pthread_detach \
 		      pthread_join pthread_tryjoin pthread_timedjoin \
 		      pthread_join_common \
diff --git a/nptl/Versions b/nptl/Versions
index 0ae5def464..b1c2da06c0 100644
--- a/nptl/Versions
+++ b/nptl/Versions
@@ -271,5 +271,6 @@  libpthread {
     __pthread_unwind; __pthread_get_minstack;
     __pthread_barrier_init; __pthread_barrier_wait;
     __shm_directory;
+    __libpthread_freeres;
   }
 }
diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
index 9c10b993fd..f9e053f9e5 100644
--- a/nptl/allocatestack.c
+++ b/nptl/allocatestack.c
@@ -251,8 +251,8 @@  get_cached_stack (size_t *sizep, void **memp)
 
 
 /* Free stacks until cache size is lower than LIMIT.  */
-void
-__free_stacks (size_t limit)
+static void
+free_stacks (size_t limit)
 {
   /* We reduce the size of the cache.  Remove the last entries until
      the size is below the limit.  */
@@ -288,6 +288,12 @@  __free_stacks (size_t limit)
     }
 }
 
+/* Free all the stacks on cleanup.  */
+void
+__nptl_stacks_freeres (void)
+{
+  free_stacks (0);
+}
 
 /* Add a stack frame which is not used anymore to the stack.  Must be
    called with the cache lock held.  */
@@ -302,7 +308,7 @@  queue_stack (struct pthread *stack)
 
   stack_cache_actsize += stack->stackblock_size;
   if (__glibc_unlikely (stack_cache_actsize > stack_cache_maxsize))
-    __free_stacks (stack_cache_maxsize);
+    free_stacks (stack_cache_maxsize);
 }
 
 
diff --git a/nptl/libc_pthread_init.c b/nptl/libc_pthread_init.c
index f390480d4b..e254ea2552 100644
--- a/nptl/libc_pthread_init.c
+++ b/nptl/libc_pthread_init.c
@@ -77,11 +77,3 @@  __libc_pthread_init (unsigned long int *ptr, void (*reclaim) (void),
   return &__libc_multiple_threads;
 #endif
 }
-
-#ifdef SHARED
-libc_freeres_fn (freeres_libptread)
-{
-  if (__libc_pthread_functions_init)
-    PTHFCT_CALL (ptr_freeres, ());
-}
-#endif
diff --git a/nptl/nptl-init.c b/nptl/nptl-init.c
index 1d3790f500..907411d5bc 100644
--- a/nptl/nptl-init.c
+++ b/nptl/nptl-init.c
@@ -78,9 +78,6 @@  extern
 void __nptl_set_robust (struct pthread *);
 
 #ifdef SHARED
-static void nptl_freeres (void);
-
-
 static const struct pthread_functions pthread_functions =
   {
     .ptr_pthread_attr_destroy = __pthread_attr_destroy,
@@ -140,8 +137,6 @@  static const struct pthread_functions pthread_functions =
 # ifdef SIGSETXID
     .ptr__nptl_setxid = __nptl_setxid,
 # endif
-    /* For now only the stack cache needs to be freed.  */
-    .ptr_freeres = nptl_freeres,
     .ptr_set_robust = __nptl_set_robust
   };
 # define ptr_pthread_functions &pthread_functions
@@ -151,16 +146,6 @@  static const struct pthread_functions pthread_functions =
 
 
 #ifdef SHARED
-/* This function is called indirectly from the freeres code in libc.  */
-static void
-__libc_freeres_fn_section
-nptl_freeres (void)
-{
-  __unwind_freeres ();
-  __free_stacks (0);
-}
-
-
 static
 #endif
 void
diff --git a/nptl/nptlfreeres.c b/nptl/nptlfreeres.c
new file mode 100644
index 0000000000..cfa54e1bc7
--- /dev/null
+++ b/nptl/nptlfreeres.c
@@ -0,0 +1,31 @@ 
+/* Clean up allocated libpthread memory on demand.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <set-hooks.h>
+#include <libc-symbols.h>
+#include <pthreadP.h>
+
+/* Free libpthread.so resources.
+   Note: Caller ensures we are called only once.  */
+void
+__libpthread_freeres (void)
+{
+  call_function_static_weak (__nptl_stacks_freeres);
+  call_function_static_weak (__shm_directory_freeres);
+  call_function_static_weak (__nptl_unwind_freeres);
+}
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 075530c15c..3cba0b6f9e 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -279,8 +279,8 @@  hidden_proto (__pthread_register_cancel)
 hidden_proto (__pthread_unregister_cancel)
 # ifdef SHARED
 extern void attribute_hidden pthread_cancel_init (void);
-extern void __unwind_freeres (void);
 # endif
+extern void __nptl_unwind_freeres (void) attribute_hidden;
 #endif
 
 
@@ -597,7 +597,8 @@  extern int __nptl_setxid (struct xid_command *cmdp) attribute_hidden;
 extern void __nptl_set_robust (struct pthread *self);
 #endif
 
-extern void __free_stacks (size_t limit) attribute_hidden;
+extern void __nptl_stacks_freeres (void) attribute_hidden;
+extern void __shm_directory_freeres (void) attribute_hidden;
 
 extern void __wait_lookup_done (void) attribute_hidden;
 
diff --git a/resolv/res-close.c b/resolv/res-close.c
index 38572b1d2f..1c36dd5f1d 100644
--- a/resolv/res-close.c
+++ b/resolv/res-close.c
@@ -140,4 +140,5 @@  __res_thread_freeres (void)
   /* Make sure we do a full re-initialization the next time.  */
   _res.options = 0;
 }
+/* Also must be called when the main thread exits.  */
 text_set_element (__libc_subfreeres, __res_thread_freeres);
diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c
index b4021ab735..2f0ffbc524 100644
--- a/resolv/resolv_conf.c
+++ b/resolv/resolv_conf.c
@@ -23,6 +23,7 @@ 
 #include <libc-lock.h>
 #include <resolv-internal.h>
 #include <sys/stat.h>
+#include <libc-symbols.h>
 
 /* _res._u._ext.__glibc_extension_index is used as an index into a
    struct resolv_conf_array object.  The intent of this construction
diff --git a/string/strerror_l.c b/string/strerror_l.c
index 2a9c3b5e0b..2a62b1f12c 100644
--- a/string/strerror_l.c
+++ b/string/strerror_l.c
@@ -21,7 +21,7 @@ 
 #include <stdlib.h>
 #include <string.h>
 #include <sys/param.h>
-
+#include <libc-symbols.h>
 
 static __thread char *last_value;
 
@@ -56,7 +56,6 @@  strerror_l (int errnum, locale_t loc)
   return (char *) translate (_sys_errlist_internal[errnum], loc);
 }
 
-
 void
 __strerror_thread_freeres (void)
 {
diff --git a/sunrpc/rpc_thread.c b/sunrpc/rpc_thread.c
index a65a90dc15..0abe6dc172 100644
--- a/sunrpc/rpc_thread.c
+++ b/sunrpc/rpc_thread.c
@@ -5,6 +5,7 @@ 
 #include <libc-lock.h>
 #include <libc-tsd.h>
 #include <shlib-compat.h>
+#include <libc-symbols.h>
 
 
 /* Variable used in non-threaded applications or for the first thread.  */
diff --git a/sysdeps/mach/strerror_l.c b/sysdeps/mach/strerror_l.c
index b9842ccf40..7111124439 100644
--- a/sysdeps/mach/strerror_l.c
+++ b/sysdeps/mach/strerror_l.c
@@ -24,6 +24,7 @@ 
 #include <mach/error.h>
 #include <errorlib.h>
 #include <sys/param.h>
+#include <libc-symbols.h>
 
 
 static __thread char *last_value;
@@ -86,7 +87,7 @@  strerror_l (int errnum, locale_t loc)
   return (char *) translate (es->subsystem[sub].codes[code], loc);
 }
 
-
+/* This is called when a thread is exiting to free the last_value string.  */
 void
 __strerror_thread_freeres (void)
 {
diff --git a/sysdeps/nptl/pthread-functions.h b/sysdeps/nptl/pthread-functions.h
index 53a4b38fbc..fa103695d9 100644
--- a/sysdeps/nptl/pthread-functions.h
+++ b/sysdeps/nptl/pthread-functions.h
@@ -94,7 +94,6 @@  struct pthread_functions
        __attribute ((noreturn)) __cleanup_fct_attribute;
   void (*ptr__nptl_deallocate_tsd) (void);
   int (*ptr__nptl_setxid) (struct xid_command *);
-  void (*ptr_freeres) (void);
   void (*ptr_set_robust) (struct pthread *);
 };
 
diff --git a/sysdeps/nptl/unwind-forcedunwind.c b/sysdeps/nptl/unwind-forcedunwind.c
index 0621c8002f..5902fa4be7 100644
--- a/sysdeps/nptl/unwind-forcedunwind.c
+++ b/sysdeps/nptl/unwind-forcedunwind.c
@@ -79,9 +79,9 @@  pthread_cancel_init (void)
   libgcc_s_handle = handle;
 }
 
+/* Register for cleanup in libpthread.so.  */
 void
-__libc_freeres_fn_section
-__unwind_freeres (void)
+__nptl_unwind_freeres (void)
 {
   void *handle = libgcc_s_handle;
   if (handle != NULL)
diff --git a/sysdeps/unix/sysv/linux/shm-directory.c b/sysdeps/unix/sysv/linux/shm-directory.c
index c494d3fbf6..6a7c7fd182 100644
--- a/sysdeps/unix/sysv/linux/shm-directory.c
+++ b/sysdeps/unix/sysv/linux/shm-directory.c
@@ -135,13 +135,13 @@  __shm_directory (size_t *len)
 }
 #if IS_IN (libpthread)
 hidden_def (__shm_directory)
-#endif
-
 
 /* Make sure the table is freed if we want to free everything before
    exiting.  */
-libc_freeres_fn (freeit)
+void
+__shm_directory_freeres (void)
 {
   if (mountpoint.dir != defaultdir)
     free (mountpoint.dir);
 }
+#endif
-- 
2.14.4