diff mbox series

[dwarves,2/4] btf_encoder: Add .BTF section using libelf

Message ID 20210125130625.2030186-3-gprocida@google.com (mailing list archive)
State Superseded
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 Jan. 25, 2021, 1:06 p.m. UTC
pahole -J uses libelf directly when updating a .BTF section. However,
it uses llvm-objcopy to add .BTF sections. This commit switches to
using libelf for both cases.

This eliminates pahole's dependency on llvm-objcopy. One unfortunate
side-effect is that vmlinux actually increases in size. It seems that
llvm-objcopy modifies the .strtab section, discarding many strings. I
speculate that is it discarding strings not referenced from .symtab
and updating the references therein.

In this initial version layout is left completely up to libelf which
may be OK for non-loadable object files, but is probably no good for
things like vmlinux where all the offsets may change. This is
addressed in a follow-up commit.

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

Comments

Jiri Olsa Jan. 27, 2021, 11:23 p.m. UTC | #1
On Mon, Jan 25, 2021 at 01:06:23PM +0000, Giuliano Procida wrote:
> pahole -J uses libelf directly when updating a .BTF section. However,
> it uses llvm-objcopy to add .BTF sections. This commit switches to
> using libelf for both cases.
> 
> This eliminates pahole's dependency on llvm-objcopy. One unfortunate
> side-effect is that vmlinux actually increases in size. It seems that
> llvm-objcopy modifies the .strtab section, discarding many strings. I
> speculate that is it discarding strings not referenced from .symtab
> and updating the references therein.
> 
> In this initial version layout is left completely up to libelf which
> may be OK for non-loadable object files, but is probably no good for
> things like vmlinux where all the offsets may change. This is
> addressed in a follow-up commit.
> 
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  libbtf.c | 145 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 100 insertions(+), 45 deletions(-)
> 
> diff --git a/libbtf.c b/libbtf.c
> index 9f76283..fb8e043 100644
> --- a/libbtf.c
> +++ b/libbtf.c
> @@ -699,6 +699,7 @@ static int btf_elf__write(const char *filename, struct btf *btf)
>  	uint32_t raw_btf_size;
>  	int fd, err = -1;
>  	size_t strndx;
> +	void *str_table = NULL;
>  
>  	fd = open(filename, O_RDWR);
>  	if (fd < 0) {
> @@ -741,74 +742,128 @@ static int btf_elf__write(const char *filename, struct btf *btf)
>  	}
>  
>  	/*
> -	 * First we look if there was already a .BTF section to overwrite.
> +	 * First we check if there is already a .BTF section present.
>  	 */
> -
>  	elf_getshdrstrndx(elf, &strndx);
> +	Elf_Scn *btf_scn = 0;

NULL


SNIP

> -		const char *llvm_objcopy;
> -		char tmp_fn[PATH_MAX];
> -		char cmd[PATH_MAX * 2];
> -
> -		llvm_objcopy = getenv("LLVM_OBJCOPY");
> -		if (!llvm_objcopy)
> -			llvm_objcopy = "llvm-objcopy";
> -
> -		/* Use objcopy to add a .BTF section */
> -		snprintf(tmp_fn, sizeof(tmp_fn), "%s.btf", filename);
> -		close(fd);
> -		fd = creat(tmp_fn, S_IRUSR | S_IWUSR);
> -		if (fd == -1) {
> -			fprintf(stderr, "%s: open(%s) failed!\n", __func__,
> -				tmp_fn);
> +		/* Add ".BTF" to the section name string table */
> +		Elf_Data *str_data = elf_getdata(str_scn, NULL);
> +		if (!str_data) {
> +			fprintf(stderr, "%s: elf_getdata(str_scn) failed: %s\n",
> +				__func__, elf_errmsg(elf_errno()));
>  			goto out;
>  		}
> -
> -		if (write(fd, raw_btf_data, raw_btf_size) != raw_btf_size) {
> -			fprintf(stderr, "%s: write of %d bytes to '%s' failed: %d!\n",
> -				__func__, raw_btf_size, tmp_fn, errno);
> -			goto unlink;
> +		dot_btf_offset = str_data->d_size;
> +		size_t new_str_size = dot_btf_offset + 5;
> +		str_table = malloc(new_str_size);
> +		if (!str_table) {
> +			fprintf(stderr, "%s: malloc(%zu) failed: %s\n", __func__,
> +				new_str_size, elf_errmsg(elf_errno()));
> +			goto out;
>  		}
> +		memcpy(str_table, str_data->d_buf, dot_btf_offset);
> +		memcpy(str_table + dot_btf_offset, ".BTF", 5);

hum, I wonder this will always copy the final zero byte

> +		str_data->d_buf = str_table;
> +		str_data->d_size = new_str_size;
> +		elf_flagdata(str_data, ELF_C_SET, ELF_F_DIRTY);
> +
> +		/* Create a new section */
> +		btf_scn = elf_newscn(elf);
> +		if (!btf_scn) {
> +			fprintf(stderr, "%s: elf_newscn failed: %s\n",
> +			__func__, elf_errmsg(elf_errno()));
> +			goto out;
> +		}
> +		btf_data = elf_newdata(btf_scn);
> +		if (!btf_data) {
> +			fprintf(stderr, "%s: elf_newdata failed: %s\n",
> +			__func__, elf_errmsg(elf_errno()));
> +			goto out;
> +		}
> +	}
>  
> -		snprintf(cmd, sizeof(cmd), "%s --add-section .BTF=%s %s",
> -			 llvm_objcopy, tmp_fn, filename);
> -		if (system(cmd)) {
> -			fprintf(stderr, "%s: failed to add .BTF section to '%s': %d!\n",
> -				__func__, filename, errno);
> -			goto unlink;
> +	/* (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;

doesn't this potentially leak btf_data->d_buf?

> +	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 */
> +	GElf_Shdr btf_shdr_mem;
> +	GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem);
> +	if (!btf_shdr) {
> +		fprintf(stderr, "%s: elf_getshdr(btf_scn) failed: %s\n",
> +			__func__, elf_errmsg(elf_errno()));
> +		goto out;
> +	}
> +	btf_shdr->sh_entsize = 0;
> +	btf_shdr->sh_flags = 0;
> +	if (dot_btf_offset)
> +		btf_shdr->sh_name = dot_btf_offset;
> +	btf_shdr->sh_type = SHT_PROGBITS;
> +	if (!gelf_update_shdr(btf_scn, btf_shdr)) {
> +		fprintf(stderr, "%s: gelf_update_shdr failed: %s\n",
> +			__func__, elf_errmsg(elf_errno()));
> +		goto out;
> +	}
> +
> +	if (elf_update(elf, ELF_C_NULL) < 0) {
> +		fprintf(stderr, "%s: elf_update (layout) failed: %s\n",
> +			__func__, elf_errmsg(elf_errno()));
> +		goto out;
> +	}
> +
> +	size_t phnum = 0;
> +	if (!elf_getphdrnum(elf, &phnum)) {
> +		for (size_t ix = 0; ix < phnum; ++ix) {
> +			GElf_Phdr phdr;
> +			GElf_Phdr *elf_phdr = gelf_getphdr(elf, ix, &phdr);
> +			size_t filesz = gelf_fsize(elf, ELF_T_PHDR, 1, EV_CURRENT);
> +			fprintf(stderr, "type: %d %d\n", elf_phdr->p_type, PT_PHDR);
> +			fprintf(stderr, "offset: %lu %lu\n", elf_phdr->p_offset, ehdr->e_phoff);
> +			fprintf(stderr, "filesize: %lu %lu\n", elf_phdr->p_filesz, filesz);

looks like s forgotten debug or you're missing
btf_elf__verbose check for fprintf calls above

jirka
Giuliano Procida Jan. 28, 2021, 1:35 p.m. UTC | #2
Hi.

On Wed, 27 Jan 2021 at 23:23, Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Jan 25, 2021 at 01:06:23PM +0000, Giuliano Procida wrote:
> > pahole -J uses libelf directly when updating a .BTF section. However,
> > it uses llvm-objcopy to add .BTF sections. This commit switches to
> > using libelf for both cases.
> >
> > This eliminates pahole's dependency on llvm-objcopy. One unfortunate
> > side-effect is that vmlinux actually increases in size. It seems that
> > llvm-objcopy modifies the .strtab section, discarding many strings. I
> > speculate that is it discarding strings not referenced from .symtab
> > and updating the references therein.
> >
> > In this initial version layout is left completely up to libelf which
> > may be OK for non-loadable object files, but is probably no good for
> > things like vmlinux where all the offsets may change. This is
> > addressed in a follow-up commit.
> >
> > Signed-off-by: Giuliano Procida <gprocida@google.com>
> > ---
> >  libbtf.c | 145 ++++++++++++++++++++++++++++++++++++++-----------------
> >  1 file changed, 100 insertions(+), 45 deletions(-)
> >
> > diff --git a/libbtf.c b/libbtf.c
> > index 9f76283..fb8e043 100644
> > --- a/libbtf.c
> > +++ b/libbtf.c
> > @@ -699,6 +699,7 @@ static int btf_elf__write(const char *filename, struct btf *btf)
> >       uint32_t raw_btf_size;
> >       int fd, err = -1;
> >       size_t strndx;
> > +     void *str_table = NULL;
> >
> >       fd = open(filename, O_RDWR);
> >       if (fd < 0) {
> > @@ -741,74 +742,128 @@ static int btf_elf__write(const char *filename, struct btf *btf)
> >       }
> >
> >       /*
> > -      * First we look if there was already a .BTF section to overwrite.
> > +      * First we check if there is already a .BTF section present.
> >        */
> > -
> >       elf_getshdrstrndx(elf, &strndx);
> > +     Elf_Scn *btf_scn = 0;
>
> NULL
>
>
> SNIP
>
> > -             const char *llvm_objcopy;
> > -             char tmp_fn[PATH_MAX];
> > -             char cmd[PATH_MAX * 2];
> > -
> > -             llvm_objcopy = getenv("LLVM_OBJCOPY");
> > -             if (!llvm_objcopy)
> > -                     llvm_objcopy = "llvm-objcopy";
> > -
> > -             /* Use objcopy to add a .BTF section */
> > -             snprintf(tmp_fn, sizeof(tmp_fn), "%s.btf", filename);
> > -             close(fd);
> > -             fd = creat(tmp_fn, S_IRUSR | S_IWUSR);
> > -             if (fd == -1) {
> > -                     fprintf(stderr, "%s: open(%s) failed!\n", __func__,
> > -                             tmp_fn);
> > +             /* Add ".BTF" to the section name string table */
> > +             Elf_Data *str_data = elf_getdata(str_scn, NULL);
> > +             if (!str_data) {
> > +                     fprintf(stderr, "%s: elf_getdata(str_scn) failed: %s\n",
> > +                             __func__, elf_errmsg(elf_errno()));
> >                       goto out;
> >               }
> > -
> > -             if (write(fd, raw_btf_data, raw_btf_size) != raw_btf_size) {
> > -                     fprintf(stderr, "%s: write of %d bytes to '%s' failed: %d!\n",
> > -                             __func__, raw_btf_size, tmp_fn, errno);
> > -                     goto unlink;
> > +             dot_btf_offset = str_data->d_size;
> > +             size_t new_str_size = dot_btf_offset + 5;
> > +             str_table = malloc(new_str_size);
> > +             if (!str_table) {
> > +                     fprintf(stderr, "%s: malloc(%zu) failed: %s\n", __func__,
> > +                             new_str_size, elf_errmsg(elf_errno()));
> > +                     goto out;
> >               }
> > +             memcpy(str_table, str_data->d_buf, dot_btf_offset);
> > +             memcpy(str_table + dot_btf_offset, ".BTF", 5);
>
> hum, I wonder this will always copy the final zero byte

It should, as strlen(".BTF") == 4.

> > +             str_data->d_buf = str_table;
> > +             str_data->d_size = new_str_size;
> > +             elf_flagdata(str_data, ELF_C_SET, ELF_F_DIRTY);
> > +
> > +             /* Create a new section */
> > +             btf_scn = elf_newscn(elf);
> > +             if (!btf_scn) {
> > +                     fprintf(stderr, "%s: elf_newscn failed: %s\n",
> > +                     __func__, elf_errmsg(elf_errno()));
> > +                     goto out;
> > +             }
> > +             btf_data = elf_newdata(btf_scn);
> > +             if (!btf_data) {
> > +                     fprintf(stderr, "%s: elf_newdata failed: %s\n",
> > +                     __func__, elf_errmsg(elf_errno()));
> > +                     goto out;
> > +             }
> > +     }
> >
> > -             snprintf(cmd, sizeof(cmd), "%s --add-section .BTF=%s %s",
> > -                      llvm_objcopy, tmp_fn, filename);
> > -             if (system(cmd)) {
> > -                     fprintf(stderr, "%s: failed to add .BTF section to '%s': %d!\n",
> > -                             __func__, filename, errno);
> > -                     goto unlink;
> > +     /* (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;
>
> doesn't this potentially leak btf_data->d_buf?

I believe libelf owns the original btf_data->d_buf and it could even
just be a pointer into mmaped memory,  but I will check.

> > +     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 */
> > +     GElf_Shdr btf_shdr_mem;
> > +     GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem);
> > +     if (!btf_shdr) {
> > +             fprintf(stderr, "%s: elf_getshdr(btf_scn) failed: %s\n",
> > +                     __func__, elf_errmsg(elf_errno()));
> > +             goto out;
> > +     }
> > +     btf_shdr->sh_entsize = 0;
> > +     btf_shdr->sh_flags = 0;
> > +     if (dot_btf_offset)
> > +             btf_shdr->sh_name = dot_btf_offset;
> > +     btf_shdr->sh_type = SHT_PROGBITS;
> > +     if (!gelf_update_shdr(btf_scn, btf_shdr)) {
> > +             fprintf(stderr, "%s: gelf_update_shdr failed: %s\n",
> > +                     __func__, elf_errmsg(elf_errno()));
> > +             goto out;
> > +     }
> > +
> > +     if (elf_update(elf, ELF_C_NULL) < 0) {
> > +             fprintf(stderr, "%s: elf_update (layout) failed: %s\n",
> > +                     __func__, elf_errmsg(elf_errno()));
> > +             goto out;
> > +     }
> > +
> > +     size_t phnum = 0;
> > +     if (!elf_getphdrnum(elf, &phnum)) {
> > +             for (size_t ix = 0; ix < phnum; ++ix) {
> > +                     GElf_Phdr phdr;
> > +                     GElf_Phdr *elf_phdr = gelf_getphdr(elf, ix, &phdr);
> > +                     size_t filesz = gelf_fsize(elf, ELF_T_PHDR, 1, EV_CURRENT);
> > +                     fprintf(stderr, "type: %d %d\n", elf_phdr->p_type, PT_PHDR);
> > +                     fprintf(stderr, "offset: %lu %lu\n", elf_phdr->p_offset, ehdr->e_phoff);
> > +                     fprintf(stderr, "filesize: %lu %lu\n", elf_phdr->p_filesz, filesz);
>
> looks like s forgotten debug or you're missing
> btf_elf__verbose check for fprintf calls above

Oops, forgotten debug.

>
> jirka
>

Thanks,
Giuliano.
Giuliano Procida Feb. 5, 2021, 1:40 p.m. UTC | #3
Hi.

On Thu, 28 Jan 2021 at 13:35, Giuliano Procida <gprocida@google.com> wrote:
>
> Hi.
>
> On Wed, 27 Jan 2021 at 23:23, Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Mon, Jan 25, 2021 at 01:06:23PM +0000, Giuliano Procida wrote:
> > > pahole -J uses libelf directly when updating a .BTF section. However,
> > > it uses llvm-objcopy to add .BTF sections. This commit switches to
> > > using libelf for both cases.
> > >
> > > This eliminates pahole's dependency on llvm-objcopy. One unfortunate
> > > side-effect is that vmlinux actually increases in size. It seems that
> > > llvm-objcopy modifies the .strtab section, discarding many strings. I
> > > speculate that is it discarding strings not referenced from .symtab
> > > and updating the references therein.
> > >
> > > In this initial version layout is left completely up to libelf which
> > > may be OK for non-loadable object files, but is probably no good for
> > > things like vmlinux where all the offsets may change. This is
> > > addressed in a follow-up commit.
> > >
> > > Signed-off-by: Giuliano Procida <gprocida@google.com>
> > > ---
> > >  libbtf.c | 145 ++++++++++++++++++++++++++++++++++++++-----------------
> > >  1 file changed, 100 insertions(+), 45 deletions(-)
> > >
> > > diff --git a/libbtf.c b/libbtf.c
> > > index 9f76283..fb8e043 100644
> > > --- a/libbtf.c
> > > +++ b/libbtf.c
> > > @@ -699,6 +699,7 @@ static int btf_elf__write(const char *filename, struct btf *btf)
> > >       uint32_t raw_btf_size;
> > >       int fd, err = -1;
> > >       size_t strndx;
> > > +     void *str_table = NULL;
> > >
> > >       fd = open(filename, O_RDWR);
> > >       if (fd < 0) {
> > > @@ -741,74 +742,128 @@ static int btf_elf__write(const char *filename, struct btf *btf)
> > >       }
> > >
> > >       /*
> > > -      * First we look if there was already a .BTF section to overwrite.
> > > +      * First we check if there is already a .BTF section present.
> > >        */
> > > -
> > >       elf_getshdrstrndx(elf, &strndx);
> > > +     Elf_Scn *btf_scn = 0;
> >
> > NULL
> >
> >
> > SNIP
> >
> > > -             const char *llvm_objcopy;
> > > -             char tmp_fn[PATH_MAX];
> > > -             char cmd[PATH_MAX * 2];
> > > -
> > > -             llvm_objcopy = getenv("LLVM_OBJCOPY");
> > > -             if (!llvm_objcopy)
> > > -                     llvm_objcopy = "llvm-objcopy";
> > > -
> > > -             /* Use objcopy to add a .BTF section */
> > > -             snprintf(tmp_fn, sizeof(tmp_fn), "%s.btf", filename);
> > > -             close(fd);
> > > -             fd = creat(tmp_fn, S_IRUSR | S_IWUSR);
> > > -             if (fd == -1) {
> > > -                     fprintf(stderr, "%s: open(%s) failed!\n", __func__,
> > > -                             tmp_fn);
> > > +             /* Add ".BTF" to the section name string table */
> > > +             Elf_Data *str_data = elf_getdata(str_scn, NULL);
> > > +             if (!str_data) {
> > > +                     fprintf(stderr, "%s: elf_getdata(str_scn) failed: %s\n",
> > > +                             __func__, elf_errmsg(elf_errno()));
> > >                       goto out;
> > >               }
> > > -
> > > -             if (write(fd, raw_btf_data, raw_btf_size) != raw_btf_size) {
> > > -                     fprintf(stderr, "%s: write of %d bytes to '%s' failed: %d!\n",
> > > -                             __func__, raw_btf_size, tmp_fn, errno);
> > > -                     goto unlink;
> > > +             dot_btf_offset = str_data->d_size;
> > > +             size_t new_str_size = dot_btf_offset + 5;
> > > +             str_table = malloc(new_str_size);
> > > +             if (!str_table) {
> > > +                     fprintf(stderr, "%s: malloc(%zu) failed: %s\n", __func__,
> > > +                             new_str_size, elf_errmsg(elf_errno()));
> > > +                     goto out;
> > >               }
> > > +             memcpy(str_table, str_data->d_buf, dot_btf_offset);
> > > +             memcpy(str_table + dot_btf_offset, ".BTF", 5);
> >
> > hum, I wonder this will always copy the final zero byte
>
> It should, as strlen(".BTF") == 4.
>
> > > +             str_data->d_buf = str_table;
> > > +             str_data->d_size = new_str_size;
> > > +             elf_flagdata(str_data, ELF_C_SET, ELF_F_DIRTY);
> > > +
> > > +             /* Create a new section */
> > > +             btf_scn = elf_newscn(elf);
> > > +             if (!btf_scn) {
> > > +                     fprintf(stderr, "%s: elf_newscn failed: %s\n",
> > > +                     __func__, elf_errmsg(elf_errno()));
> > > +                     goto out;
> > > +             }
> > > +             btf_data = elf_newdata(btf_scn);
> > > +             if (!btf_data) {
> > > +                     fprintf(stderr, "%s: elf_newdata failed: %s\n",
> > > +                     __func__, elf_errmsg(elf_errno()));
> > > +                     goto out;
> > > +             }
> > > +     }
> > >
> > > -             snprintf(cmd, sizeof(cmd), "%s --add-section .BTF=%s %s",
> > > -                      llvm_objcopy, tmp_fn, filename);
> > > -             if (system(cmd)) {
> > > -                     fprintf(stderr, "%s: failed to add .BTF section to '%s': %d!\n",
> > > -                             __func__, filename, errno);
> > > -                     goto unlink;
> > > +     /* (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;
> >
> > doesn't this potentially leak btf_data->d_buf?
>
> I believe libelf owns the original btf_data->d_buf and it could even
> just be a pointer into mmaped memory,  but I will check.
>

FTR, that pointer *is* owned by libelf. If I free it, I get a
double-free warning later on.

> > > +     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 */
> > > +     GElf_Shdr btf_shdr_mem;
> > > +     GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem);
> > > +     if (!btf_shdr) {
> > > +             fprintf(stderr, "%s: elf_getshdr(btf_scn) failed: %s\n",
> > > +                     __func__, elf_errmsg(elf_errno()));
> > > +             goto out;
> > > +     }
> > > +     btf_shdr->sh_entsize = 0;
> > > +     btf_shdr->sh_flags = 0;
> > > +     if (dot_btf_offset)
> > > +             btf_shdr->sh_name = dot_btf_offset;
> > > +     btf_shdr->sh_type = SHT_PROGBITS;
> > > +     if (!gelf_update_shdr(btf_scn, btf_shdr)) {
> > > +             fprintf(stderr, "%s: gelf_update_shdr failed: %s\n",
> > > +                     __func__, elf_errmsg(elf_errno()));
> > > +             goto out;
> > > +     }
> > > +
> > > +     if (elf_update(elf, ELF_C_NULL) < 0) {
> > > +             fprintf(stderr, "%s: elf_update (layout) failed: %s\n",
> > > +                     __func__, elf_errmsg(elf_errno()));
> > > +             goto out;
> > > +     }
> > > +
> > > +     size_t phnum = 0;
> > > +     if (!elf_getphdrnum(elf, &phnum)) {
> > > +             for (size_t ix = 0; ix < phnum; ++ix) {
> > > +                     GElf_Phdr phdr;
> > > +                     GElf_Phdr *elf_phdr = gelf_getphdr(elf, ix, &phdr);
> > > +                     size_t filesz = gelf_fsize(elf, ELF_T_PHDR, 1, EV_CURRENT);
> > > +                     fprintf(stderr, "type: %d %d\n", elf_phdr->p_type, PT_PHDR);
> > > +                     fprintf(stderr, "offset: %lu %lu\n", elf_phdr->p_offset, ehdr->e_phoff);
> > > +                     fprintf(stderr, "filesize: %lu %lu\n", elf_phdr->p_filesz, filesz);
> >
> > looks like s forgotten debug or you're missing
> > btf_elf__verbose check for fprintf calls above
>
> Oops, forgotten debug.
>
> >
> > jirka
> >
>
> Thanks,
> Giuliano.
diff mbox series

Patch

diff --git a/libbtf.c b/libbtf.c
index 9f76283..fb8e043 100644
--- a/libbtf.c
+++ b/libbtf.c
@@ -699,6 +699,7 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 	uint32_t raw_btf_size;
 	int fd, err = -1;
 	size_t strndx;
+	void *str_table = NULL;
 
 	fd = open(filename, O_RDWR);
 	if (fd < 0) {
@@ -741,74 +742,128 @@  static int btf_elf__write(const char *filename, struct btf *btf)
 	}
 
 	/*
-	 * First we look if there was already a .BTF section to overwrite.
+	 * First we check if there is already a .BTF section present.
 	 */
-
 	elf_getshdrstrndx(elf, &strndx);
+	Elf_Scn *btf_scn = 0;
 	while ((scn = elf_nextscn(elf, scn)) != NULL) {
 		shdr = gelf_getshdr(scn, &shdr_mem);
 		if (shdr == NULL)
 			continue;
 		char *secname = elf_strptr(elf, strndx, shdr->sh_name);
 		if (strcmp(secname, ".BTF") == 0) {
-			btf_data = elf_getdata(scn, btf_data);
+			btf_scn = scn;
 			break;
 		}
 	}
 
-	raw_btf_data = btf__get_raw_data(btf, &raw_btf_size);
-
-	if (btf_data) {
-		/* Exisiting .BTF section found */
-		btf_data->d_buf = (void *)raw_btf_data;
-		btf_data->d_size = raw_btf_size;
-		elf_flagdata(btf_data, ELF_C_SET, ELF_F_DIRTY);
+	Elf_Scn *str_scn = elf_getscn(elf, strndx);
+	if (!str_scn) {
+		fprintf(stderr, "%s: elf_getscn(strndx) failed\n", __func__);
+		goto out;
+	}
 
-		if (elf_update(elf, ELF_C_NULL) >= 0 &&
-		    elf_update(elf, ELF_C_WRITE) >= 0)
-			err = 0;
-		else
-			fprintf(stderr, "%s: elf_update failed: %s.\n",
-				__func__, elf_errmsg(elf_errno()));
+	size_t dot_btf_offset = 0;
+	if (btf_scn) {
+		/* Existing .BTF section found */
+		btf_data = elf_getdata(btf_scn, NULL);
+		if (!btf_data) {
+			fprintf(stderr, "%s: elf_getdata failed: %s\n", __func__,
+				elf_errmsg(elf_errno()));
+			goto out;
+		}
 	} else {
-		const char *llvm_objcopy;
-		char tmp_fn[PATH_MAX];
-		char cmd[PATH_MAX * 2];
-
-		llvm_objcopy = getenv("LLVM_OBJCOPY");
-		if (!llvm_objcopy)
-			llvm_objcopy = "llvm-objcopy";
-
-		/* Use objcopy to add a .BTF section */
-		snprintf(tmp_fn, sizeof(tmp_fn), "%s.btf", filename);
-		close(fd);
-		fd = creat(tmp_fn, S_IRUSR | S_IWUSR);
-		if (fd == -1) {
-			fprintf(stderr, "%s: open(%s) failed!\n", __func__,
-				tmp_fn);
+		/* Add ".BTF" to the section name string table */
+		Elf_Data *str_data = elf_getdata(str_scn, NULL);
+		if (!str_data) {
+			fprintf(stderr, "%s: elf_getdata(str_scn) failed: %s\n",
+				__func__, elf_errmsg(elf_errno()));
 			goto out;
 		}
-
-		if (write(fd, raw_btf_data, raw_btf_size) != raw_btf_size) {
-			fprintf(stderr, "%s: write of %d bytes to '%s' failed: %d!\n",
-				__func__, raw_btf_size, tmp_fn, errno);
-			goto unlink;
+		dot_btf_offset = str_data->d_size;
+		size_t new_str_size = dot_btf_offset + 5;
+		str_table = malloc(new_str_size);
+		if (!str_table) {
+			fprintf(stderr, "%s: malloc(%zu) failed: %s\n", __func__,
+				new_str_size, elf_errmsg(elf_errno()));
+			goto out;
 		}
+		memcpy(str_table, str_data->d_buf, dot_btf_offset);
+		memcpy(str_table + dot_btf_offset, ".BTF", 5);
+		str_data->d_buf = str_table;
+		str_data->d_size = new_str_size;
+		elf_flagdata(str_data, ELF_C_SET, ELF_F_DIRTY);
+
+		/* Create a new section */
+		btf_scn = elf_newscn(elf);
+		if (!btf_scn) {
+			fprintf(stderr, "%s: elf_newscn failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+			goto out;
+		}
+		btf_data = elf_newdata(btf_scn);
+		if (!btf_data) {
+			fprintf(stderr, "%s: elf_newdata failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+			goto out;
+		}
+	}
 
-		snprintf(cmd, sizeof(cmd), "%s --add-section .BTF=%s %s",
-			 llvm_objcopy, tmp_fn, filename);
-		if (system(cmd)) {
-			fprintf(stderr, "%s: failed to add .BTF section to '%s': %d!\n",
-				__func__, filename, errno);
-			goto unlink;
+	/* (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_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 */
+	GElf_Shdr btf_shdr_mem;
+	GElf_Shdr *btf_shdr = gelf_getshdr(btf_scn, &btf_shdr_mem);
+	if (!btf_shdr) {
+		fprintf(stderr, "%s: elf_getshdr(btf_scn) failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+		goto out;
+	}
+	btf_shdr->sh_entsize = 0;
+	btf_shdr->sh_flags = 0;
+	if (dot_btf_offset)
+		btf_shdr->sh_name = dot_btf_offset;
+	btf_shdr->sh_type = SHT_PROGBITS;
+	if (!gelf_update_shdr(btf_scn, btf_shdr)) {
+		fprintf(stderr, "%s: gelf_update_shdr failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+		goto out;
+	}
+
+	if (elf_update(elf, ELF_C_NULL) < 0) {
+		fprintf(stderr, "%s: elf_update (layout) failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+		goto out;
+	}
+
+	size_t phnum = 0;
+	if (!elf_getphdrnum(elf, &phnum)) {
+		for (size_t ix = 0; ix < phnum; ++ix) {
+			GElf_Phdr phdr;
+			GElf_Phdr *elf_phdr = gelf_getphdr(elf, ix, &phdr);
+			size_t filesz = gelf_fsize(elf, ELF_T_PHDR, 1, EV_CURRENT);
+			fprintf(stderr, "type: %d %d\n", elf_phdr->p_type, PT_PHDR);
+			fprintf(stderr, "offset: %lu %lu\n", elf_phdr->p_offset, ehdr->e_phoff);
+			fprintf(stderr, "filesize: %lu %lu\n", elf_phdr->p_filesz, filesz);
 		}
+	}
 
-		err = 0;
-	unlink:
-		unlink(tmp_fn);
+	if (elf_update(elf, ELF_C_WRITE) < 0) {
+		fprintf(stderr, "%s: elf_update (write) failed: %s\n",
+			__func__, elf_errmsg(elf_errno()));
+		goto out;
 	}
+	err = 0;
 
 out:
+	if (str_table)
+		free(str_table);
 	if (fd != -1)
 		close(fd);
 	if (elf)