resolve DT_RELR packed relative relocations

Submitted by Fangrui Song on March 6, 2019, 1:14 p.m.

Details

Message ID MWHPR22MB16792B19EA7487EA90BDB32CCB730@MWHPR22MB1679.namprd22.prod.outlook.com
State New
Series "resolve DT_RELR packed relative relocations"
Headers show

Commit Message

Fangrui Song March 6, 2019, 1:14 p.m.
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)


From 2921fb00cb5967c1d55921f0c807980969caf90c Mon Sep 17 00:00:00 2001
From: Fangrui Song <i@maskray.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.

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

since llvm 7.0.0,
ld.lld --pack-dyn-relocs=relr can generate a SHT_RELR section.
llvm-readelf -r can decode SHT_RELR sections.

Signed-off-by: Fangrui Song <i@maskray.me>

---
 include/elf.h          |  8 ++++++--
 ldso/dynlink.c         | 17 +++++++++++++++++
 src/internal/dynlink.h |  2 +-
 3 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.20.1

Patch hide | download patch | download mbox

diff --git a/include/elf.h b/include/elf.h
index 54f41a10..41c27ac7 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -384,7 +384,8 @@  typedef struct {
 #define SHT_PREINIT_ARRAY 16
 #define SHT_GROUP   17
 #define SHT_SYMTAB_SHNDX  18
-#define SHT_NUM   19
+#define SHT_RELR   19
+#define SHT_NUM   20
 #define SHT_LOOS   0x60000000
 #define SHT_GNU_ATTRIBUTES 0x6ffffff5
 #define SHT_GNU_HASH   0x6ffffff6
@@ -744,7 +745,10 @@  typedef struct {
 #define DT_PREINIT_ARRAY 32
 #define DT_PREINIT_ARRAYSZ 33
 #define DT_SYMTAB_SHNDX 34
-#define DT_NUM 35
+#define DT_RELRSZ 35
+#define DT_RELR 36
+#define DT_RELRENT 37
+#define DT_NUM 38
 #define DT_LOOS 0x6000000d
 #define DT_HIOS 0x6ffff000
 #define DT_LOPROC 0x70000000
diff --git a/ldso/dynlink.c b/ldso/dynlink.c
index 35cacd76..bedae482 100644
--- a/ldso/dynlink.c
+++ b/ldso/dynlink.c
@@ -491,6 +491,22 @@  static void do_relocs(struct dso *dso, size_t
*rel, size_t rel_size, size_t stri
  }
 }

+static void do_relr_relocs(struct dso *dso, size_t *rel, size_t rel_size) {
+ unsigned char *base = dso->base;
+ size_t *rel_addr;
+ for (; rel_size; rel++, rel_size-=sizeof(size_t))
+ if (rel[0]%2 == 0) {
+ rel_addr = laddr(dso, rel[0]);
+ *rel_addr++ += (size_t)base;
+ } else {
+ int i = 0;
+ for (size_t bitmap=rel[0]; (bitmap>>=1); i++)
+ if (bitmap&1)
+ rel_addr[i] += (size_t)base;
+ rel_addr += 8*sizeof(size_t)-1;
+ }
+}
+
 static void redo_lazy_relocs()
 {
  struct dso *p = lazy_head, *next;
@@ -1314,6 +1330,7 @@  static void reloc_all(struct dso *p)
  2+(dyn[DT_PLTREL]==DT_RELA));
  do_relocs(p, laddr(p, dyn[DT_REL]), dyn[DT_RELSZ], 2);
  do_relocs(p, laddr(p, dyn[DT_RELA]), dyn[DT_RELASZ], 3);
+ do_relr_relocs(p, laddr(p, dyn[DT_RELR]), dyn[DT_RELRSZ]);

  if (head != &ldso && p->relro_start != p->relro_end &&
      mprotect(laddr(p, p->relro_start), p->relro_end-p->relro_start, PROT_READ)
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 *);

Comments

Szabolcs Nagy March 6, 2019, 2:30 p.m.
* Ray <i@maskray.me> [2019-03-06 13:14:17 +0000]:
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -384,7 +384,8 @@ typedef struct {
>  #define SHT_PREINIT_ARRAY 16
>  #define SHT_GROUP   17
>  #define SHT_SYMTAB_SHNDX  18
> -#define SHT_NUM   19
> +#define SHT_RELR   19
> +#define SHT_NUM   20
>  #define SHT_LOOS   0x60000000
>  #define SHT_GNU_ATTRIBUTES 0x6ffffff5
>  #define SHT_GNU_HASH   0x6ffffff6
> @@ -744,7 +745,10 @@ typedef struct {
>  #define DT_PREINIT_ARRAY 32
>  #define DT_PREINIT_ARRAYSZ 33
>  #define DT_SYMTAB_SHNDX 34
> -#define DT_NUM 35
> +#define DT_RELRSZ 35
> +#define DT_RELR 36
> +#define DT_RELRENT 37
> +#define DT_NUM 38
>  #define DT_LOOS 0x6000000d
>  #define DT_HIOS 0x6ffff000
>  #define DT_LOPROC 0x70000000

if these are really generic elf things (instead of
os specific) then i don't see how lld can implement
this before the numbers are allocated in the gabi.

the last gabi change was in 2013
http://www.sco.com/developers/gabi/latest/revision.html
and currently its status is unclear, see discussions at
https://groups.google.com/forum/#!msg/generic-abi/XsQpUE6s02o/6rJd339BEAAJ

i think we do want a document instead of a mailing list
post to avoid conflicts. but it is a bit tricky since
the copyright owner is not interested maintaining it.
Rich Felker March 6, 2019, 3:10 p.m.
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@maskray.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