Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190306151019.GP23599@brightrain.aerifal.cx>
Date: Wed, 6 Mar 2019 10:10:19 -0500
From: Rich Felker <dalias@...c.org>
To: Ray <i@...kray.me>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: [PATCH] resolve DT_RELR packed relative relocations

On Wed, Mar 06, 2019 at 01:14:17PM +0000, Ray wrote:
> The SHT_RELR (DT_RELR) idea originated from the ChromeOS land but it
> resulted in a proposal in the generic ABI mailing list
> https://groups.google.com/forum/#!topic/generic-abi/bX460iggiKg (As I
> understant it, that forum has no official maintainer now so the
> dyanmic tag numbers cannot be officially
> assigned).https://android-review.googlesource.com/c/platform/build/soong/+/709131/
> has some saving numbers. 3.93% for some Android directory.
> 
> We may consider adopting this section type and benefit from its size
> savings. In llvm, the lld linker can generate SHT_RELR (since
> https://reviews.llvm.org/D48247) sections and llvm-readelf -r can
> decode them (since https://reviews.llvm.org/D47919).
> 
> (I worry the webmail may break the tabs used in this patch. I hope it
> wouldn't cause too much trouble)

Yes, it broke the formatting. Attachments are preferred for patches,
especially if your mail system botches inline ones, but they're easier
to handle either way.

> From 2921fb00cb5967c1d55921f0c807980969caf90c Mon Sep 17 00:00:00 2001
> From: Fangrui Song <i@...kray.me>
> Date: Wed, 6 Mar 2019 10:06:14 +0000
> Subject: [PATCH] resolve DT_RELR packed relative relocations
> 
> this doesn't resolve DT_RELR relocations in the dynamic linker itself.

I don't think it would help a lot, but if this becomes ok for
inclusion, I think it should be supported there too, since that would
also add support in static PIE binaries at the same time.

> proposal for adding SHT_RELR sections to generic-abi:
>   https://groups.google.com/forum/#!topic/generic-abi/bX460iggiKg

As noted by Szabolcs Nagy, this needs to be accepted ABI, not a
proposal from one party, before it can be adopted.

> diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
> index cbe0a6fe..c55ca9e7 100644
> --- a/src/internal/dynlink.h
> +++ b/src/internal/dynlink.h
> @@ -92,7 +92,7 @@ struct fdpic_dummy_loadmap {
>  #endif
> 
>  #define AUX_CNT 32
> -#define DYN_CNT 32
> +#define DYN_CNT 37
> 
>  typedef void (*stage2_func)(unsigned char *, size_t *);
>  typedef _Noreturn void (*stage3_func)(size_t *);

I noticed the change taking DYN_CNT over 32. decode_vec currently does
not support this, and invokes UB if cnt is >32 on a 32-bit arch, via
1UL<<v[0]. A long time ago, it used to be 1ULL<<v[0] to make it
well-defined but lose the flag bit; however, if I recall that
introduced problematic early dependencies on being able to call
libgcc for archs with no native 64-bit shift. It could instead be
replaced with just a conditional if (v[0]<8*sizeof(long)), but it does
need to be changed if this patch is merged.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.