Re: [PATCH dwarves v2 1/4] btf_encoder: Add .BTF section using libelf
From: Andrii Nakryiko <hidden>
Date: 2021-02-04 04:11:17
On Mon, Feb 1, 2021 at 9:26 AM Giuliano Procida [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 and indeed offsets of existing sections are likely to change. Signed-off-by: Giuliano Procida <redacted> --- libbtf.c | 134 ++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 46 deletions(-)diff --git a/libbtf.c b/libbtf.c index 81b1b36..5b91d3a 100644 --- a/libbtf.c +++ b/libbtf.c@@ -698,6 +698,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) {@@ -740,74 +741,115 @@ 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, not 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__);
no elf_errmsg(elf_errno()) here? BTW, this form is very common (and a
bit verbose), so how about having a local macro that would make this
shorter, e.g.:
elf_error("elf_getscn(strndx) failed"); ?
+ 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;
5 is a bit magical, maybe use sizeof(".BTF") or a dedicated constant?
+ str_table = malloc(new_str_size);
+ if (!str_table) {
+ fprintf(stderr, "%s: malloc (strtab) failed\n", __func__);
+ 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;
+ memcpy(str_table, str_data->d_buf, dot_btf_offset);
+ memcpy(str_table + dot_btf_offset, ".BTF", 5);same about magical 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;
}
+ }
- err = 0;
- unlink:
- unlink(tmp_fn);
+ /* (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) {
btf_shdr just points to btf_shdr_mem, no? This duplication is not
pretty, why not:
GElf_Shdr btf_shdr;
if (!gelf_getshdr(btf_scn, &btf_shdr_mem)) { ... }
And then use btf_shdr. everywhere below
+ 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 = SHF_ALLOC;
this is wrong, making .BTF allocatable should be an opt-in, not all applications need to have a loadable .BTF section. Plus this patch doesn't really make it loadable, so SHF_ALLOC should be updated in the later patch. And I don't think we'll use that for vmlinux BTF or kernel module BTFs either, because there is still going to be linker script involved.
+ 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;
+ }
+
+ 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)
--
2.30.0.365.g02bc693789-goog