From 94b6309aecaddfcf11672f6cfad9575d68ad3b40 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Fri, 26 May 2017 09:32:08 -0700 Subject: [PATCH] Merge exec and non-exec segments while merging executable bit. The bfd and gold linkers create segments like this: r/x, r/w where the r/x segment covers the start of the ELF file. lld's segments look like this: r, r/x, r/w where the r segment covers the start of the ELF file. So we cannot rely on the location of the r/x to tell where the start of the ELF is. But we can still rely on the r and r/x mappings being adjacent. So what we do is when we see an r segment followed by an r/x, merge the r into the r/x and claim that it is executable. This way, the minidump writer will continue to see a single executable segment covering the entire executable. Testing: "make check" passes when breakpad is compiled with lld compiled from trunk (requires bug fix from LLVM r303689). Also patched change into chromium and tested these builds: $ cat args.gn is_chrome_branded = true is_debug = false is_official_build = true use_lld = true allow_posix_link_time_opt = false is_cfi = false $ cat args.gn target_os = "android" target_cpu = "arm" is_debug = false is_official_build = true is_chrome_branded = true With both builds breakpad_unittests passes and chrome/chrome_modern_public_apk create good minidumps after navigating to chrome://inducebrowsercrashforrealz (checked that minidump contains stack trace entry for content::HandleDebugURL). Bug: chromium:716484 Change-Id: Ib6ed3a8420b83acf4a5962843930fb006734cb95 Reviewed-on: https://chromium-review.googlesource.com/513610 Reviewed-by: Primiano Tucci --- src/client/linux/minidump_writer/linux_dumper.cc | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc index 8de32d44..141aa07c 100644 --- a/src/client/linux/minidump_writer/linux_dumper.cc +++ b/src/client/linux/minidump_writer/linux_dumper.cc @@ -520,15 +520,20 @@ bool LinuxDumper::EnumerateMappings() { name = kLinuxGateLibraryName; offset = 0; } - // Merge adjacent mappings with the same name into one module, - // assuming they're a single library mapped by the dynamic linker + // Merge adjacent mappings into one module, assuming they're a single + // library mapped by the dynamic linker. Do this only if their name + // matches and either they have the same +x protection flag, or if the + // previous mapping is not executable and the new one is, to handle + // lld's output (see crbug.com/716484). if (name && !mappings_.empty()) { MappingInfo* module = mappings_.back(); if ((start_addr == module->start_addr + module->size) && (my_strlen(name) == my_strlen(module->name)) && (my_strncmp(name, module->name, my_strlen(name)) == 0) && - (exec == module->exec)) { + ((exec == module->exec) || (!module->exec && exec))) { + module->system_mapping_info.end_addr = end_addr; module->size = end_addr - module->start_addr; + module->exec |= exec; line_reader->PopLine(line_len); continue; }