Skip to content

Commit

Permalink
Reland "Compute base addresses from program headers while reading /pr…
Browse files Browse the repository at this point in the history 
…oc/self/maps."

This is a reland of 6f903f3a228bc8e10cd7bda76342954feb3000c5

Fixed Android build issue.

Original change's description:
> Compute base addresses from program headers while reading /proc/self/maps.
>
> This cherry picks this glog change:
> google/glog#261
> with a reimplementation of the program header reading logic for the
> sandboxed symbolizer.
>
> This should cause unsymbolized stack traces to contain the correct
> addresses for binaries linked with lld.
>
> Bug: 772559
> Change-Id: Ief9cbb463b3b4c32149da893c89c2eefd76b05d4
> Reviewed-on: https://chromium-review.googlesource.com/752753
> Commit-Queue: Peter Collingbourne <[email protected]>
> Reviewed-by: Mark Mentovai <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#513956}

Bug: 772559
Change-Id: Id6a8c62c0b05a7d12f817e389c7b96223fec8073
Reviewed-on: https://chromium-review.googlesource.com/754232
Reviewed-by: Mark Mentovai <[email protected]>
Commit-Queue: Peter Collingbourne <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#514018}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1a3ed318cbbe2b53d9d6410a376e75439b9e6007
  • Loading branch information
Peter Collingbourne authored and Commit Bot committed Nov 4, 2017
1 parent 06a9a3e commit debb89a
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 89 deletions.
39 changes: 20 additions & 19 deletions android/library_loader/library_prefetcher_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,48 +27,49 @@ const uint8_t kExecutePrivate = base::debug::MappedMemoryRegion::EXECUTE |

TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchNoRange) {
const base::debug::MappedMemoryRegion regions[4] = {
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, kReadPrivate, ""},
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, kReadPrivate, "foo"},
base::debug::MappedMemoryRegion{
0x4000, 0x5000, 10, kReadPrivate, "foobar.apk"},
base::debug::MappedMemoryRegion{
0x4000, 0x5000, 10, kReadPrivate, "libchromium.so"}};
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate, ""},
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
"foo"},
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
"foobar.apk"},
base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
"libchromium.so"}};
for (int i = 0; i < 4; ++i) {
ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(regions[i]));
}
}

TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchUnreadableRange) {
const base::debug::MappedMemoryRegion region = {
0x4000, 0x5000, 10, kExecutePrivate, "base.apk"};
0x4000, 0x5000, 10, 0, kExecutePrivate, "base.apk"};
ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
}

TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchSkipSharedRange) {
const base::debug::MappedMemoryRegion region = {
0x4000, 0x5000, 10, kRead, "base.apk"};
0x4000, 0x5000, 10, 0, kRead, "base.apk"};
ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
}

TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchLibchromeRange) {
const base::debug::MappedMemoryRegion region = {
0x4000, 0x5000, 10, kReadPrivate, "libchrome.so"};
0x4000, 0x5000, 10, 0, kReadPrivate, "libchrome.so"};
ASSERT_TRUE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
}

TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchBaseApkRange) {
const base::debug::MappedMemoryRegion region = {
0x4000, 0x5000, 10, kReadPrivate, "base.apk"};
0x4000, 0x5000, 10, 0, kReadPrivate, "base.apk"};
ASSERT_TRUE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
}

TEST(NativeLibraryPrefetcherTest,
TestFilterLibchromeRangesOnlyIfPossibleNoLibchrome) {
std::vector<base::debug::MappedMemoryRegion> regions;
regions.push_back(
base::debug::MappedMemoryRegion{0x1, 0x2, 0, kReadPrivate, "base.apk"});
regions.push_back(
base::debug::MappedMemoryRegion{0x3, 0x4, 0, kReadPrivate, "base.apk"});
regions.push_back(base::debug::MappedMemoryRegion{0x1, 0x2, 0, 0,
kReadPrivate, "base.apk"});
regions.push_back(base::debug::MappedMemoryRegion{0x3, 0x4, 0, 0,
kReadPrivate, "base.apk"});
std::vector<NativeLibraryPrefetcher::AddressRange> ranges;
NativeLibraryPrefetcher::FilterLibchromeRangesOnlyIfPossible(regions,
&ranges);
Expand All @@ -82,12 +83,12 @@ TEST(NativeLibraryPrefetcherTest,
TEST(NativeLibraryPrefetcherTest,
TestFilterLibchromeRangesOnlyIfPossibleHasLibchrome) {
std::vector<base::debug::MappedMemoryRegion> regions;
regions.push_back(
base::debug::MappedMemoryRegion{0x1, 0x2, 0, kReadPrivate, "base.apk"});
regions.push_back(base::debug::MappedMemoryRegion{0x1, 0x2, 0, 0,
kReadPrivate, "base.apk"});
regions.push_back(base::debug::MappedMemoryRegion{
0x6, 0x7, 0, kReadPrivate, "libchrome.so"});
regions.push_back(
base::debug::MappedMemoryRegion{0x3, 0x4, 0, kReadPrivate, "base.apk"});
0x6, 0x7, 0, 0, kReadPrivate, "libchrome.so"});
regions.push_back(base::debug::MappedMemoryRegion{0x3, 0x4, 0, 0,
kReadPrivate, "base.apk"});
std::vector<NativeLibraryPrefetcher::AddressRange> ranges;
NativeLibraryPrefetcher::FilterLibchromeRangesOnlyIfPossible(regions,
&ranges);
Expand Down
3 changes: 3 additions & 0 deletions debug/proc_maps_linux.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ struct MappedMemoryRegion {
// Byte offset into |path| of the range mapped into memory.
unsigned long long offset;

// Image base, if this mapping corresponds to an ELF image.
uintptr_t base;

// Bitmask of read/write/execute/private/shared permissions.
uint8_t permissions;

Expand Down
76 changes: 60 additions & 16 deletions debug/stack_trace_posix.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/param.h>
#include <sys/stat.h>
#include <sys/types.h>
Expand Down Expand Up @@ -40,6 +41,7 @@

#include "base/cfi_flags.h"
#include "base/debug/debugger.h"
#include "base/files/scoped_file.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/memory/free_deleter.h"
Expand Down Expand Up @@ -565,24 +567,10 @@ class SandboxSymbolizeHelper {
// std::vector<MappedMemoryRegion> using a const_iterator does not allocate
// dynamic memory, hence it is async-signal-safe.
std::vector<MappedMemoryRegion>::const_iterator it;
bool is_first = true;
for (it = instance->regions_.begin(); it != instance->regions_.end();
++it, is_first = false) {
const MappedMemoryRegion& region = *it;
for (const MappedMemoryRegion& region : instance->regions_) {
if (region.start <= pc && pc < region.end) {
start_address = region.start;
// Don't subtract 'start_address' from the first entry:
// * If a binary is compiled w/o -pie, then the first entry in
// process maps is likely the binary itself (all dynamic libs
// are mapped higher in address space). For such a binary,
// instruction offset in binary coincides with the actual
// instruction address in virtual memory (as code section
// is mapped to a fixed memory range).
// * If a binary is compiled with -pie, all the modules are
// mapped high at address space (in particular, higher than
// shadow memory of the tool), so the module can't be the
// first entry.
base_address = (is_first ? 0U : start_address) - region.offset;
base_address = region.base;
if (file_path && file_path_size > 0) {
strncpy(file_path, region.path.c_str(), file_path_size);
// Ensure null termination.
Expand All @@ -594,6 +582,60 @@ class SandboxSymbolizeHelper {
return -1;
}

// Set the base address for each memory region by reading ELF headers in
// process memory.
void SetBaseAddressesForMemoryRegions() {
base::ScopedFD mem_fd(
HANDLE_EINTR(open("/proc/self/mem", O_RDONLY | O_CLOEXEC)));
if (!mem_fd.is_valid())
return;

auto safe_memcpy = [&mem_fd](void* dst, uintptr_t src, size_t size) {
return HANDLE_EINTR(pread(mem_fd.get(), dst, size, src)) == ssize_t(size);
};

uintptr_t cur_base = 0;
for (auto& r : regions_) {
ElfW(Ehdr) ehdr;
static_assert(SELFMAG <= sizeof(ElfW(Ehdr)), "SELFMAG too large");
if ((r.permissions & MappedMemoryRegion::READ) &&
safe_memcpy(&ehdr, r.start, sizeof(ElfW(Ehdr))) &&
memcmp(ehdr.e_ident, ELFMAG, SELFMAG) == 0) {
switch (ehdr.e_type) {
case ET_EXEC:
cur_base = 0;
break;
case ET_DYN:
// Find the segment containing file offset 0. This will correspond
// to the ELF header that we just read. Normally this will have
// virtual address 0, but this is not guaranteed. We must subtract
// the virtual address from the address where the ELF header was
// mapped to get the base address.
//
// If we fail to find a segment for file offset 0, use the address
// of the ELF header as the base address.
cur_base = r.start;
for (unsigned i = 0; i != ehdr.e_phnum; ++i) {
ElfW(Phdr) phdr;
if (safe_memcpy(&phdr, r.start + ehdr.e_phoff + i * sizeof(phdr),
sizeof(phdr)) &&
phdr.p_type == PT_LOAD && phdr.p_offset == 0) {
cur_base = r.start - phdr.p_vaddr;
break;
}
}
break;
default:
// ET_REL or ET_CORE. These aren't directly executable, so they
// don't affect the base address.
break;
}
}

r.base = cur_base;
}
}

// Parses /proc/self/maps in order to compile a list of all object file names
// for the modules that are loaded in the current process.
// Returns true on success.
Expand All @@ -611,6 +653,8 @@ class SandboxSymbolizeHelper {
return false;
}

SetBaseAddressesForMemoryRegions();

is_initialized_ = true;
return true;
}
Expand Down
6 changes: 4 additions & 2 deletions third_party/symbolize/README.chromium
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ https://github.com/google/glog/tree/a5ffa884137f7687d0393ccba22557d583654a25
- symbolize.cc
- symbolize.h

Cherry picked upstream change https://github.com/google/glog/pull/115 to
fix a symbolization issue when using lld.
Cherry picked upstream changes:
https://github.com/google/glog/pull/115
https://github.com/google/glog/pull/261
to fix symbolization issues when using lld.

The following files are minimal stubs created for use in Chromium:

Expand Down
110 changes: 58 additions & 52 deletions third_party/symbolize/symbolize.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@

#if defined(HAVE_SYMBOLIZE)

#include <string.h>

#include <algorithm>
#include <limits>

Expand Down Expand Up @@ -328,41 +330,17 @@ FindSymbol(uint64_t pc, const int fd, char *out, int out_size,
// both regular and dynamic symbol tables if necessary. On success,
// write the symbol name to "out" and return true. Otherwise, return
// false.
static bool GetSymbolFromObjectFile(const int fd, uint64_t pc,
char *out, int out_size,
uint64_t map_base_address) {
static bool GetSymbolFromObjectFile(const int fd,
uint64_t pc,
char* out,
int out_size,
uint64_t base_address) {
// Read the ELF header.
ElfW(Ehdr) elf_header;
if (!ReadFromOffsetExact(fd, &elf_header, sizeof(elf_header), 0)) {
return false;
}

uint64_t symbol_offset = 0;
if (elf_header.e_type == ET_DYN) { // DSO needs offset adjustment.
ElfW(Phdr) phdr;
// We need to find the PT_LOAD segment corresponding to the read-execute
// file mapping in order to correctly perform the offset adjustment.
for (unsigned i = 0; i != elf_header.e_phnum; ++i) {
if (!ReadFromOffsetExact(fd, &phdr, sizeof(phdr),
elf_header.e_phoff + i * sizeof(phdr)))
return false;
if (phdr.p_type == PT_LOAD &&
(phdr.p_flags & (PF_R | PF_X)) == (PF_R | PF_X)) {
// Find the mapped address corresponding to virtual address zero. We do
// this by first adding p_offset. This gives us the mapped address of
// the start of the segment, or in other words the mapped address
// corresponding to the virtual address of the segment. (Note that this
// is distinct from the start address, as p_offset is not guaranteed to
// be page aligned.) We then subtract p_vaddr, which takes us to virtual
// address zero.
symbol_offset = map_base_address + phdr.p_offset - phdr.p_vaddr;
break;
}
}
if (symbol_offset == 0)
return false;
}

ElfW(Shdr) symtab, strtab;

// Consult a regular symbol table first.
Expand All @@ -372,8 +350,7 @@ static bool GetSymbolFromObjectFile(const int fd, uint64_t pc,
symtab.sh_link * sizeof(symtab))) {
return false;
}
if (FindSymbol(pc, fd, out, out_size, symbol_offset,
&strtab, &symtab)) {
if (FindSymbol(pc, fd, out, out_size, base_address, &strtab, &symtab)) {
return true; // Found the symbol in a regular symbol table.
}
}
Expand All @@ -385,8 +362,7 @@ static bool GetSymbolFromObjectFile(const int fd, uint64_t pc,
symtab.sh_link * sizeof(symtab))) {
return false;
}
if (FindSymbol(pc, fd, out, out_size, symbol_offset,
&strtab, &symtab)) {
if (FindSymbol(pc, fd, out, out_size, base_address, &strtab, &symtab)) {
return true; // Found the symbol in a dynamic symbol table.
}
}
Expand Down Expand Up @@ -535,14 +511,20 @@ OpenObjectFileContainingPcAndGetStartAddress(uint64_t pc,
int out_file_name_size) {
int object_fd;

// Open /proc/self/maps.
int maps_fd;
NO_INTR(maps_fd = open("/proc/self/maps", O_RDONLY));
FileDescriptor wrapped_maps_fd(maps_fd);
if (wrapped_maps_fd.get() < 0) {
return -1;
}

int mem_fd;
NO_INTR(mem_fd = open("/proc/self/mem", O_RDONLY));
FileDescriptor wrapped_mem_fd(mem_fd);
if (wrapped_mem_fd.get() < 0) {
return -1;
}

// Iterate over maps and look for the map containing the pc. Then
// look into the symbol tables inside.
char buf[1024]; // Big enough for line of sane /proc/self/maps
Expand Down Expand Up @@ -578,11 +560,6 @@ OpenObjectFileContainingPcAndGetStartAddress(uint64_t pc,
}
++cursor; // Skip ' '.

// Check start and end addresses.
if (!(start_address <= pc && pc < end_address)) {
continue; // We skip this map. PC isn't in this map.
}

// Read flags. Skip flags until we encounter a space or eol.
const char * const flags_start = cursor;
while (cursor < eol && *cursor != ' ') {
Expand All @@ -593,6 +570,48 @@ OpenObjectFileContainingPcAndGetStartAddress(uint64_t pc,
return -1; // Malformed line.
}

// Determine the base address by reading ELF headers in process memory.
ElfW(Ehdr) ehdr;
if (flags_start[0] == 'r' &&
ReadFromOffsetExact(mem_fd, &ehdr, sizeof(ElfW(Ehdr)), start_address) &&
memcmp(ehdr.e_ident, ELFMAG, SELFMAG) == 0) {
switch (ehdr.e_type) {
case ET_EXEC:
base_address = 0;
break;
case ET_DYN:
// Find the segment containing file offset 0. This will correspond
// to the ELF header that we just read. Normally this will have
// virtual address 0, but this is not guaranteed. We must subtract
// the virtual address from the address where the ELF header was
// mapped to get the base address.
//
// If we fail to find a segment for file offset 0, use the address
// of the ELF header as the base address.
base_address = start_address;
for (unsigned i = 0; i != ehdr.e_phnum; ++i) {
ElfW(Phdr) phdr;
if (ReadFromOffsetExact(
mem_fd, &phdr, sizeof(phdr),
start_address + ehdr.e_phoff + i * sizeof(phdr)) &&
phdr.p_type == PT_LOAD && phdr.p_offset == 0) {
base_address = start_address - phdr.p_vaddr;
break;
}
}
break;
default:
// ET_REL or ET_CORE. These aren't directly executable, so they don't
// affect the base address.
break;
}
}

// Check start and end addresses.
if (!(start_address <= pc && pc < end_address)) {
continue; // We skip this map. PC isn't in this map.
}

// Check flags. We are only interested in "r-x" maps.
if (memcmp(flags_start, "r-x", 3) != 0) { // Not a "r-x" map.
continue; // We skip this map.
Expand All @@ -607,19 +626,6 @@ OpenObjectFileContainingPcAndGetStartAddress(uint64_t pc,
}
++cursor; // Skip ' '.

// Don't subtract 'start_address' from the first entry:
// * If a binary is compiled w/o -pie, then the first entry in
// process maps is likely the binary itself (all dynamic libs
// are mapped higher in address space). For such a binary,
// instruction offset in binary coincides with the actual
// instruction address in virtual memory (as code section
// is mapped to a fixed memory range).
// * If a binary is compiled with -pie, all the modules are
// mapped high at address space (in particular, higher than
// shadow memory of the tool), so the module can't be the
// first entry.
base_address = ((num_maps == 1) ? 0U : start_address) - file_offset;

// Skip to file name. "cursor" now points to dev. We need to
// skip at least two spaces for dev and inode.
int num_spaces = 0;
Expand Down

0 comments on commit debb89a

Please sign in to comment.