Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for reading symbols from /tmp/perf-pid.map #573

Merged
merged 1 commit into from
Jun 20, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add support for reading symbols from /tmp/perf-pid.map
This adds basic support for /tmp/perf-pid.map. To cope with processes in
containers, it supports:

* mapping from BCC's PID namespace to the target process's PID namespace
  using /proc/pid/status
* resolving a target process's root filesystem using /proc/pid/root
  • Loading branch information
Mark Drayton committed Jun 15, 2016
commit 769edf959f8dc5cb67a08246d461885eb92ed687
4 changes: 2 additions & 2 deletions src/cc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ if (CMAKE_COMPILER_IS_GNUCC)
endif()
endif()

add_library(bcc-shared SHARED bpf_common.cc bpf_module.cc libbpf.c perf_reader.c shared_table.cc exported_files.cc bcc_elf.c bcc_proc.c bcc_syms.cc usdt_args.cc usdt.cc)
add_library(bcc-shared SHARED bpf_common.cc bpf_module.cc libbpf.c perf_reader.c shared_table.cc exported_files.cc bcc_elf.c bcc_perf_map.c bcc_proc.c bcc_syms.cc usdt_args.cc usdt.cc)
set_target_properties(bcc-shared PROPERTIES VERSION ${REVISION_LAST} SOVERSION 0)
set_target_properties(bcc-shared PROPERTIES OUTPUT_NAME bcc)

add_library(bcc-loader-static libbpf.c perf_reader.c bcc_elf.c bcc_proc.c)
add_library(bcc-loader-static libbpf.c perf_reader.c bcc_elf.c bcc_perf_map.c bcc_proc.c)
add_library(bcc-static STATIC bpf_common.cc bpf_module.cc shared_table.cc exported_files.cc bcc_syms.cc usdt_args.cc usdt.cc)
set_target_properties(bcc-static PROPERTIES OUTPUT_NAME bcc)

Expand Down
102 changes: 102 additions & 0 deletions src/cc/bcc_perf_map.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright (c) 2016 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "bcc_perf_map.h"

int bcc_perf_map_nspid(int pid) {
char status_path[64];
FILE *status;

snprintf(status_path, sizeof(status_path), "/proc/%d/status", pid);
status = fopen(status_path, "r");

if (!status)
return -1;

// return the original PID if the NSpid line is missing
int nspid = pid;

size_t size;
char *line = NULL;
while (getline(&line, &size, status) != -1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite right here. According to POSIX.1-200, getline will only allocate a new buffer to store the line when line == NULL and size == 0. Since size is uninitialized here, this can break in really unexpected ways.

On top of that, getline allocates a buffer for each call (even in failure cases), and you're calling free only once outside the loop, hence leaking significant amounts of memory.

Please fix the initialization and deallocation, and add some curly braces to prevent further bugs like this in the future. Thanks! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback!

Fixing size is fine. My getline man page actually says it's ignored if *lineptr == NULL, but http://man7.org/linux/man-pages/man3/getline.3.html says it must be zero. Regardless, there's clearly there's no downside to intializing to zero so I'll do that.

Re: getline allocating on every call: are you sure about that? http://man7.org/linux/man-pages/man3/getline.3.html says getline will use the buffer passed and realloc it if necessary. It doesn't say it'll allocate every time around and the example code in the man page just has a single free outside the loop. Additionally, my own simple test with valgrind appears to confirm this is safe:

https://gist.github.com/markdrayton/1ef00109ff2092898ab3b908d0598c41

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're totally right, I misread the manpage. It will indeed realloc if the original pointer is not NULL... That is a much more friendly API to use! So yes, the only frisky thing is initializing the value of size. Sorry for wasting your time!

if (strstr(line, "NSpid:") != NULL)
// PID namespaces can be nested -- last number is innermost PID
nspid = (int)strtol(strrchr(line, '\t'), NULL, 10);
free(line);

return nspid;
}

bool bcc_perf_map_path(char *map_path, size_t map_len, int pid) {
char source[64];
snprintf(source, sizeof(source), "/proc/%d/root", pid);

char target[4096];
ssize_t target_len = readlink(source, target, sizeof(target) - 1);
if (target_len == -1)
return false;

target[target_len] = '\0';
if (strcmp(target, "/") == 0)
target[0] = '\0';

int nspid = bcc_perf_map_nspid(pid);

snprintf(map_path, map_len, "%s/tmp/perf-%d.map", target, nspid);
return true;
}

int bcc_perf_map_foreach_sym(const char *path, bcc_perf_map_symcb callback,
void* payload) {
FILE* file = fopen(path, "r");
if (!file)
return -1;

char *line = NULL;
size_t size;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. This needs zero initialization.

long long begin, len;
while (getline(&line, &size, file) != -1) {
char *cursor = line;
char *newline, *sep;

begin = strtoull(cursor, &sep, 16);
if (*sep != ' ' || (sep == cursor && begin == 0))
continue;
cursor = sep;
while (*cursor && isspace(*cursor)) cursor++;

len = strtoull(cursor, &sep, 16);
if (*sep != ' ' || (sep == cursor && begin == 0))
continue;
cursor = sep;
while (*cursor && isspace(*cursor)) cursor++;

newline = strchr(cursor, '\n');
if (newline)
newline[0] = '\0';

callback(cursor, begin, len - 1, 0, payload);
}

free(line);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, leaking memory here. getline always allocates a buffer.

fclose(file);

return 0;
}
38 changes: 38 additions & 0 deletions src/cc/bcc_perf_map.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2016 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#ifndef LIBBCC_PERF_MAP_H
#define LIBBCC_PERF_MAP_H

#ifdef __cplusplus
extern "C" {
#endif

#include <stdint.h>
#include <stdbool.h>
#include <unistd.h>

typedef int (*bcc_perf_map_symcb)(const char *, uint64_t, uint64_t, int,
void *);

int bcc_perf_map_nspid(int pid);
bool bcc_perf_map_path(char *map_path, size_t map_len, int pid);
int bcc_perf_map_foreach_sym(const char *path, bcc_perf_map_symcb callback,
void* payload);

#ifdef __cplusplus
}
#endif
#endif
10 changes: 9 additions & 1 deletion src/cc/bcc_proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <ctype.h>
#include <stdio.h>

#include "bcc_perf_map.h"
#include "bcc_proc.h"
#include "bcc_elf.h"

Expand Down Expand Up @@ -85,7 +86,7 @@ int bcc_procutils_each_module(int pid, bcc_procutils_modulecb callback,
char perm[8], dev[8];
long long begin, end, size, inode;

ret = fscanf(procmap, "%llx-%llx %s %llx %s %llx", &begin, &end, perm,
ret = fscanf(procmap, "%llx-%llx %s %llx %s %lld", &begin, &end, perm,
&size, dev, &inode);

if (!fgets(endline, sizeof(endline), procmap))
Expand All @@ -108,6 +109,13 @@ int bcc_procutils_each_module(int pid, bcc_procutils_modulecb callback,
} while (ret && ret != EOF);

fclose(procmap);

// Add a mapping to /tmp/perf-pid.map for the entire address space. This will
// be used if symbols aren't resolved in an earlier mapping.
char map_path[4096];
if (bcc_perf_map_path(map_path, sizeof(map_path), pid))
callback(map_path, 0, -1, payload);

return 0;
}

Expand Down
10 changes: 9 additions & 1 deletion src/cc/bcc_syms.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <unistd.h>

#include "bcc_elf.h"
#include "bcc_perf_map.h"
#include "bcc_proc.h"
#include "bcc_syms.h"

Expand Down Expand Up @@ -139,11 +140,18 @@ bool ProcSyms::Module::is_so() const {
return strstr(name_.c_str(), ".so") != nullptr;
}

bool ProcSyms::Module::is_perf_map() const {
return strstr(name_.c_str(), ".map") != nullptr;
}

void ProcSyms::Module::load_sym_table() {
if (syms_.size())
return;

bcc_elf_foreach_sym(name_.c_str(), _add_symbol, this);
if (is_perf_map())
bcc_perf_map_foreach_sym(name_.c_str(), _add_symbol, this);
else
bcc_elf_foreach_sym(name_.c_str(), _add_symbol, this);
}

bool ProcSyms::Module::find_name(const char *symname, uint64_t *addr) {
Expand Down
1 change: 1 addition & 0 deletions src/cc/syms.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class ProcSyms : SymbolCache {
bool find_addr(uint64_t addr, struct bcc_symbol *sym);
bool find_name(const char *symname, uint64_t *addr);
bool is_so() const;
bool is_perf_map() const;

static int _add_symbol(const char *symname, uint64_t start, uint64_t end,
int flags, void *p);
Expand Down
82 changes: 82 additions & 0 deletions tests/cc/test_c_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,14 @@
#include <dlfcn.h>
#include <stdint.h>
#include <string.h>
#include <sys/mman.h>
#include <unistd.h>

#include "bcc_elf.h"
#include "bcc_perf_map.h"
#include "bcc_proc.h"
#include "bcc_syms.h"
#include "vendor/tinyformat.hpp"

#include "catch.hpp"

Expand Down Expand Up @@ -109,3 +112,82 @@ TEST_CASE("resolve symbol addresses for a given PID", "[c_api]") {
REQUIRE(string("strtok") == sym.name);
}
}

#define STACK_SIZE (1024 * 1024)
static char child_stack[STACK_SIZE];

static string perf_map_path(pid_t pid) {
return tfm::format("/tmp/perf-%d.map", pid);
}

static int child_func(void *arg) {
unsigned long long map_addr = (unsigned long long)arg;

const char *path = perf_map_path(getpid()).c_str();
FILE *file = fopen(path, "w");
if (file == NULL) {
return -1;
}
fprintf(file, "%llx 10 dummy_fn\n", map_addr);
fclose(file);

sleep(5);

unlink(path);
return 0;
}

static pid_t spawn_child(void *map_addr, bool own_pidns) {
int flags = 0;
if (own_pidns)
flags |= CLONE_NEWPID;

pid_t child = clone(child_func, /* stack grows down */ child_stack + STACK_SIZE,
flags, (void*)map_addr);
if (child < 0)
return -1;

sleep(1); // let the child get set up
return child;
}

TEST_CASE("resolve symbols using /tmp/perf-pid.map", "[c_api]") {
const int map_sz = 4096;
void *map_addr = mmap(NULL, map_sz, PROT_READ | PROT_EXEC,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
REQUIRE(map_addr != MAP_FAILED);

struct bcc_symbol sym;
pid_t child = -1;

SECTION("same namespace") {
child = spawn_child(map_addr, /* own_pidns */ false);
REQUIRE(child > 0);

void *resolver = bcc_symcache_new(child);
REQUIRE(resolver);

REQUIRE(bcc_symcache_resolve(resolver, (unsigned long long)map_addr,
&sym) == 0);
REQUIRE(sym.module);
REQUIRE(string(sym.module) == perf_map_path(child));
REQUIRE(string("dummy_fn") == sym.name);
}

SECTION("separate namespace") {
child = spawn_child(map_addr, /* own_pidns */ true);
REQUIRE(child > 0);

void *resolver = bcc_symcache_new(child);
REQUIRE(resolver);

REQUIRE(bcc_symcache_resolve(resolver, (unsigned long long)map_addr,
&sym) == 0);
REQUIRE(sym.module);
// child is PID 1 in its namespace
REQUIRE(string(sym.module) == perf_map_path(1));
REQUIRE(string("dummy_fn") == sym.name);
}

munmap(map_addr, map_sz);
}