lkml.org 
[lkml]   [2012]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] jump labels: Add infrastructure to update jump labels at compile time
Hi Steven,

I really like this patchset! A few comments below,

* Steven Rostedt (rostedt@goodmis.org) wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Add the infrastructure to allow architectures to modify the jump label
> locations at compile time. This is mainly for x86, where the jmps may

is it "at compile time" or "after compilation" ? AFAIU, this
update_jump_label script gets executed on the .o, which makes it
technically more part of the linking stage than the compilation
stage.

> be either 2 bytes or 5 bytes. Instead of wasting 5 bytes for all jump labels,
> this code will let x86 put in a jmp instead of a 5 byte nop. Then the
> assembler will make either a 2 byte or 5 byte jmp depending on where
> the target is.
>
> At compile time, this code will replace the jmps with either a 2 byte
> or 5 byte nop depending on the size of the jmp that was added.
>

A small note somewhere saying that this 2 vs 5 bytes choice done by the
compiler is a pessimistic approximation would be good, so people don't
end up complaining if they see that the compiler chose a 5-byte nop for
a 120 bytes offset.

> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> Makefile | 7 +
> arch/Kconfig | 6 +
> scripts/Makefile | 1 +
> scripts/Makefile.build | 15 ++-
> scripts/update_jump_label.c | 335 +++++++++++++++++++++++++++++++++++++++++++
> scripts/update_jump_label.h | 283 ++++++++++++++++++++++++++++++++++++
> 6 files changed, 645 insertions(+), 2 deletions(-)
> create mode 100644 scripts/update_jump_label.c
> create mode 100644 scripts/update_jump_label.h
>
[...]
> diff --git a/scripts/update_jump_label.c b/scripts/update_jump_label.c
> new file mode 100644
> index 0000000..399d898
> --- /dev/null
> +++ b/scripts/update_jump_label.c
> @@ -0,0 +1,335 @@
> +/*
> + * update_jump_label.c: replace jmps with nops at compile time.
> + * Copyright 2010 Steven Rostedt <srostedt@redhat.com>, Red Hat Inc.
> + * Parsing of the elf file was influenced by recordmcount.c
> + * originally written by and copyright to John F. Reiser <jreiser@BitWagon.com>.
> + */
> +
> +/*
> + * Note, this code is originally designed for x86, but may be used by
> + * other archs to do the nop updates at compile time instead of at boot time.
> + * X86 uses this as an optimization, as jmps can be either 2 bytes or 5 bytes.
> + * Inserting a 2 byte where possible helps with both CPU performance and
> + * icache strain.
> + */
> +#include <sys/types.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <getopt.h>
> +#include <elf.h>
> +#include <fcntl.h>
> +#include <setjmp.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdarg.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +static int fd_map; /* File descriptor for file being modified. */
> +static struct stat sb; /* Remember .st_size, etc. */
> +static int mmap_failed; /* Boolean flag. */
> +
> +static void die(const char *err, const char *fmt, ...)
> +{
> + va_list ap;
> +
> + if (err)
> + perror(err);
> +
> + if (fmt) {
> + va_start(ap, fmt);
> + fprintf(stderr, "Fatal error: ");
> + vfprintf(stderr, fmt, ap);
> + fprintf(stderr, "\n");
> + va_end(ap);
> + }
> +
> + exit(1);
> +}
> +
> +static void usage(char **argv)
> +{
> + char *arg = argv[0];
> + char *p = arg+strlen(arg);

I'm pretty sure checkpatch will complain about the coding style of this
file. I won't point out the various nits, but there is a need for
cleanup before pulling it.

> +
> + while (p >= arg && *p != '/')
> + p--;
> + p++;
> +
> + printf("usage: %s file\n"
> + "\n", p);
> + exit(-1);
> +}

[...]

> +/*
> + * Read the relocation table again, but this time its called on sections
> + * that are not going to be traced. The mcount calls here will be converted

mcount calls -> jump labels

> + * into nops.
> + */
> +static void FUNC(nop_jump_label)(Elf_Shdr const *const relhdr,
> + Elf_Ehdr const *const ehdr,
> + const char *const txtname)
> +{
> + Elf_Shdr *const shdr0 = get_shdr(ehdr);
> + Elf_Sym const *sym0;
> + char const *str0;
> + Elf_Rel const *relp;
> + Elf_Rela const *relap;
> + Elf_Shdr const *const shdr = &shdr0[w(relhdr->sh_info)];
> + unsigned rel_entsize = w(relhdr->sh_entsize);
> + unsigned const nrel = _w(relhdr->sh_size) / rel_entsize;
> + int t;
> +
> + FUNC(get_sym_str_and_relp)(relhdr, ehdr, &sym0, &str0, &relp);
> +
> + for (t = nrel; t > 0; t -= 3) {
> + int ret = -1;
> +
> + relap = (Elf_Rela const *)relp;
> + printf("rel offset=%lx info=%lx sym=%lx type=%lx addend=%lx\n",
> + (long)relap->r_offset, (long)relap->r_info,
> + (long)ELF_R_SYM(relap->r_info),
> + (long)ELF_R_TYPE(relap->r_info),
> + (long)relap->r_addend);
> +
> + if (0 && make_nop)
> + ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);

if (0 && -> leftover code ?

This whole FUNC(nop_jump_label) function seems to be unused.

> +
> + /* jump label sections are paired in threes */
> + relp = (Elf_Rel const *)(rel_entsize * 3 + (void *)relp);
> + }
> +}
> +
> +/* Find relocation section hdr for a given section */
> +static const Elf_Shdr *
> +FUNC(find_relhdr)(const Elf_Ehdr *ehdr, const Elf_Shdr *shdr)
> +{
> + const Elf_Shdr *shdr0 = get_shdr(ehdr);
> + int nhdr = w2(ehdr->e_shnum);
> + const Elf_Shdr *hdr;
> + int i;
> +
> + for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
> + if (w(hdr->sh_type) != SHT_REL &&
> + w(hdr->sh_type) != SHT_RELA)
> + continue;
> +
> + /*
> + * The relocation section's info field holds
> + * the section index that it represents.
> + */
> + if (shdr == &shdr0[w(hdr->sh_info)])
> + return hdr;
> + }
> + return NULL;
> +}
> +
> +/* Find a section headr based on name and type */
> +static const Elf_Shdr *
> +FUNC(find_shdr)(const Elf_Ehdr *ehdr, const char *name, uint_t type)
> +{
> + const Elf_Shdr *shdr0 = get_shdr(ehdr);
> + const Elf_Shdr *shstr = &shdr0[w2(ehdr->e_shstrndx)];
> + const char *shstrtab = (char *)get_section_loc(ehdr, shstr);
> + int nhdr = w2(ehdr->e_shnum);
> + const Elf_Shdr *hdr;
> + const char *hdrname;
> + int i;
> +
> + for (hdr = shdr0, i = 0; i < nhdr; hdr = &shdr0[++i]) {
> + if (w(hdr->sh_type) != type)
> + continue;
> +
> + /* If we are just looking for a section by type (ie. SYMTAB) */
> + if (!name)
> + return hdr;
> +
> + hdrname = &shstrtab[w(hdr->sh_name)];
> + if (strcmp(hdrname, name) == 0)
> + return hdr;
> + }
> + return NULL;
> +}
> +
> +static void
> +FUNC(section_update)(const Elf_Ehdr *ehdr, const Elf_Shdr *symhdr,
> + unsigned shtype, const Elf_Rel *rel, void *data)
> +{
> + const Elf_Shdr *shdr0 = get_shdr(ehdr);
> + const Elf_Shdr *targethdr;
> + const Elf_Rela *rela;
> + const Elf_Sym *syment;
> + uint_t offset = _w(rel->r_offset);
> + uint_t info = _w(rel->r_info);
> + uint_t sym = ELF_R_SYM(info);
> + uint_t type = ELF_R_TYPE(info);
> + uint_t addend;
> + uint_t targetloc;
> +
> + if (shtype == SHT_RELA) {
> + rela = (const Elf_Rela *)rel;
> + addend = _w(rela->r_addend);
> + } else
> + addend = _w(*(int *)(data + offset));
> +
> + syment = (const Elf_Sym *)get_section_loc(ehdr, symhdr);
> + targethdr = &shdr0[w2(syment[sym].st_shndx)];
> + targetloc = _w(targethdr->sh_offset);
> +
> + /* TODO, need a separate function for all archs */

maybe this is where you want to call FUNC(nop_jump_label) too ?

> + if (type != R_386_32)
> + die(NULL, "Arch relocation type %d not supported", type);
> +
> + targetloc += addend;
> +
> +#if 0
> + printf("offset=%x target=%x shoffset=%x add=%x indx=%x\n",
> + offset, targetloc, _w(targethdr->sh_offset), addend,
> + targetloc - _w(targethdr->sh_offset));
> +#endif
> + *(uint_t *)(data + offset) = targetloc;
> +}
> +
> +/* Overall supervision for Elf32 ET_REL file. */
> +static void
> +FUNC(do_func)(Elf_Ehdr *ehdr, char const *const fname, unsigned const reltype)
> +{
> + const Elf_Shdr *jlshdr;
> + const Elf_Shdr *jlrhdr;
> + const Elf_Shdr *symhdr;
> + const Elf_Rel *rel;
> + unsigned size;
> + unsigned cnt;
> + unsigned i;
> + uint_t type;
> + void *jdata;
> + void *data;
> +
> + jlshdr = FUNC(find_shdr)(ehdr, "__jump_table", SHT_PROGBITS);
> + if (!jlshdr)
> + return;
> +
> + jlrhdr = FUNC(find_relhdr)(ehdr, jlshdr);
> + if (!jlrhdr)
> + return;
> +
> + /*
> + * Create and fill in the __jump_table section and use it to
> + * find the offsets into the text that we want to update.
> + * We create it so that we do not depend on the order of the
> + * relocations, and use the table directly, as it is broken
> + * up into sections.
> + */
> + size = _w(jlshdr->sh_size);
> + data = umalloc(size);
> +
> + jdata = (void *)get_section_loc(ehdr, jlshdr);
> + memcpy(data, jdata, size);
> +
> + cnt = _w(jlrhdr->sh_size) / w(jlrhdr->sh_entsize);
> +
> + rel = (const Elf_Rel *)get_section_loc(ehdr, jlrhdr);
> +
> + /* Is this as Rel or Rela? */
> + type = w(jlrhdr->sh_type);
> +
> + symhdr = FUNC(find_shdr)(ehdr, NULL, SHT_SYMTAB);
> +
> + for (i = 0; i < cnt; i++) {
> + FUNC(section_update)(ehdr, symhdr, type, rel, data);
> + rel = (void *)rel + w(jlrhdr->sh_entsize);
> + }
> +
> + /*
> + * This is specific to x86. The jump_table is stored in three
> + * long words. The first is the location of the jmp target we
> + * must update.
> + */
> + cnt = size / sizeof(uint_t);
> +
> + for (i = 0; i < cnt; i += 3)
> + make_nop((void *)ehdr, *(uint_t *)(data + i * sizeof(uint_t)));
> +

Why is this function iterating twice on the __jump_table section rather
than simply writing the nops in the first pass, within the
section_update function ?

Thanks,

Mathieu


> + free(data);
> +}
> --
> 1.7.8.3
>
>

--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com


\
 
 \ /
  Last update: 2012-01-19 15:27    [W:1.164 / U:0.292 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site