* Re: [PATCH v2] trace-cmd: Save the tracee memory map into the trace.dat file.
2019-06-18 14:32 [PATCH v2] trace-cmd: Save the tracee memory map into the trace.dat file tz.stoyanov
@ 2019-06-18 21:29 ` Steven Rostedt
2019-06-19 6:28 ` Yordan Karadzhov (VMware)
1 sibling, 0 replies; 3+ messages in thread
From: Steven Rostedt @ 2019-06-18 21:29 UTC (permalink / raw)
To: tz.stoyanov; +Cc: y.karadz, linux-trace-devel
On Tue, 18 Jun 2019 17:32:45 +0300
tz.stoyanov@gmail.com wrote:
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> [
> v2 changes:
> - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API.
> - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used.
> - Return error in case memory allocation fails.
> - Return error if option string is not in the expected format.
> - Sort memory maps and use binary search to find matching library in the map.
> ]
Hi Tzvetomir!
Note, "v2 changes" text should be below the 3 lines after the SOB.
>
> A new trace-cmd record option is added: "--mmap". When it is set with
> combination of -F or -P options, the memory map of the traced applications
> is stored in the trace.dat file. A new API tracecmd_search_tracee_mmap()
> can be used to look up into stored memory maps. The map is retrieved from
> /proc/<pid>/maps file.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
v2 changes:
- Replaced usage of tracecmd_add_option_v() with
tracecmd_add_option() API.
- Added checks to prevent buffer overflow when sscanf (... "%s",
buf) is used.
- Return error in case memory allocation fails.
- Return error if option string is not in the expected format.
- Sort memory maps and use binary search to find matching library
in the map.
This way a "git am" will ignore it. This is what the Linux kernel
community usually does.
> include/trace-cmd/trace-cmd.h | 4 +
> lib/trace-cmd/trace-input.c | 140 ++++++++++++++++++++++++++++++++-
> tracecmd/include/trace-local.h | 16 ++++
> tracecmd/trace-record.c | 137 ++++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+), 3 deletions(-)
>
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 6f62ab9..210be03 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -82,6 +82,7 @@ enum {
> TRACECMD_OPTION_OFFSET,
> TRACECMD_OPTION_CPUCOUNT,
> TRACECMD_OPTION_VERSION,
> + TRACECMD_OPTION_PIDMMAPS,
> };
>
> enum {
> @@ -208,6 +209,9 @@ unsigned long long tracecmd_page_ts(struct tracecmd_input *handle,
> unsigned int tracecmd_record_ts_delta(struct tracecmd_input *handle,
> struct tep_record *record);
>
> +char *tracecmd_get_tracee_lib(struct tracecmd_input *handle,
> + int pid, unsigned long addr);
Hmm, I rather not call it "tracee", that's a bit slang.
Perhaps just call it "task"
tracecmd_get_task_lib()
> +
> #ifndef SWIG
> /* hack for function graph work around */
> extern __thread struct tracecmd_input *tracecmd_curr_thread_handle;
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 61566ba..e2a06a8 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -101,6 +101,7 @@ struct tracecmd_input {
> struct tracecmd_ftrace finfo;
>
> struct hook_list *hooks;
> + struct pid_mem_maps *pid_mmaps;
> /* file information */
> size_t header_files_start;
> size_t ftrace_files_start;
> @@ -2134,6 +2135,138 @@ void tracecmd_set_ts2secs(struct tracecmd_input *handle,
> handle->use_trace_clock = false;
> }
>
> +static int trace_pid_mmap_cmp(const void *a, const void *b)
> +{
> + struct lib_mem_map *map_a = (struct lib_mem_map *)a;
> + struct lib_mem_map *map_b = (struct lib_mem_map *)b;
> +
> + if (map_a->start > map_b->start)
> + return 1;
> + if (map_a->start < map_b->start)
> + return -1;
> + return 0;
> +}
> +
> +#define STR_MMAP_LINE_MAX (PATH_MAX+34)
> +static int trace_pid_mmap_load(struct tracecmd_input *handle, char *buf)
> +{
> + struct pid_mem_maps *maps = NULL;
> + char mapname[STR_MMAP_LINE_MAX];
> + char *line;
> + int ret;
> + int i;
> +
> + maps = calloc(1, sizeof(*maps));
> + if (!maps)
> + return -ENOMEM;
> +
> + line = strchr(buf, '\n');
> + if (!line) {
> + free(maps);
> + return -EINVAL;
> + }
> + *line = '\0';
> + if (strlen(buf) > STR_MMAP_LINE_MAX) {
> + free(maps);
> + return -EINVAL;
> + }
> + ret = sscanf(buf, "%x %x %s", &maps->pid, &maps->nr_lib_maps, mapname);
> + if (ret != 3) {
> + free(maps);
> + return -EINVAL;
> + }
> + maps->proc_name = strdup(mapname);
> + if (!maps->proc_name) {
> + free(maps);
> + return -ENOMEM;
> + }
> + maps->lib_maps = calloc(maps->nr_lib_maps, sizeof(struct lib_mem_map));
> + if (!maps->lib_maps) {
> + free(maps->proc_name);
> + free(maps);
> + return -ENOMEM;
> + }
BTW, when you have a lot failure paths like this, you can make do the
following:
ret = -EINVAL;
line = strchr(buf, '\n');
if (!line)
goto out_fail;
[..]
if (strlen(buf) > STR_MMAP_LINE_MAX)
goto out_fail;
[..]
if (ret != 3)
goto out_fail;
ret = -ENOMEM;
maps->proc_name = strdup(mapname);
if (!maps->proc_name)
goto out_fail;
[..]
if (!maps->lib_maps)
goto out_fail;
> + maps->next = handle->pid_mmaps;
> + handle->pid_mmaps = maps;
> + buf = line+1;
> + line = strchr(buf, '\n');
> + for (i = 0; i < maps->nr_lib_maps; i++) {
> + if (!line)
> + break;
> + *line = '\0';
> + if (strlen(buf) > STR_MMAP_LINE_MAX)
> + break;
> + ret = sscanf(buf, "%llx %llx %s", &maps->lib_maps[i].start,
> + &maps->lib_maps[i].end, mapname);
> + if (ret == 3) {
> + maps->lib_maps[i].lib_name = strdup(mapname);
> + if (!maps->lib_maps[i].lib_name)
> + return -ENOMEM;
Hmm, perhaps we should wait to update handle->pid_maps until after
this, and have this goto out_fail as well.
> + }
> + buf = line+1;
> + line = strchr(buf, '\n');
> + }
> +
> + if (i != maps->nr_lib_maps)
Here too.
> + return -EINVAL;
> +
> + qsort(maps->lib_maps, maps->nr_lib_maps,
> + sizeof(*maps->lib_maps), trace_pid_mmap_cmp);
Here we should update handle->pid_mmaps = maps;
> +
> + return 0;
Here we have:
out_fail:
free(maps->lib_maps);
free(maps->proc_name);
free(maps);
return ret;
> +}
> +
> +static int trace_pid_mmap_search(const void *a, const void *b)
> +{
> + struct lib_mem_map *key = (struct lib_mem_map *)a;
> + struct lib_mem_map *map = (struct lib_mem_map *)b;
> +
> + if (key->start > map->end)
> + return 1;
> + if (key->start < map->start)
> + return -1;
> + return 0;
> +}
> +
> +/**
> + * tracecmd_search_tracee_mmap - Search tracee memory address map
Probably want to call this tracecmd_search_task_mmap().
> + * @handle: input handle to the trace.dat file
> + * @pid: pid of the tracee
of the task
> + * @addr: address from the tracee memory space.
> + *
> + * Map of the tracee memory can be saved in the trace.dat file, using the option
> + * "--mmap". If there is such information, this API can be used to look up into
> + * this memory map to find library is loaded at the given @addr.
> + *
> + * The name of the library at given tracee @addr is returned.
> + */
> +char *tracecmd_search_tracee_mmap(struct tracecmd_input *handle,
> + int pid, unsigned long long addr)
> +{
> + struct pid_mem_maps *maps;
> + struct lib_mem_map *lib;
> + struct lib_mem_map key;
> +
> + if (!handle || !handle->pid_mmaps)
> + return NULL;
> +
> + maps = handle->pid_mmaps;
> + while (maps) {
> + if (maps->pid == pid)
> + break;
> + maps = maps->next;
> + }
> + if (!maps || !maps->nr_lib_maps || !maps->lib_maps)
> + return NULL;
> + key.start = addr;
> + lib = bsearch(&key, maps->lib_maps, maps->nr_lib_maps,
> + sizeof(*maps->lib_maps), trace_pid_mmap_search);
> + if (lib)
> + return lib->lib_name;
> +
> + return NULL;
> +}
> +
> static int handle_options(struct tracecmd_input *handle)
> {
> unsigned long long offset;
> @@ -2221,9 +2354,6 @@ static int handle_options(struct tracecmd_input *handle)
> case TRACECMD_OPTION_UNAME:
> handle->uname = strdup(buf);
> break;
> - case TRACECMD_OPTION_VERSION:
> - handle->version = strdup(buf);
> - break;
> case TRACECMD_OPTION_HOOK:
> hook = tracecmd_create_event_hook(buf);
> hook->next = handle->hooks;
> @@ -2233,6 +2363,10 @@ static int handle_options(struct tracecmd_input *handle)
> cpus = *(int *)buf;
> handle->cpus = tep_read_number(handle->pevent, &cpus, 4);
> break;
> + case TRACECMD_OPTION_PIDMMAPS:
> + if (buf[size] == '\0')
> + trace_pid_mmap_load(handle, buf);
> + break;
> default:
> warning("unknown option %d", option);
> break;
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index 1cad3cc..dc4dc03 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -157,6 +157,20 @@ struct func_list {
> const char *mod;
> };
>
> +struct lib_mem_map {
> + unsigned long long start;
> + unsigned long long end;
> + char *lib_name;
> +};
> +
> +struct pid_mem_maps {
> + struct pid_mem_maps *next;
> + struct lib_mem_map *lib_maps;
> + unsigned int nr_lib_maps;
> + char *proc_name;
> + int pid;
> +};
> +
> struct buffer_instance {
> struct buffer_instance *next;
> const char *name;
> @@ -183,6 +197,8 @@ struct buffer_instance {
> struct tracecmd_msg_handle *msg_handle;
> struct tracecmd_output *network_handle;
>
> + struct pid_mem_maps *mem_maps;
> +
> char *max_graph_depth;
>
> int flags;
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 5dc6f17..a938ce3 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -84,6 +84,7 @@ static int max_kb;
> static bool use_tcp;
>
> static int do_ptrace;
> +static int get_mmap;
>
> static int filter_task;
> static int filter_pid = -1;
> @@ -1062,6 +1063,98 @@ static char *make_pid_filter(char *curr_filter, const char *field)
> return filter;
> }
>
> +static int get_pid_mmaps(int pid)
> +{
> + struct buffer_instance *instance = &top_instance;
> + struct pid_mem_maps *maps = instance->mem_maps;
> + unsigned long long begin, end, inode, tmp;
> + struct lib_mem_map *map;
> + char mapname[PATH_MAX+1];
> + char fname[PATH_MAX+1];
> + char buf[PATH_MAX+100];
> + char perm[5];
> + char dev[6];
> + FILE *f;
> + int ret;
> + int res;
> + int i;
> +
> + while (maps) {
> + if (pid == maps->pid)
> + break;
> + maps = maps->next;
> + }
BTW, each time I look at this code, I think this is a bug because you
don't check for (!maps) at the end of this loop. But then notice below
you do. Let's move this while loop to just before the:
if (!maps) {
As it makes more sense to keep them together, and less likely that we
think maps is used as NULL.
> +
> + sprintf(fname, "/proc/%d/exe", pid);
> + ret = readlink(fname, mapname, PATH_MAX);
> + if (ret >= PATH_MAX || ret < 0)
> + return -ENOENT;
> +
> + sprintf(fname, "/proc/%d/maps", pid);
> + f = fopen(fname, "r");
> + if (!f)
> + return -ENOENT;
> +
> + if (!maps) {
> + maps = calloc(1, sizeof(*maps));
> + if (!maps)
> + return -ENOMEM;
> + maps->pid = pid;
> + maps->next = instance->mem_maps;
> + instance->mem_maps = maps;
> + } else {
> + for (i = 0; i < maps->nr_lib_maps; i++)
> + free(maps->lib_maps[i].lib_name);
> + free(maps->lib_maps);
> + maps->lib_maps = NULL;
> + maps->nr_lib_maps = 0;
> + free(maps->proc_name);
> + }
> +
> + maps->proc_name = strdup(mapname);
> + if (!maps->proc_name)
> + return -ENOMEM;
We need to close 'f' before we return an error. Let's do:
ret = -ENOMEM;
maps->proc_name = strdup(mapname);
if (!maps->proc_name)
goto out;
> +
> + ret = 0;
> + while (fgets(buf, sizeof(buf), f)) {
> + mapname[0] = '\0';
> + res = sscanf(buf, "%llx-%llx %4s %llx %5s %lld %s",
> + &begin, &end, perm, &tmp, dev, &inode, mapname);
> + if (res == 7 && mapname[0] != '\0') {
> + map = realloc(maps->lib_maps,
> + (maps->nr_lib_maps+1)*sizeof(*map));
> + if (!map) {
> + ret = -ENOMEM;
> + break;
> + }
> + map[maps->nr_lib_maps].end = end;
> + map[maps->nr_lib_maps].start = begin;
> + map[maps->nr_lib_maps].lib_name = strdup(mapname);
> + if (!map[maps->nr_lib_maps].lib_name) {
> + ret = -ENOMEM;
> + free(map);
> + break;
I'm thinking on error, we goto "out_fail" and remove all maps.
> + }
> + maps->lib_maps = map;
> + maps->nr_lib_maps++;
> + }
> + }
out:
> + fclose(f);
> +
> + return ret;
out_fail:
for (i = 0; i < maps->nr_lib_maps; i++) {
free(maps->lib_maps[i].lib_name);
free(maps->lib_maps);
But we would also need to remove it from the instance->mem_maps list.
-- Steve
> +}
> +
> +static void get_filter_pid_mmaps(void)
> +{
> + struct filter_pids *p;
> +
> + for (p = filter_pids; p; p = p->next) {
> + if (p->exclude)
> + continue;
> + get_pid_mmaps(p->pid);
> + }
> +}
> +
> static void update_task_filter(void)
> {
> struct buffer_instance *instance;
> @@ -1070,6 +1163,9 @@ static void update_task_filter(void)
> if (no_filter)
> return;
>
> + if (get_mmap && filter_pids)
> + get_filter_pid_mmaps();
> +
> if (filter_task)
> add_filter_pid(pid, 0);
>
> @@ -1264,6 +1360,8 @@ static void ptrace_wait(enum trace_type type, int main_pid)
> break;
>
> case PTRACE_EVENT_EXIT:
> + if (get_mmap)
> + get_pid_mmaps(main_pid);
> ptrace(PTRACE_GETEVENTMSG, pid, NULL, &cstatus);
> ptrace(PTRACE_DETACH, pid, NULL, NULL);
> break;
> @@ -3094,6 +3192,33 @@ static void append_buffer(struct tracecmd_output *handle,
> }
> }
>
> +
> +static void
> +add_pid_mem_maps(struct tracecmd_output *handle, struct buffer_instance *instance)
> +{
> + struct pid_mem_maps *maps = instance->mem_maps;
> + struct trace_seq s;
> + int i;
> +
> + trace_seq_init(&s);
> + while (maps) {
> + if (!maps->nr_lib_maps)
> + continue;
> + trace_seq_reset(&s);
> + trace_seq_printf(&s, "%x %x %s\n",
> + maps->pid, maps->nr_lib_maps, maps->proc_name);
> + for (i = 0; i < maps->nr_lib_maps; i++)
> + trace_seq_printf(&s, "%llx %llx %s\n",
> + maps->lib_maps[i].start,
> + maps->lib_maps[i].end,
> + maps->lib_maps[i].lib_name);
> + tracecmd_add_option(handle, TRACECMD_OPTION_PIDMMAPS,
> + s.len+1, s.buffer);
> + maps = maps->next;
> + }
> + trace_seq_destroy(&s);
> +}
> +
> static void
> add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance)
> {
> @@ -3287,6 +3412,10 @@ static void record_data(struct common_record_context *ctx)
> if (!no_top_instance() && !top_instance.msg_handle)
> print_stat(&top_instance);
>
> + for_all_instances(instance) {
> + add_pid_mem_maps(handle, instance);
> + }
> +
> tracecmd_append_cpu_data(handle, local_cpu_count, temp_files);
>
> for (i = 0; i < max_cpu_count; i++)
> @@ -4397,6 +4526,7 @@ void update_first_instance(struct buffer_instance *instance, int topt)
> }
>
> enum {
> + OPT_mmap = 244,
> OPT_quiet = 245,
> OPT_debug = 246,
> OPT_no_filter = 247,
> @@ -4627,6 +4757,7 @@ static void parse_record_options(int argc,
> {"debug", no_argument, NULL, OPT_debug},
> {"quiet", no_argument, NULL, OPT_quiet},
> {"help", no_argument, NULL, '?'},
> + {"mmap", no_argument, NULL, OPT_mmap},
> {"module", required_argument, NULL, OPT_module},
> {NULL, 0, NULL, 0}
> };
> @@ -4858,6 +4989,9 @@ static void parse_record_options(int argc,
> case 'i':
> ignore_event_not_found = 1;
> break;
> + case OPT_mmap:
> + get_mmap = 1;
> + break;
> case OPT_date:
> ctx->date = 1;
> if (ctx->data_flags & DATA_FL_OFFSET)
> @@ -4924,6 +5058,9 @@ static void parse_record_options(int argc,
> add_func(&ctx->instance->filter_funcs,
> ctx->instance->filter_mod, "*");
>
> + if (filter_task && get_mmap)
> + do_ptrace = 1;
> +
> if (do_ptrace && !filter_task && (filter_pid < 0))
> die(" -c can only be used with -F (or -P with event-fork support)");
> if (ctx->do_child && !filter_task &&! filter_pid)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] trace-cmd: Save the tracee memory map into the trace.dat file.
2019-06-18 14:32 [PATCH v2] trace-cmd: Save the tracee memory map into the trace.dat file tz.stoyanov
2019-06-18 21:29 ` Steven Rostedt
@ 2019-06-19 6:28 ` Yordan Karadzhov (VMware)
1 sibling, 0 replies; 3+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-06-19 6:28 UTC (permalink / raw)
To: tz.stoyanov, rostedt; +Cc: linux-trace-devel
On 18.06.19 г. 17:32 ч., tz.stoyanov@gmail.com wrote:
> From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com>
>
> [
> v2 changes:
> - Replaced usage of tracecmd_add_option_v() with tracecmd_add_option() API.
> - Added checks to prevent buffer overflow when sscanf (... "%s", buf) is used.
> - Return error in case memory allocation fails.
> - Return error if option string is not in the expected format.
> - Sort memory maps and use binary search to find matching library in the map.
> ]
>
> A new trace-cmd record option is added: "--mmap". When it is set with
> combination of -F or -P options, the memory map of the traced applications
> is stored in the trace.dat file. A new API tracecmd_search_tracee_mmap()
> can be used to look up into stored memory maps. The map is retrieved from
> /proc/<pid>/maps file.
>
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
> include/trace-cmd/trace-cmd.h | 4 +
> lib/trace-cmd/trace-input.c | 140 ++++++++++++++++++++++++++++++++-
> tracecmd/include/trace-local.h | 16 ++++
> tracecmd/trace-record.c | 137 ++++++++++++++++++++++++++++++++
> 4 files changed, 294 insertions(+), 3 deletions(-)
>
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 6f62ab9..210be03 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -82,6 +82,7 @@ enum {
> TRACECMD_OPTION_OFFSET,
> TRACECMD_OPTION_CPUCOUNT,
> TRACECMD_OPTION_VERSION,
> + TRACECMD_OPTION_PIDMMAPS,
> };
>
> enum {
> @@ -208,6 +209,9 @@ unsigned long long tracecmd_page_ts(struct tracecmd_input *handle,
> unsigned int tracecmd_record_ts_delta(struct tracecmd_input *handle,
> struct tep_record *record);
>
> +char *tracecmd_get_tracee_lib(struct tracecmd_input *handle,
> + int pid, unsigned long addr);
> +
> #ifndef SWIG
> /* hack for function graph work around */
> extern __thread struct tracecmd_input *tracecmd_curr_thread_handle;
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 61566ba..e2a06a8 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -101,6 +101,7 @@ struct tracecmd_input {
> struct tracecmd_ftrace finfo;
>
> struct hook_list *hooks;
> + struct pid_mem_maps *pid_mmaps;
> /* file information */
> size_t header_files_start;
> size_t ftrace_files_start;
> @@ -2134,6 +2135,138 @@ void tracecmd_set_ts2secs(struct tracecmd_input *handle,
> handle->use_trace_clock = false;
> }
>
> +static int trace_pid_mmap_cmp(const void *a, const void *b)
> +{
> + struct lib_mem_map *map_a = (struct lib_mem_map *)a;
> + struct lib_mem_map *map_b = (struct lib_mem_map *)b;
> +
> + if (map_a->start > map_b->start)
> + return 1;
> + if (map_a->start < map_b->start)
> + return -1;
> + return 0;
> +}
> +
> +#define STR_MMAP_LINE_MAX (PATH_MAX+34)
> +static int trace_pid_mmap_load(struct tracecmd_input *handle, char *buf)
> +{
> + struct pid_mem_maps *maps = NULL;
> + char mapname[STR_MMAP_LINE_MAX];
> + char *line;
> + int ret;
> + int i;
> +
> + maps = calloc(1, sizeof(*maps));
> + if (!maps)
> + return -ENOMEM;
> +
> + line = strchr(buf, '\n');
> + if (!line) {
> + free(maps);
> + return -EINVAL;
> + }
> + *line = '\0';
> + if (strlen(buf) > STR_MMAP_LINE_MAX) {
> + free(maps);
> + return -EINVAL;
> + }
> + ret = sscanf(buf, "%x %x %s", &maps->pid, &maps->nr_lib_maps, mapname);
> + if (ret != 3) {
> + free(maps);
> + return -EINVAL;
> + }
> + maps->proc_name = strdup(mapname);
> + if (!maps->proc_name) {
> + free(maps);
> + return -ENOMEM;
> + }
> + maps->lib_maps = calloc(maps->nr_lib_maps, sizeof(struct lib_mem_map));
> + if (!maps->lib_maps) {
> + free(maps->proc_name);
> + free(maps);
> + return -ENOMEM;
> + }
> + maps->next = handle->pid_mmaps;
> + handle->pid_mmaps = maps;
> + buf = line+1;
> + line = strchr(buf, '\n');
> + for (i = 0; i < maps->nr_lib_maps; i++) {
> + if (!line)
> + break;
> + *line = '\0';
> + if (strlen(buf) > STR_MMAP_LINE_MAX)
> + break;
> + ret = sscanf(buf, "%llx %llx %s", &maps->lib_maps[i].start,
> + &maps->lib_maps[i].end, mapname);
> + if (ret == 3) {
> + maps->lib_maps[i].lib_name = strdup(mapname);
> + if (!maps->lib_maps[i].lib_name)
> + return -ENOMEM;
> + }
> + buf = line+1;
> + line = strchr(buf, '\n');
> + }
> +
> + if (i != maps->nr_lib_maps)
> + return -EINVAL;
> +
> + qsort(maps->lib_maps, maps->nr_lib_maps,
> + sizeof(*maps->lib_maps), trace_pid_mmap_cmp);
> +
> + return 0;
> +}
> +
> +static int trace_pid_mmap_search(const void *a, const void *b)
> +{
> + struct lib_mem_map *key = (struct lib_mem_map *)a;
> + struct lib_mem_map *map = (struct lib_mem_map *)b;
> +
> + if (key->start > map->end)
> + return 1;
> + if (key->start < map->start)
> + return -1;
> + return 0;
> +}
> +
> +/**
> + * tracecmd_search_tracee_mmap - Search tracee memory address map
> + * @handle: input handle to the trace.dat file
> + * @pid: pid of the tracee
> + * @addr: address from the tracee memory space.
> + *
> + * Map of the tracee memory can be saved in the trace.dat file, using the option
> + * "--mmap". If there is such information, this API can be used to look up into
> + * this memory map to find library is loaded at the given @addr.
> + *
> + * The name of the library at given tracee @addr is returned.
> + */
> +char *tracecmd_search_tracee_mmap(struct tracecmd_input *handle,
> + int pid, unsigned long long addr)
> +{
> + struct pid_mem_maps *maps;
> + struct lib_mem_map *lib;
> + struct lib_mem_map key;
> +
> + if (!handle || !handle->pid_mmaps)
> + return NULL;
> +
> + maps = handle->pid_mmaps;
> + while (maps) {
> + if (maps->pid == pid)
> + break;
> + maps = maps->next;
> + }
> + if (!maps || !maps->nr_lib_maps || !maps->lib_maps)
> + return NULL;
> + key.start = addr;
> + lib = bsearch(&key, maps->lib_maps, maps->nr_lib_maps,
> + sizeof(*maps->lib_maps), trace_pid_mmap_search);
> + if (lib)
> + return lib->lib_name;
> +
> + return NULL;
> +}
> +
Having only the name of the library doesn't provide enough information.
We also need the range in the address space of the process occupied by
this library. Without knowing this, there is no way to map one
instruction pointer in the address space of the process to the its
address in the library.
Maybe you can change the interface function to return constant pointer
to the lib_mem_map object instead of just the string containing the name.
Thanks!
Yordan
> static int handle_options(struct tracecmd_input *handle)
> {
> unsigned long long offset;
> @@ -2221,9 +2354,6 @@ static int handle_options(struct tracecmd_input *handle)
> case TRACECMD_OPTION_UNAME:
> handle->uname = strdup(buf);
> break;
> - case TRACECMD_OPTION_VERSION:
> - handle->version = strdup(buf);
> - break;
> case TRACECMD_OPTION_HOOK:
> hook = tracecmd_create_event_hook(buf);
> hook->next = handle->hooks;
> @@ -2233,6 +2363,10 @@ static int handle_options(struct tracecmd_input *handle)
> cpus = *(int *)buf;
> handle->cpus = tep_read_number(handle->pevent, &cpus, 4);
> break;
> + case TRACECMD_OPTION_PIDMMAPS:
> + if (buf[size] == '\0')
> + trace_pid_mmap_load(handle, buf);
> + break;
> default:
> warning("unknown option %d", option);
> break;
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
> index 1cad3cc..dc4dc03 100644
> --- a/tracecmd/include/trace-local.h
> +++ b/tracecmd/include/trace-local.h
> @@ -157,6 +157,20 @@ struct func_list {
> const char *mod;
> };
>
> +struct lib_mem_map {
> + unsigned long long start;
> + unsigned long long end;
> + char *lib_name;
> +};
> +
> +struct pid_mem_maps {
> + struct pid_mem_maps *next;
> + struct lib_mem_map *lib_maps;
> + unsigned int nr_lib_maps;
> + char *proc_name;
> + int pid;
> +};
> +
> struct buffer_instance {
> struct buffer_instance *next;
> const char *name;
> @@ -183,6 +197,8 @@ struct buffer_instance {
> struct tracecmd_msg_handle *msg_handle;
> struct tracecmd_output *network_handle;
>
> + struct pid_mem_maps *mem_maps;
> +
> char *max_graph_depth;
>
> int flags;
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 5dc6f17..a938ce3 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -84,6 +84,7 @@ static int max_kb;
> static bool use_tcp;
>
> static int do_ptrace;
> +static int get_mmap;
>
> static int filter_task;
> static int filter_pid = -1;
> @@ -1062,6 +1063,98 @@ static char *make_pid_filter(char *curr_filter, const char *field)
> return filter;
> }
>
> +static int get_pid_mmaps(int pid)
> +{
> + struct buffer_instance *instance = &top_instance;
> + struct pid_mem_maps *maps = instance->mem_maps;
> + unsigned long long begin, end, inode, tmp;
> + struct lib_mem_map *map;
> + char mapname[PATH_MAX+1];
> + char fname[PATH_MAX+1];
> + char buf[PATH_MAX+100];
> + char perm[5];
> + char dev[6];
> + FILE *f;
> + int ret;
> + int res;
> + int i;
> +
> + while (maps) {
> + if (pid == maps->pid)
> + break;
> + maps = maps->next;
> + }
> +
> + sprintf(fname, "/proc/%d/exe", pid);
> + ret = readlink(fname, mapname, PATH_MAX);
> + if (ret >= PATH_MAX || ret < 0)
> + return -ENOENT;
> +
> + sprintf(fname, "/proc/%d/maps", pid);
> + f = fopen(fname, "r");
> + if (!f)
> + return -ENOENT;
> +
> + if (!maps) {
> + maps = calloc(1, sizeof(*maps));
> + if (!maps)
> + return -ENOMEM;
> + maps->pid = pid;
> + maps->next = instance->mem_maps;
> + instance->mem_maps = maps;
> + } else {
> + for (i = 0; i < maps->nr_lib_maps; i++)
> + free(maps->lib_maps[i].lib_name);
> + free(maps->lib_maps);
> + maps->lib_maps = NULL;
> + maps->nr_lib_maps = 0;
> + free(maps->proc_name);
> + }
> +
> + maps->proc_name = strdup(mapname);
> + if (!maps->proc_name)
> + return -ENOMEM;
> +
> + ret = 0;
> + while (fgets(buf, sizeof(buf), f)) {
> + mapname[0] = '\0';
> + res = sscanf(buf, "%llx-%llx %4s %llx %5s %lld %s",
> + &begin, &end, perm, &tmp, dev, &inode, mapname);
> + if (res == 7 && mapname[0] != '\0') {
> + map = realloc(maps->lib_maps,
> + (maps->nr_lib_maps+1)*sizeof(*map));
> + if (!map) {
> + ret = -ENOMEM;
> + break;
> + }
> + map[maps->nr_lib_maps].end = end;
> + map[maps->nr_lib_maps].start = begin;
> + map[maps->nr_lib_maps].lib_name = strdup(mapname);
> + if (!map[maps->nr_lib_maps].lib_name) {
> + ret = -ENOMEM;
> + free(map);
> + break;
> + }
> + maps->lib_maps = map;
> + maps->nr_lib_maps++;
> + }
> + }
> + fclose(f);
> +
> + return ret;
> +}
> +
> +static void get_filter_pid_mmaps(void)
> +{
> + struct filter_pids *p;
> +
> + for (p = filter_pids; p; p = p->next) {
> + if (p->exclude)
> + continue;
> + get_pid_mmaps(p->pid);
> + }
> +}
> +
> static void update_task_filter(void)
> {
> struct buffer_instance *instance;
> @@ -1070,6 +1163,9 @@ static void update_task_filter(void)
> if (no_filter)
> return;
>
> + if (get_mmap && filter_pids)
> + get_filter_pid_mmaps();
> +
> if (filter_task)
> add_filter_pid(pid, 0);
>
> @@ -1264,6 +1360,8 @@ static void ptrace_wait(enum trace_type type, int main_pid)
> break;
>
> case PTRACE_EVENT_EXIT:
> + if (get_mmap)
> + get_pid_mmaps(main_pid);
> ptrace(PTRACE_GETEVENTMSG, pid, NULL, &cstatus);
> ptrace(PTRACE_DETACH, pid, NULL, NULL);
> break;
> @@ -3094,6 +3192,33 @@ static void append_buffer(struct tracecmd_output *handle,
> }
> }
>
> +
> +static void
> +add_pid_mem_maps(struct tracecmd_output *handle, struct buffer_instance *instance)
> +{
> + struct pid_mem_maps *maps = instance->mem_maps;
> + struct trace_seq s;
> + int i;
> +
> + trace_seq_init(&s);
> + while (maps) {
> + if (!maps->nr_lib_maps)
> + continue;
> + trace_seq_reset(&s);
> + trace_seq_printf(&s, "%x %x %s\n",
> + maps->pid, maps->nr_lib_maps, maps->proc_name);
> + for (i = 0; i < maps->nr_lib_maps; i++)
> + trace_seq_printf(&s, "%llx %llx %s\n",
> + maps->lib_maps[i].start,
> + maps->lib_maps[i].end,
> + maps->lib_maps[i].lib_name);
> + tracecmd_add_option(handle, TRACECMD_OPTION_PIDMMAPS,
> + s.len+1, s.buffer);
> + maps = maps->next;
> + }
> + trace_seq_destroy(&s);
> +}
> +
> static void
> add_buffer_stat(struct tracecmd_output *handle, struct buffer_instance *instance)
> {
> @@ -3287,6 +3412,10 @@ static void record_data(struct common_record_context *ctx)
> if (!no_top_instance() && !top_instance.msg_handle)
> print_stat(&top_instance);
>
> + for_all_instances(instance) {
> + add_pid_mem_maps(handle, instance);
> + }
> +
> tracecmd_append_cpu_data(handle, local_cpu_count, temp_files);
>
> for (i = 0; i < max_cpu_count; i++)
> @@ -4397,6 +4526,7 @@ void update_first_instance(struct buffer_instance *instance, int topt)
> }
>
> enum {
> + OPT_mmap = 244,
> OPT_quiet = 245,
> OPT_debug = 246,
> OPT_no_filter = 247,
> @@ -4627,6 +4757,7 @@ static void parse_record_options(int argc,
> {"debug", no_argument, NULL, OPT_debug},
> {"quiet", no_argument, NULL, OPT_quiet},
> {"help", no_argument, NULL, '?'},
> + {"mmap", no_argument, NULL, OPT_mmap},
> {"module", required_argument, NULL, OPT_module},
> {NULL, 0, NULL, 0}
> };
> @@ -4858,6 +4989,9 @@ static void parse_record_options(int argc,
> case 'i':
> ignore_event_not_found = 1;
> break;
> + case OPT_mmap:
> + get_mmap = 1;
> + break;
> case OPT_date:
> ctx->date = 1;
> if (ctx->data_flags & DATA_FL_OFFSET)
> @@ -4924,6 +5058,9 @@ static void parse_record_options(int argc,
> add_func(&ctx->instance->filter_funcs,
> ctx->instance->filter_mod, "*");
>
> + if (filter_task && get_mmap)
> + do_ptrace = 1;
> +
> if (do_ptrace && !filter_task && (filter_pid < 0))
> die(" -c can only be used with -F (or -P with event-fork support)");
> if (ctx->do_child && !filter_task &&! filter_pid)
>
^ permalink raw reply [flat|nested] 3+ messages in thread