Re: [PATCH 2/4] Add a signal-safe malloc replacement

[Thomas Schwinge <thomas at codesourcery dot com>]

Hi!

On Wed, 18 Dec 2013 16:51:09 -0800, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> On Wed, Dec 18, 2013 at 4:12 PM, Paul Pluzhnikov <ppluzhnikov@google.com> wrote:
> > On Wed, Dec 18, 2013 at 4:06 PM, Andrew Hunter <ahh@google.com> wrote:
> >
> >> I think the proper fix is to reorder the patches -- move Factor out
> >> _dl_clear_dtv before this one (having it just call free, not
> >> signal_safe_).  Then apply this patch and replace the free with
> >> signal_safe_free here.)
> >
> > Aha. I haven't looked at patch 3 (until now).
> >
> > I agree, applying slightly modified patch 3, then patch 2, is the way to go.
> >
> > I'll test and commit in that order.
> 
> Tested attached patch on Linux/x86_64 and i686.
> Committed as 1f33d36a8a9e78c81bed59b47f260723f56bb7e6

(Again with ambiguous ChangeLog author vs. Git commit author.)


> --- a/elf/dl-misc.c
> +++ b/elf/dl-misc.c

> +void *weak_function
> +__signal_safe_memalign (size_t boundary, size_t size)
> +{
> +  [...]
> +  void *actual = mmap (NULL, actual_size, PROT_READ | PROT_WRITE,
> +		       MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +  [...]
> +	  int ret = munmap (actual, (start_pg - actual_pg - 1) * pg);
> +  [...]

> +void weak_function
> +__signal_safe_free (void *ptr)
> +{
> +  [...]
> +  int ret = munmap (header->start, header->size);

This causes a link failure on Hurd, because to resolve the mmap and
munmap symbols it tries to pull in object files that are not meant to be
pulled in:

    gcc-4.7   -nostdlib -nostartfiles -r -o [...]/elf/librtld.map.o '-Wl,-(' [...]/elf/dl-allobjs.os [...]/libc_pic.a [...]/mach/libmachuser_pic.a [...]/hurd/libhurduser_pic.a -lgcc '-Wl,-)' -Wl,-Map,[...]/elf/librtld.mapT
    [...]/libc_pic.a(init-first.os):(.data+0x0): multiple definition of `__libc_multiple_libcs'
    [...]/elf/dl-allobjs.os:[...]/elf/rtld.c:797: first defined here
    [...]/libc_pic.a(_itoa.os): In function `_itoa':
    [...]/stdio-common/_itoa.c:199: multiple definition of `_itoa'
    [...]/elf/dl-allobjs.os:[...]/elf/dl-minimal.c:317: first defined here
    collect2: error: ld returned 1 exit status
    make[2]: *** [[...]/elf/librtld.map] Error 1

OK to fix as follows?

	* elf/dl-misc.c (__signal_safe_memalign, __signal_safe_free): Use
	internal symbols for mmap and munmap.

diff --git elf/dl-misc.c elf/dl-misc.c
index b529af3..0ea4329 100644
--- elf/dl-misc.c
+++ elf/dl-misc.c
@@ -418,8 +418,8 @@ __signal_safe_memalign (size_t boundary, size_t size)
 
 
   size_t actual_size = roundup (padded_size, pg);
-  void *actual = mmap (NULL, actual_size, PROT_READ | PROT_WRITE,
-		       MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+  void *actual = __mmap (NULL, actual_size, PROT_READ | PROT_WRITE,
+			 MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
   if (actual == MAP_FAILED)
     return NULL;
 
@@ -435,7 +435,7 @@ __signal_safe_memalign (size_t boundary, size_t size)
       start_pg -= start_pg % boundary_pg;
       if (start_pg > (actual_pg + 1))
 	{
-	  int ret = munmap (actual, (start_pg - actual_pg - 1) * pg);
+	  int ret = __munmap (actual, (start_pg - actual_pg - 1) * pg);
 	  assert (ret == 0);
 	  actual = (void *) ((start_pg - 1) * pg);
 	}
@@ -466,7 +466,7 @@ __signal_safe_free (void *ptr)
 
   struct __signal_safe_allocator_header *header
     = ptr_to_signal_safe_allocator_header (ptr);
-  int ret = munmap (header->start, header->size);
+  int ret = __munmap (header->start, header->size);
 
   assert (ret == 0);
 }


GrÃÃe,
 Thomas

Attachment: pgpPWoX7PI09C.pgp
Description: PGP signature