Fixes for Linux minidump_stackwalk (using mold linker)

[Marc Gonzalez]

Hello,

Linkers such as lld and mold produce binaries with data & text
sections laid out differently than the GNU linker.

This confuses minidump_stackwalk, and breaks symbol lookup.

Relevant discussion:
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4582790

I'll add as replies my patches fixing the issue.

Regards

[Marc Gonzalez]

Apparently, minidump_stackwalk expects symbols files to contain
offsets from the first page containing the executable segment.
---
src/common/linux/dump_symbols.cc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc
index 86c948d9..302d4cde 100644
--- a/src/common/linux/dump_symbols.cc
+++ b/src/common/linux/dump_symbols.cc
@@ -179,8 +179,8 @@ typename ElfClass::Addr GetLoadingAddress(
// normally be zero.
for (int i = 0; i < nheader; ++i) {
const Phdr& header = program_headers[i];
- if (header.p_type == PT_LOAD)
- return header.p_vaddr;
+ if (header.p_type == PT_LOAD && header.p_flags & PF_X)
+ return header.p_vaddr & ~(typename ElfClass::Addr)0xFFF;
}
return 0;
}
--
2.34.1

[Marc Gonzalez]

We need accurate information on the executable section.
---
src/client/linux/minidump_writer/linux_dumper.cc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc
index 5c4c389c..bdf7966e 100644
--- a/src/client/linux/minidump_writer/linux_dumper.cc
+++ b/src/client/linux/minidump_writer/linux_dumper.cc
@@ -588,6 +588,7 @@ bool LinuxDumper::EnumerateMappings() {
name = kLinuxGateLibraryName;
offset = 0;
}
+#if 0
// 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
@@ -606,6 +607,7 @@ bool LinuxDumper::EnumerateMappings() {
continue;
}
}
+#endif
MappingInfo* const module = new(allocator_) MappingInfo;
mappings_.push_back(module);
my_memset(module, 0, sizeof(MappingInfo));
--
2.34.1

[Marc Gonzalez]

Data segments are not needed in minidump_stackwalk
---
src/client/linux/minidump_writer/linux_dumper.cc | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc
index bdf7966e..0be16515 100644
--- a/src/client/linux/minidump_writer/linux_dumper.cc
+++ b/src/client/linux/minidump_writer/linux_dumper.cc
@@ -608,6 +608,11 @@ bool LinuxDumper::EnumerateMappings() {
}
}
#endif
+ char c = 0;
+ sscanf(line, "%*s %*s %*s %*s %*s %c", &c);
+ if (c == '/' && !exec) /* skip non-executable file-based segments */
+ goto next_line;
+

MappingInfo* const module = new(allocator_) MappingInfo;
mappings_.push_back(module);
my_memset(module, 0, sizeof(MappingInfo));

@@ -625,6 +630,7 @@ bool LinuxDumper::EnumerateMappings() {
}
}
}
+next_line:
line_reader->PopLine(line_len);
}

--
2.34.1

[Mark Mentovai]

Hi, Marc. Thanks for your patches!

We don’t accept patch submissions by e-mail. Breakpad has a Chromium-like development process, and patch review is conducted on Gerrit. In general, a patch can be submitted for review by using git cl upload, provided that you have Chromium’s depot_tools in your PATH.

[Marc Gonzalez]

On 05/07/2023 17:57, Mark Mentovai wrote:

> Hi, Marc. Thanks for your patches!
>
> We don't accept patch submissions by e-mail.
> Breakpad has a Chromium-like development process,
> and patch review is conducted on Gerrit.
> In general, a patch can be submitted for review by using git cl upload,
> provided that you have Chromium's depot_tools in your PATH.

Hello Mark,

Roger that.
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4668738
https://chromium-review.googlesource.com/c/breakpad/breakpad/+/4668735