Re: [PATCH -tip v3 1/2] kcov: Make runtime functions noinstr-compatible
From: Peter Zijlstra
Date: Thu Jun 11 2020 - 17:58:23 EST
On Thu, Jun 11, 2020 at 11:55:38PM +0200, Peter Zijlstra wrote:
> On Mon, Jun 08, 2020 at 01:01:08PM +0200, Peter Zijlstra wrote:
> > On Mon, Jun 08, 2020 at 09:57:39AM +0200, Dmitry Vyukov wrote:
> >
> > > As a crazy idea: is it possible to employ objtool (linker script?) to
> > > rewrite all coverage calls to nops in the noinstr section? Or relocate
> > > to nop function?
> > > What we are trying to do is very static, it _should_ have been done
> > > during build. We don't have means in existing _compilers_ to do this,
> > > but maybe we could do it elsewhere during build?...
> >
> > Let me try and figure out how to make objtool actually rewrite code.
>
> The below is quite horrific but seems to sorta work.
>
>
> It turns this:
>
> 12: e8 00 00 00 00 callq 17 <lockdep_hardirqs_on+0x17>
> 13: R_X86_64_PLT32 __sanitizer_cov_trace_pc-0x4
>
> Into this:
>
> 12: 90 nop
> 13: 90 nop
> 13: R_X86_64_NONE __sanitizer_cov_trace_pc-0x4
> 14: 90 nop
> 15: 90 nop
> 16: 90 nop
>
>
> I'll have to dig around a little more to see if I can't get rid of the
> relocation entirely. Also, I need to steal better arch_nop_insn() from
> the kernel :-)
Damn, paste buffer confusion, this is the right version.
---
tools/objtool/arch.h | 2 ++
tools/objtool/arch/x86/decode.c | 24 ++++++++++++++++++++++
tools/objtool/check.c | 17 +++++++++++++++-
tools/objtool/elf.c | 45 ++++++++++++++++++++++++++++++++++++++++-
tools/objtool/elf.h | 11 ++++++++--
5 files changed, 95 insertions(+), 4 deletions(-)
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index eda15a5a285e..3c5967748abb 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -84,4 +84,6 @@ unsigned long arch_jump_destination(struct instruction *insn);
unsigned long arch_dest_rela_offset(int addend);
+const char *arch_nop_insn(int len);
+
#endif /* _ARCH_H */
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 4b504fc90bbb..b615c32e21db 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -565,3 +565,27 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
state->regs[16].base = CFI_CFA;
state->regs[16].offset = -8;
}
+
+const char *arch_nop_insn(int len)
+{
+ static const char insn[16] = {
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ 0x90,
+ };
+
+ return insn;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 5fbb90a80d23..6f5e9c9465cb 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -765,6 +765,19 @@ static int add_call_destinations(struct objtool_file *file)
} else
insn->call_dest = rela->sym;
+ if (insn->sec->noinstr &&
+ !strncmp(insn->call_dest->name, "__sanitizer_cov_", 16)) {
+ if (rela) {
+ rela->type = R_X86_64_NONE;
+ elf_write_rela(file->elf, rela);
+ }
+
+ elf_write_insn(file->elf, insn->sec,
+ insn->offset, insn->len,
+ arch_nop_insn(insn->len));
+ insn->type = INSN_NOP;
+ }
+
/*
* Whatever stack impact regular CALLs have, should be undone
* by the RETURN of the called function.
@@ -2802,11 +2815,13 @@ int check(const char *_objname, bool orc)
if (ret < 0)
goto out;
+ }
+
+ if (file.elf->changed) {
ret = elf_write(file.elf);
if (ret < 0)
goto out;
}
-
out:
if (ret < 0) {
/*
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 84225679f96d..6f8021a5076a 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -525,6 +525,7 @@ static int read_relas(struct elf *elf)
return -1;
}
+ rela->idx = i;
rela->type = GELF_R_TYPE(rela->rela.r_info);
rela->addend = rela->rela.r_addend;
rela->offset = rela->rela.r_offset;
@@ -713,6 +714,8 @@ struct section *elf_create_section(struct elf *elf, const char *name,
elf_hash_add(elf->section_hash, &sec->hash, sec->idx);
elf_hash_add(elf->section_name_hash, &sec->name_hash, str_hash(sec->name));
+ elf->changed = true;
+
return sec;
}
@@ -779,7 +782,43 @@ int elf_rebuild_rela_section(struct section *sec)
return 0;
}
-int elf_write(const struct elf *elf)
+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) {
+ printf("ponies\n");
+ return -1;
+ }
+
+ memcpy(sec->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_rela(struct elf *elf, struct rela *rela)
+{
+ struct section *sec = rela->sec;
+
+ rela->rela.r_info = GELF_R_INFO(rela->sym->idx, rela->type);
+ rela->rela.r_addend = rela->addend;
+
+ gelf_update_rela(sec->data, rela->idx, &rela->rela);
+
+ sec->changed = true;
+ elf->changed = true;
+
+ return 0;
+}
+
+int elf_write(struct elf *elf)
{
struct section *sec;
Elf_Scn *s;
@@ -796,6 +835,8 @@ int elf_write(const struct elf *elf)
WARN_ELF("gelf_update_shdr");
return -1;
}
+
+ sec->changed = false;
}
}
@@ -808,6 +849,8 @@ int elf_write(const struct elf *elf)
return -1;
}
+ elf->changed = false;
+
return 0;
}
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index f4fe1d6ea392..4a3fe4f455c5 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -64,9 +64,10 @@ struct rela {
GElf_Rela rela;
struct section *sec;
struct symbol *sym;
- unsigned int type;
unsigned long offset;
+ unsigned int type;
int addend;
+ int idx;
bool jump_table_start;
};
@@ -76,6 +77,7 @@ struct elf {
Elf *elf;
GElf_Ehdr ehdr;
int fd;
+ bool changed;
char *name;
struct list_head sections;
DECLARE_HASHTABLE(symbol_hash, ELF_HASH_BITS);
@@ -118,7 +120,7 @@ struct elf *elf_open_read(const char *name, int flags);
struct section *elf_create_section(struct elf *elf, const char *name, size_t entsize, int nr);
struct section *elf_create_rela_section(struct elf *elf, struct section *base);
void elf_add_rela(struct elf *elf, struct rela *rela);
-int elf_write(const struct elf *elf);
+int elf_write(struct elf *elf);
void elf_close(struct elf *elf);
struct section *find_section_by_name(const struct elf *elf, const char *name);
@@ -132,6 +134,11 @@ struct rela *find_rela_by_dest_range(const struct elf *elf, struct section *sec,
struct symbol *find_func_containing(struct section *sec, unsigned long offset);
int elf_rebuild_rela_section(struct section *sec);
+int elf_write_rela(struct elf *elf, struct rela *rela);
+int elf_write_insn(struct elf *elf, struct section *sec,
+ unsigned long offset, unsigned int len,
+ const char *insn);
+
#define for_each_sec(file, sec) \
list_for_each_entry(sec, &file->elf->sections, list)