[ktap] Re: [PATCH] add new symbol module to search symbols in DSO (for uprobes)
- From: Yicheng <qycqycqycqycqyc@xxxxxxxxx>
- To: ktap@xxxxxxxxxxxxx
- Date: Mon, 4 Nov 2013 17:02:09 -0500
Looks good!
The main concern in my view is the performance issue for ktap vm
interrupter.
vm runs parse_events_resolve_symbol() to parse the symbol, but it may
be the unnecessary burden for vm because it can be calculated in parser
and written into bytecode. And I wonder whether or not the parse process
consumes a lot of time.
One suggestion about parse_events_resolve_symbol(): IMHO, strndup may
be executed after checking whether r is a number, or it may do useless
malloc operations when r is a number.
2013/11/4 Azat Khuzhin <a3at.mail@xxxxxxxxx>
> This will allow to don't hardcoding addresses of functions in ktap
> scripts, that use uprobes, but instead, use normal function names.
>
> Before this patch:
> $ ktap -e 'trace probe:/path/to/bin:0xDEADBEAF' # instead some real address
>
> After this path:
> $ ktap -e 'trace probe:/path/to/bin:foo'
>
> Works also for ret probes (%return)
> Requires libelf.
>
> Signed-off-by: Azat Khuzhin <a3at.mail@xxxxxxxxx>
> ---
> Makefile | 5 ++-
> include/ktap_types.h | 2 +
> userspace/eventdef.c | 34 +++++++++++++++
> userspace/symbol.c | 114
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> userspace/symbol.h | 32 +++++++++++++++
> 5 files changed, 186 insertions(+), 1 deletion(-)
> create mode 100644 userspace/symbol.c
> create mode 100644 userspace/symbol.h
>
> diff --git a/Makefile b/Makefile
> index 372b41a..344c82a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -59,6 +59,8 @@ $(UDIR)/tstring.o: $(INTP)/tstring.c $(INC)/*
> $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
> $(UDIR)/object.o: $(INTP)/object.c $(INC)/*
> $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
> +$(UDIR)/symbol.o: $(UDIR)/symbol.c $(INC)/*
> + $(QUIET_CC)$(CC) $(DEBUGINFO_FLAG) $(KTAPC_CFLAGS) -o $@ -c $<
>
> KTAPOBJS =
> KTAPOBJS += $(UDIR)/lex.o
> @@ -73,9 +75,10 @@ KTAPOBJS += $(UDIR)/opcode.o
> KTAPOBJS += $(UDIR)/table.o
> KTAPOBJS += $(UDIR)/tstring.o
> KTAPOBJS += $(UDIR)/object.o
> +KTAPOBJS += $(UDIR)/symbol.o
>
> ktap: $(KTAPOBJS)
> - $(QUIET_LINK)$(CC) $(KTAPC_CFLAGS) -o $@ $(KTAPOBJS) -lpthread
> + $(QUIET_LINK)$(CC) $(KTAPC_CFLAGS) -o $@ $(KTAPOBJS) -lpthread
> -lelf
>
> KMISC := /lib/modules/$(KVERSION)/ktapvm/
>
> diff --git a/include/ktap_types.h b/include/ktap_types.h
> index 1dae9d8..4e19f0b 100644
> --- a/include/ktap_types.h
> +++ b/include/ktap_types.h
> @@ -600,5 +600,7 @@ extern ktap_global_state dummy_global_state;
> #define KTAP_QL(x) "'" x "'"
> #define KTAP_QS KTAP_QL("%s")
>
> +#define STRINGIFY(type) #type
> +
> #endif /* __KTAP_TYPES_H__ */
>
> diff --git a/userspace/eventdef.c b/userspace/eventdef.c
> index 76a68ac..dab964a 100644
> --- a/userspace/eventdef.c
> +++ b/userspace/eventdef.c
> @@ -29,6 +29,7 @@
> #include "../include/ktap_types.h"
> #include "../include/ktap_opcodes.h"
> #include "ktapc.h"
> +#include "symbol.h"
>
> static char tracing_events_path[] = "/sys/kernel/debug/tracing/events";
>
> @@ -305,6 +306,35 @@ static int parse_events_add_kprobe(char *old_event)
>
> #define UPROBE_EVENTS_PATH "/sys/kernel/debug/tracing/uprobe_events"
>
> +static char* parse_events_resolve_symbol(char *event, char *r)
> +{
> + char *colon;
> + colon = strchr(event, ':');
> +
> + char *binary = strndup(event, colon - event);
> + char *symbol = strndup(colon + 1 /* skip ":" */,
> + r ? (r - 1 /* skip "%" */ - colon) : strlen((const char
> *)colon));
> +
> + /**
> + * Test whethere we have symbol address, or symbol name.
> + * If second, than we need to scan dso to resolve it.
> + */
> + vaddr_t symbol_address;
> + if ((sscanf(symbol, "%lx", &symbol_address) != 1) &&
> + (symbol_address = find_symbol(binary, symbol))) {
> + verbose_printf("symbol %s resolved to 0x%lx\n",
> + event, symbol_address);
> +
> + event = realloc(event, strlen(event) +
> (strlen(STRINGIFY(ULONG_MAX))));
> + sprintf(event, "%s:0x%lx", binary, symbol_address);
> + }
> +
> + free(binary);
> + free(symbol);
> +
> + return event;
> +}
> +
> static int parse_events_add_uprobe(char *old_event)
> {
> static int event_seq = 0;
> @@ -326,6 +356,10 @@ static int parse_events_add_uprobe(char *old_event)
> r = strstr(event, "%return");
> if (r) {
> memset(r, ' ', 7);
> + }
> + event = parse_events_resolve_symbol(event, r);
> +
> + if (r) {
> snprintf(probe_event, 128, "r:uprobes/kp%d %s",
> event_seq, event);
> } else
> diff --git a/userspace/symbol.c b/userspace/symbol.c
> new file mode 100644
> index 0000000..23333b8
> --- /dev/null
> +++ b/userspace/symbol.c
> @@ -0,0 +1,114 @@
> +/*
> + * symbol.c
> + *
> + * This file is part of ktap by Jovi Zhangwei.
> + *
> + * Copyright (C) 2013 Azat Khuzhin <a3at.mail@xxxxxxxxx>.
> + *
> + * ktap is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * ktap is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> along with
> + * this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include "symbol.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <string.h>
> +
> +#include <libelf.h>
> +
> +
> +/**
> + * @return v_addr of "LOAD" program header, that have zero offset.
> + */
> +static vaddr_t find_load_address(Elf *elf)
> +{
> + GElf_Phdr header;
> + size_t headers;
> + vaddr_t address = 0;
> +
> + elf_getphdrnum(elf, &headers);
> + while (headers-- > 0) {
> + gelf_getphdr(elf, headers, &header);
> + if (header.p_type != PT_LOAD || header.p_offset != 0)
> + continue;
> +
> + address = header.p_vaddr;
> + break;
> + }
> +
> + return address;
> +}
> +
> +static vaddr_t search_symbol(Elf *elf, const char *symbol)
> +{
> + Elf_Data *elf_data = NULL;
> + Elf_Scn *scn = NULL;
> + GElf_Sym sym;
> + GElf_Shdr shdr;
> +
> + vaddr_t load_address = find_load_address(elf);
> +
> + if (!load_address)
> + return 0;
> +
> + while ((scn = elf_nextscn(elf, scn))) {
> + int i, symbols;
> + char *current_symbol;
> +
> + gelf_getshdr(scn, &shdr);
> +
> + if (shdr.sh_type != SHT_SYMTAB)
> + continue;
> +
> + elf_data = elf_getdata(scn, elf_data);
> + symbols = shdr.sh_size / shdr.sh_entsize;
> +
> + for (i = 0; i < symbols; i++) {
> + gelf_getsym(elf_data, i, &sym);
> +
> + if (GELF_ST_TYPE(sym.st_info) != STT_FUNC)
> + continue;
> +
> + current_symbol = elf_strptr(elf, shdr.sh_link,
> sym.st_name);
> + if (!strcmp(current_symbol, symbol)) {
> + return (sym.st_value - load_address);
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +vaddr_t find_symbol(const char *exec, const char *symbol)
> +{
> + Elf *elf = NULL;
> + int fd = 0;
> + vaddr_t vaddr = 0;
> +
> + if (((fd = open(exec, O_RDONLY)) != -1) &&
> + (elf_version(EV_CURRENT) != EV_NONE) &&
> + (elf = elf_begin(fd, ELF_C_READ, NULL)))
> + vaddr = search_symbol(elf, symbol);
> +
> + if (elf)
> + elf_end(elf);
> + if (fd)
> + close(fd);
> + return vaddr;
> +}
> diff --git a/userspace/symbol.h b/userspace/symbol.h
> new file mode 100644
> index 0000000..478ad2e
> --- /dev/null
> +++ b/userspace/symbol.h
> @@ -0,0 +1,32 @@
> +/*
> + * symbol.h
> + *
> + * This file is part of ktap by Jovi Zhangwei.
> + *
> + * Copyright (C) 2013 Azat Khuzhin <a3at.mail@xxxxxxxxx>.
> + *
> + * ktap is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * ktap is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> along with
> + * this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + */
> +
> +#include <gelf.h> // GElf_Addr
> +
> +
> +typedef GElf_Addr vaddr_t;
> +
> +/**
> + * Find symbol in DSO
> + *
> + * @return 0 on failure, otherwise address of symbol.
> + */
> +vaddr_t find_symbol(const char *exec, const char *symbol);
> --
> 1.8.4.rc3
>
>
>
--
Sincerely,
Yicheng Qin
Other related posts:
