Skip to content

Commit

Permalink
Improve main executable detection (#44)
Browse files Browse the repository at this point in the history
The logic here is a bit flimsy and should be reworked, but let's at
least mark first executables as the "main" ones.

More context in the comment but overall this is not always correct, in
particular Rust + static musl is broken due how the loader operates for
that case.

We should understand the kernel and loader's logic in detail and fully
replicate it here.

Test Plan
=========

Ran locally for several minutes without issues. The profilers looked
good (see PR) and 0 unexpected errors were emitted.
  • Loading branch information
javierhonduco authored May 20, 2024
1 parent a1ad903 commit 4eedba4
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl Collector {
load_offset: v.load_offset,
load_vaddr: v.load_vaddr,
is_dyn: v.is_dyn,
main_bin: v.main_bin,
main_exec: v.main_exec,
},
);
}
Expand Down
28 changes: 15 additions & 13 deletions src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub struct ObjectFileInfo {
pub load_offset: u64,
pub load_vaddr: u64,
pub is_dyn: bool,
pub main_bin: bool,
pub main_exec: bool,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -735,30 +735,31 @@ impl Profiler<'_> {
panic!("build id should not be none for file backed mappings");
}

let my_lock = self.object_files.lock().unwrap();
let object_files = self.object_files.lock().unwrap();

// We might know about a mapping that failed to open for some reason.
let object_file_info = my_lock.get(&mapping.build_id.clone().unwrap());
let object_file_info = object_files.get(&mapping.build_id.clone().unwrap());
if object_file_info.is_none() {
warn!("mapping not found");
continue;
}
let object_file_info = object_file_info.unwrap();
let obj_path = object_file_info.path.clone();

// TODO: rework this logic as it's quite kludgy at the moment and this is broken with
// some loaders. Particularly, Rust statically linked with musl does not work. We must
// ensure everything works with ASLR enabled loading as well.
let mut load_address = 0;
// Hopefully this is the main object
// TODO: make this work with ASLR + static
if object_file_info.main_bin {
if object_file_info.main_exec {
if object_file_info.is_dyn {
load_address = mapping.load_address; // mapping.start_addr - mapping.offset - object_file_info.elf_load.0;
load_address = mapping.load_address;
}
} else {
load_address = mapping.load_address; //mapping.start_addr - object_file_info.elf_load.0;
load_address = mapping.load_address;
}

// Avoid deadlock
std::mem::drop(my_lock);
std::mem::drop(object_files);

let build_id = mapping.build_id.clone().unwrap();
match self
Expand Down Expand Up @@ -1042,7 +1043,7 @@ impl Profiler<'_> {
let mut mappings = vec![];
let object_files_clone = self.object_files.clone();

for (i, map) in maps.iter().enumerate() {
for map in maps.iter() {
if !map.perms.contains(procfs::process::MMPermissions::EXECUTE) {
continue;
}
Expand Down Expand Up @@ -1091,19 +1092,20 @@ impl Profiler<'_> {
unwinder,
});

let mut my_lock = object_files_clone.lock().expect("lock");
let mut object_files = object_files_clone.lock().expect("lock object_files");
let main_exec = object_files.is_empty();

match object_file.elf_load() {
Ok(elf_load) => {
my_lock.insert(
object_files.insert(
build_id,
ObjectFileInfo {
path: abs_path,
file,
load_offset: elf_load.offset,
load_vaddr: elf_load.vaddr,
is_dyn: object_file.is_dynamic(),
main_bin: i < 3,
main_exec,
},
);
}
Expand Down

0 comments on commit 4eedba4

Please sign in to comment.