Re: [RFC][PATCH 2/3] objtool: Provide elf_write_{insn,reloc}()

2020-06-16 Thread Matt Helsley 
On Tue, Jun 16, 2020 at 11:12:53AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2020 at 04:30:36PM +0200, Peter Zijlstra wrote:
> > +int elf_write_insn(struct elf *elf, struct section *sec,
> > +  unsigned long offset, unsigned int len,
> > +  const char *insn)
> > +{
> > +   Elf_Data *data = sec->data;
> > +
> > +   if (data->d_type != ELF_T_BYTE || data->d_off) {
> > +   WARN("write to unexpected data for section: %s", sec->name);
> > +   return -1;
> > +   }
> > +
> > +   memcpy(data->d_buf + offset, insn, len);
> > +   elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY);
> > +
> > +   sec->changed = true;
> > +   elf->changed = true;
> > +
> > +   return 0;
> > +}
> > +
> > +int elf_write_reloc(struct elf *elf, struct reloc *reloc)
> > +{
> > +   struct section *sec = reloc->sec;
> > +
> > +   if (sec->sh.sh_type == SHT_REL) {
> > +   reloc->rel.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> > +   reloc->rel.r_offset = reloc->offset;
> > +
> > +   if (!gelf_update_rel(sec->data, reloc->idx, &reloc->rel)) {
> > +   WARN_ELF("gelf_update_rel");
> > +   return -1;
> > +   }
> > +   } else {
> > +   reloc->rela.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> > +   reloc->rela.r_addend = reloc->addend;
> > +   reloc->rela.r_offset = reloc->offset;
> > +
> > +   if (!gelf_update_rela(sec->data, reloc->idx, &reloc->rela)) {
> > +   WARN_ELF("gelf_update_rela");
> > +   return -1;
> > +   }
> > +   }
> > +
> > +   sec->changed = true;
> > +   elf->changed = true;
> > +
> > +   return 0;
> > +}
> 
> Doing the change Matt asked for #1, I realized that sec->changed is only
> required if we need to rewrite the section header, neither of these two
> changes requires that, they already mark the elf data dirty so
> elf_update() DTRT.

This is really useful information.

As long as you're adding the elf->changed flag it might make sense to add this
as a comment in the struct section definition or even rename sec->changed
to reflect this (e.g. sec->shdr_changed).

Cheers,
 -Matt Helsley

Re: [RFC][PATCH 2/3] objtool: Provide elf_write_{insn,reloc}()

2020-06-16 Thread Peter Zijlstra 
On Fri, Jun 12, 2020 at 04:30:36PM +0200, Peter Zijlstra wrote:
> +int elf_write_insn(struct elf *elf, struct section *sec,
> +unsigned long offset, unsigned int len,
> +const char *insn)
> +{
> + Elf_Data *data = sec->data;
> +
> + if (data->d_type != ELF_T_BYTE || data->d_off) {
> + WARN("write to unexpected data for section: %s", sec->name);
> + return -1;
> + }
> +
> + memcpy(data->d_buf + offset, insn, len);
> + elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY);
> +
> + sec->changed = true;
> + elf->changed = true;
> +
> + return 0;
> +}
> +
> +int elf_write_reloc(struct elf *elf, struct reloc *reloc)
> +{
> + struct section *sec = reloc->sec;
> +
> + if (sec->sh.sh_type == SHT_REL) {
> + reloc->rel.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> + reloc->rel.r_offset = reloc->offset;
> +
> + if (!gelf_update_rel(sec->data, reloc->idx, &reloc->rel)) {
> + WARN_ELF("gelf_update_rel");
> + return -1;
> + }
> + } else {
> + reloc->rela.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
> + reloc->rela.r_addend = reloc->addend;
> + reloc->rela.r_offset = reloc->offset;
> +
> + if (!gelf_update_rela(sec->data, reloc->idx, &reloc->rela)) {
> + WARN_ELF("gelf_update_rela");
> + return -1;
> + }
> + }
> +
> + sec->changed = true;
> + elf->changed = true;
> +
> + return 0;
> +}

Doing the change Matt asked for #1, I realized that sec->changed is only
required if we need to rewrite the section header, neither of these two
changes requires that, they already mark the elf data dirty so
elf_update() DTRT.

[RFC][PATCH 2/3] objtool: Provide elf_write_{insn,reloc}()

2020-06-12 Thread Peter Zijlstra 
This provides infrastructure to rewrite instructions; this is
immediately useful for helping out with KCOV-vs-noinstr, but will
also come in handy for a bunch of variable sized jump-label patches
that are still on ice.

Signed-off-by: Peter Zijlstra (Intel) 
---
 tools/objtool/elf.c |   52 +++-
 tools/objtool/elf.h |7 ++-
 2 files changed, 57 insertions(+), 2 deletions(-)

--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -562,8 +562,9 @@ static int read_relocs(struct elf *elf)
return -1;
}
 
-   reloc->sym = find_symbol_by_index(elf, symndx);
reloc->sec = sec;
+   reloc->idx = i;
+   reloc->sym = find_symbol_by_index(elf, symndx);
if (!reloc->sym) {
WARN("can't find reloc entry symbol %d for %s",
 symndx, sec->name);
@@ -848,6 +849,55 @@ static int elf_rebuild_rela_section(stru
 
return 0;
 }
+
+int elf_write_insn(struct elf *elf, struct section *sec,
+  unsigned long offset, unsigned int len,
+  const char *insn)
+{
+   Elf_Data *data = sec->data;
+
+   if (data->d_type != ELF_T_BYTE || data->d_off) {
+   WARN("write to unexpected data for section: %s", sec->name);
+   return -1;
+   }
+
+   memcpy(data->d_buf + offset, insn, len);
+   elf_flagdata(data, ELF_C_SET, ELF_F_DIRTY);
+
+   sec->changed = true;
+   elf->changed = true;
+
+   return 0;
+}
+
+int elf_write_reloc(struct elf *elf, struct reloc *reloc)
+{
+   struct section *sec = reloc->sec;
+
+   if (sec->sh.sh_type == SHT_REL) {
+   reloc->rel.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
+   reloc->rel.r_offset = reloc->offset;
+
+   if (!gelf_update_rel(sec->data, reloc->idx, &reloc->rel)) {
+   WARN_ELF("gelf_update_rel");
+   return -1;
+   }
+   } else {
+   reloc->rela.r_info = GELF_R_INFO(reloc->sym->idx, reloc->type);
+   reloc->rela.r_addend = reloc->addend;
+   reloc->rela.r_offset = reloc->offset;
+
+   if (!gelf_update_rela(sec->data, reloc->idx, &reloc->rela)) {
+   WARN_ELF("gelf_update_rela");
+   return -1;
+   }
+   }
+
+   sec->changed = true;
+   elf->changed = true;
+
+   return 0;
+}
 
 int elf_rebuild_reloc_section(struct section *sec)
 {
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -67,9 +67,10 @@ struct reloc {
};
struct section *sec;
struct symbol *sym;
-   unsigned int type;
unsigned long offset;
+   unsigned int type;
int addend;
+   int idx;
bool jump_table_start;
 };
 
@@ -122,6 +123,10 @@ struct elf *elf_open_read(const char *na
 struct section *elf_create_section(struct elf *elf, const char *name, size_t 
entsize, int nr);
 struct section *elf_create_reloc_section(struct elf *elf, struct section 
*base, int reltype);
 void elf_add_reloc(struct elf *elf, struct reloc *reloc);
+int elf_write_insn(struct elf *elf, struct section *sec,
+  unsigned long offset, unsigned int len,
+  const char *insn);
+int elf_write_reloc(struct elf *elf, struct reloc *reloc);
 int elf_write(struct elf *elf);
 void elf_close(struct elf *elf);