diff mbox series

[dwarves,v2,2/4] btf_encoder: Manually lay out updated ELF sections

Message ID 20210201172530.1141087-3-gprocida@google.com (mailing list archive)
State Not Applicable
Delegated to: BPF
Headers show
Series BTF ELF writing changes | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Giuliano Procida Feb. 1, 2021, 5:25 p.m. UTC
pahole -J needs to do the following to an ELF file:

* add or update the ".BTF" section
* maybe update the section name string table
* update the Section Header Table (SHT)

libelf either takes full control of layout or requires the user to
specify offset, size and alignment of all new and updated sections and
headers.

To avoid libelf moving program segments in particular, we position the
".BTF" and section name string table (typically named ".shstrtab")
sections after all others. The SHT always lives at the end of the file.

Note that the last section in an ELF file is normally the section name
string table and any ".BTF" section will normally be second last.
However, if these sections appear earlier, then we'll waste some space
in the ELF file when we rewrite them.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 libbtf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 2 deletions(-)

Comments

Andrii Nakryiko Feb. 4, 2021, 4:13 a.m. UTC | #1
On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida <gprocida@google.com> wrote:
>
> pahole -J needs to do the following to an ELF file:
>
> * add or update the ".BTF" section
> * maybe update the section name string table
> * update the Section Header Table (SHT)
>
> libelf either takes full control of layout or requires the user to
> specify offset, size and alignment of all new and updated sections and
> headers.
>
> To avoid libelf moving program segments in particular, we position the

It's not clear to me what's wrong with libelf handling all the layout.
Even if libelf will move program segments around, what's the harm?
Does it break anything if we just let libelf do this?

> ".BTF" and section name string table (typically named ".shstrtab")
> sections after all others. The SHT always lives at the end of the file.
>
> Note that the last section in an ELF file is normally the section name
> string table and any ".BTF" section will normally be second last.
> However, if these sections appear earlier, then we'll waste some space
> in the ELF file when we rewrite them.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  libbtf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 62 insertions(+), 2 deletions(-)
>

[...]
Giuliano Procida Feb. 4, 2021, 6:34 p.m. UTC | #2
Hi.

On Thu, 4 Feb 2021 at 04:13, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida <gprocida@google.com> wrote:
> >
> > pahole -J needs to do the following to an ELF file:
> >
> > * add or update the ".BTF" section
> > * maybe update the section name string table
> > * update the Section Header Table (SHT)
> >
> > libelf either takes full control of layout or requires the user to
> > specify offset, size and alignment of all new and updated sections and
> > headers.
> >
> > To avoid libelf moving program segments in particular, we position the
>
> It's not clear to me what's wrong with libelf handling all the layout.
> Even if libelf will move program segments around, what's the harm?
> Does it break anything if we just let libelf do this?
>

It doesn't hurt the userspace case I care about. I've no idea what it
means in terms of vmlinux.

However, I wrote that text before I discovered that pahole -J isn't
actually used to modify kernel images.

One thing I haven't tried is to try to make .BTF loadable but leave
placement to libelf.

> > ".BTF" and section name string table (typically named ".shstrtab")
> > sections after all others. The SHT always lives at the end of the file.
> >
> > Note that the last section in an ELF file is normally the section name
> > string table and any ".BTF" section will normally be second last.
> > However, if these sections appear earlier, then we'll waste some space
> > in the ELF file when we rewrite them.
> >
> > Signed-off-by: Giuliano Procida <gprocida@google.com>
> > ---
> >  libbtf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 62 insertions(+), 2 deletions(-)
> >
>
> [...]
Andrii Nakryiko Feb. 4, 2021, 11:06 p.m. UTC | #3
On Thu, Feb 4, 2021 at 10:34 AM Giuliano Procida <gprocida@google.com> wrote:
>
> Hi.
>
> On Thu, 4 Feb 2021 at 04:13, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida <gprocida@google.com> wrote:
> > >
> > > pahole -J needs to do the following to an ELF file:
> > >
> > > * add or update the ".BTF" section
> > > * maybe update the section name string table
> > > * update the Section Header Table (SHT)
> > >
> > > libelf either takes full control of layout or requires the user to
> > > specify offset, size and alignment of all new and updated sections and
> > > headers.
> > >
> > > To avoid libelf moving program segments in particular, we position the
> >
> > It's not clear to me what's wrong with libelf handling all the layout.
> > Even if libelf will move program segments around, what's the harm?
> > Does it break anything if we just let libelf do this?
> >
>
> It doesn't hurt the userspace case I care about. I've no idea what it
> means in terms of vmlinux.
>
> However, I wrote that text before I discovered that pahole -J isn't
> actually used to modify kernel images.
>
> One thing I haven't tried is to try to make .BTF loadable but leave
> placement to libelf.

I'd concentrate on getting rid of llvm-objcopy dependency first.
Making .BTF loadable is even riskier change and there is no use case
that relies on that today, so definitely worth to split that out.


>
> > > ".BTF" and section name string table (typically named ".shstrtab")
> > > sections after all others. The SHT always lives at the end of the file.
> > >
> > > Note that the last section in an ELF file is normally the section name
> > > string table and any ".BTF" section will normally be second last.
> > > However, if these sections appear earlier, then we'll waste some space
> > > in the ELF file when we rewrite them.
> > >
> > > Signed-off-by: Giuliano Procida <gprocida@google.com>
> > > ---
> > >  libbtf.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 62 insertions(+), 2 deletions(-)
> > >
> >
> > [...]
diff mbox series

Patch

diff --git a/libbtf.c b/libbtf.c
index 5b91d3a..6e06a58 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -741,9 +741,28 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 	}
 
 	/*
-	 * First we check if there is already a .BTF section present.
+	 * The SHT is at the very end of the ELF and gets re-written in any case.
+	 *
+	 * We'll always add or update the .BTF section and when adding have to
+	 * re-write the section name string table (usually named .shstrtab). In
+	 * fact, as good citizens, we'll always leave the string table last, in
+	 * case someone else wants to add a section.
+	 *
+	 * However, if .BTF or the section name string table are followed by
+	 * further sections, we'll not try to be clever about shuffling
+	 * everything else in the ELF file, we'll just leave some dead space.
+	 * This actually happens in practice with vmlinux which has .strtab
+	 * after .shstrtab, resulting in a (small) hole the size of the original
+	 * .shstrtab.
+	 */
+
+	/*
+	 * First we look if there was already a .BTF section present and
+	 * determine the first usable offset in the ELF (for .BTF and the
+	 * section name table).
 	 */
 	elf_getshdrstrndx(elf, &strndx);
+	size_t high_water_mark = 0;
 	Elf_Scn *btf_scn = 0;
 	while ((scn = elf_nextscn(elf, scn)) != NULL) {
 		shdr = gelf_getshdr(scn, &shdr_mem);
@@ -752,7 +771,10 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 		char *secname = elf_strptr(elf, strndx, shdr->sh_name);
 		if (strcmp(secname, ".BTF") == 0) {
 			btf_scn = scn;
-			break;
+		} else if (elf_ndxscn(scn) != strndx) {
+			size_t limit = shdr->sh_offset + shdr->sh_size;
+			if (limit > high_water_mark)
+				high_water_mark = limit;
 		}
 	}
 
@@ -761,6 +783,12 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 		fprintf(stderr, "%s: elf_getscn(strndx) failed\n", __func__);
 		goto out;
 	}
+	GElf_Shdr str_shdr_mem;
+	GElf_Shdr *str_shdr = gelf_getshdr(str_scn, &str_shdr_mem);
+	if (!str_shdr) {
+		fprintf(stderr, "%s: elf_getshdr(str_scn) failed\n", __func__);
+		goto out;
+	}
 
 	size_t dot_btf_offset = 0;
 	if (btf_scn) {
@@ -791,6 +819,7 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 		str_data->d_buf = str_table;
 		str_data->d_size = new_str_size;
 		elf_flagdata(str_data, ELF_C_SET, ELF_F_DIRTY);
+		str_shdr->sh_size = new_str_size;
 
 		/* Create a new section */
 		btf_scn = elf_newscn(elf);
@@ -810,12 +839,15 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 	/* (Re)populate the BTF section data */
 	raw_btf_data = btf__get_raw_data(btf, &raw_btf_size);
 	btf_data->d_buf = (void *)raw_btf_data;
+	btf_data->d_off = 0;
 	btf_data->d_size = raw_btf_size;
 	btf_data->d_type = ELF_T_BYTE;
 	btf_data->d_version = EV_CURRENT;
 	elf_flagdata(btf_data, ELF_C_SET, ELF_F_DIRTY);
 
 	/* Update .BTF section in the SHT */
+	size_t new_btf_offset = high_water_mark;
+	size_t new_btf_size = raw_btf_size;
 	GElf_Shdr btf_shdr_mem;
 	GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem);
 	if (!btf_shdr) {
@@ -827,6 +859,8 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 	btf_shdr->sh_flags = SHF_ALLOC;
 	if (dot_btf_offset)
 		btf_shdr->sh_name = dot_btf_offset;
+	btf_shdr->sh_offset = new_btf_offset;
+	btf_shdr->sh_size = new_btf_size;
 	btf_shdr->sh_type = SHT_PROGBITS;
 	if (!gelf_update_shdr(btf_scn, btf_shdr)) {
 		fprintf(stderr, "%s: gelf_update_shdr failed: %s\n",
@@ -834,6 +868,32 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 		goto out;
 	}
 
+	/* Update section name string table */
+	size_t new_str_offset = new_btf_offset + new_btf_size;
+	str_shdr->sh_offset = new_str_offset;
+	if (!gelf_update_shdr(str_scn, str_shdr)) {
+		fprintf(stderr, "gelf_update_shdr failed\n");
+		goto out;
+	}
+
+	/* Update SHT, allowing for ELF64 alignment */
+	size_t sht_offset = roundup(new_str_offset + str_shdr->sh_size, 8);
+	ehdr->e_shoff = sht_offset;
+	if (!gelf_update_ehdr(elf, ehdr)) {
+		fprintf(stderr, "gelf_update_ehdr failed\n");
+		goto out;
+	}
+
+	if (btf_elf__verbose) {
+		fprintf(stderr, ".BTF [0x%lx, +0x%lx)\n",
+			btf_shdr->sh_offset, btf_shdr->sh_size);
+		fprintf(stderr, ".shstrtab [0x%lx, +0x%lx)\n",
+			str_shdr->sh_offset, str_shdr->sh_size);
+		fprintf(stderr, "SHT [0x%lx, +%d*0x%x)\n",
+			ehdr->e_shoff, ehdr->e_shnum, ehdr->e_shentsize);
+	}
+
+	elf_flagelf(elf, ELF_C_SET, ELF_F_LAYOUT);
 	if (elf_update(elf, ELF_C_NULL) < 0) {
 		fprintf(stderr, "%s: elf_update (layout) failed: %s\n",
 			__func__, elf_errmsg(elf_errno()));