Skip to content

Comments

System: use nacl_minidump header from libs/, Saigo doesn't ship it unlike PNaCl#1501

Merged
illwieckz merged 3 commits intomasterfrom
illwieckz/nacl-minidump
Mar 19, 2025
Merged

System: use nacl_minidump header from libs/, Saigo doesn't ship it unlike PNaCl#1501
illwieckz merged 3 commits intomasterfrom
illwieckz/nacl-minidump

Conversation

@illwieckz
Copy link
Member

@illwieckz illwieckz commented Jan 16, 2025

We always had those headers around.

The related .a files are shipped with PNaCl and Saigo for their respective targets.

slipher
slipher reviewed Jan 16, 2025
View reviewed changes
@illwieckz
Copy link
Member Author

illwieckz commented Jan 16, 2025

The nacl_minidump.h file from libs/is exactly the same one as shipped with pnacl.

@illwieckz illwieckz changed the title System: use nacl_minidump and nacl_exception headers from libs/, Saigo doesn't ship them unlike PNaCl System: use nacl_minidump header from libs/, Saigo doesn't ship it unlike PNaCl Jan 16, 2025
@slipher
Copy link
Member

slipher commented Jan 16, 2025

I will test if minidump generation and stack tracing works in a bit. Or if anyone else wants to try it, https://wiki.unvanquished.net/wiki/Breakpad has the procedure.

@slipher
Copy link
Member

slipher commented Jan 17, 2025

A Saigo-built binary can produce a crash dump when crashing under Daemon (though we don't yet know if it's valid). However, Breakpad's dump_syms crashes when asked to symbolize a Saigo-built binary.

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007ffff7afd535 in __GI_abort () at abort.c:79
#2  0x00007ffff7afd40f in __assert_fail_base (fmt=0x7ffff7c5eef0 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
    assertion=0x555555585250 "i == kCFARegister", file=0x5555555851e8 "src/common/dwarf_cfi_to_module.cc", line=175, function=<optimized out>)
    at assert.c:92
#3  0x00007ffff7b0b1a2 in __GI___assert_fail (assertion=assertion@entry=0x555555585250 "i == kCFARegister",
    file=file@entry=0x5555555851e8 "src/common/dwarf_cfi_to_module.cc", line=line@entry=175,
    function=function@entry=0x555555585740 <google_breakpad::DwarfCFIToModule::RegisterName[abi:cxx11](int)::__PRETTY_FUNCTION__> "std::__cxx11::string google_breakpad::DwarfCFIToModule::RegisterName(int)") at assert.c:101
#4  0x000055555555daa0 in google_breakpad::DwarfCFIToModule::RegisterName[abi:cxx11](int) (this=this@entry=0x7fffffffd780, i=i@entry=-2)
    at /usr/include/c++/8/bits/basic_string.h:936
#5  0x000055555555e879 in google_breakpad::DwarfCFIToModule::ValOffsetRule (this=0x7fffffffd780, address=141192, reg=-1, base_register=-2, offset=8)
    at src/common/dwarf_cfi_to_module.cc:236
#6  0x00005555555724c9 in dwarf2reader::CallFrameInfo::State::DoInstruction (this=0x7fffffffd4d0) at src/common/dwarf/dwarf2reader.cc:1966
#7  0x000055555557325c in dwarf2reader::CallFrameInfo::State::InterpretFDE (fde=..., this=0x7fffffffd4d0) at src/common/dwarf/dwarf2reader.cc:1786
#8  dwarf2reader::CallFrameInfo::Start (this=this@entry=0x7fffffffd650) at src/common/dwarf/dwarf2reader.cc:2662
#9  0x000055555557f88c in (anonymous namespace)::LoadDwarfCFI<google_breakpad::ElfClass64> (
    dwarf_filename="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", elf_header=<optimized out>, section_name=<optimized out>,
    section=<optimized out>, eh_frame=<optimized out>, got_section=<optimized out>, text_section=0x7ffff79dabb8, big_endian=false,
    module=0x5555555ab280) at ./src/common/dwarf/dwarf2reader.h:910
#10 0x000055555557fe48 in (anonymous namespace)::LoadSymbols<google_breakpad::ElfClass64> (
    obj_file="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", big_endian=<optimized out>, elf_header=0x7ffff5270000,
    read_gnu_debug_link=<optimized out>, info=0x7fffffffde50, options=..., module=<optimized out>) at /usr/include/c++/8/ext/new_allocator.h:86
#11 0x00005555555810fd in (anonymous namespace)::ReadSymbolDataElfClass<google_breakpad::ElfClass64> (out_module=0x7fffffffdfb8, options=...,
    debug_dirs=..., obj_os="Linux", obj_filename="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", elf_header=0x7ffff5270000)
    at /usr/include/c++/8/bits/stl_iterator.h:783
#12 google_breakpad::ReadSymbolDataInternal (obj_file=0x7ffff5270000 "\177ELF\002\001\001",
    obj_filename="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", obj_os="Linux", debug_dirs=..., options=..., module=0x7fffffffdfb8)
    at src/common/linux/dump_symbols.cc:1061
#13 0x00005555555818d9 in google_breakpad::ReadSymbolData (load_path=..., obj_file="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", obj_os="Linux",
    debug_dirs=std::vector of length 1, capacity 1 = {...}, options=..., module=0x7fffffffdfb8) at src/common/linux/dump_symbols.cc:1139
#14 0x0000555555581906 in google_breakpad::WriteSymbolFile (load_path="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe",
    obj_file="/mnt/c/unv/imgdiff/x/dumptest/sgame-amd64.nexe", obj_os="Linux", debug_dirs=std::vector of length 1, capacity 1 = {...}, options=...,
    sym_stream=...) at src/common/linux/dump_symbols.cc:1076
#15 0x000055555555d064 in main (argc=<optimized out>, argv=<optimized out>) at /usr/include/c++/8/ext/new_allocator.h:79

@slipher
Copy link
Member

slipher commented Jan 17, 2025

Tried upstream Breakpad (https://chromium.googlesource.com/breakpad/breakpad) and dump_syms still crashes at the same place.

@slipher slipher mentioned this pull request Jan 17, 2025
Open
26 tasks
@illwieckz
Copy link
Member Author

illwieckz commented Jan 17, 2025 •
edited
Loading

I added a workaround for finding the minidump-generator and nacl-exception libraries in x86_64-nacl/lib32 when linking an i686 nexe, this workaround will be removed once Saigo is properly repackaged and not mix paths like i686-nacl and x86_64-nacl/lib32.

For some unknown reasons modifying the CMAKE_FIND_ROOT_PATH and enabling FIND_LIBRARY_USE_LIB32_PATHS didn't worked. Fortunately the paths for those libraries are known.

@illwieckz illwieckz force-pushed the illwieckz/nacl-minidump branch from 79f19c8 to 79127a5 Compare January 17, 2025 05:36
@illwieckz illwieckz mentioned this pull request Jan 17, 2025
Closed
@DolceTriade
Copy link
Contributor

DolceTriade commented Jan 17, 2025

Also remember that crash.unvanquished.net is a thing. We just need to remember to upload syms to it.

@DolceTriade DolceTriade mentioned this pull request Feb 20, 2025
Closed
@illwieckz
Copy link
Member Author

illwieckz commented Feb 20, 2025

@DolceTriade I'm reacting to:

This isn't blocked by anything, the fact the feature doesn't work yet when the game is built with Saigo doesn't prevent us to reshape the CMake files to be compatible with more than one toolchain. It's not CMake's fault.

For the C++ change, in fact we were lucky it built with PNaCl, it only built previously because we luckily relied on a copy of the header that was done in our back (and luckily, it was the exact same one).

@DolceTriade
Copy link
Contributor

DolceTriade commented Feb 21, 2025

@slipher I'm fine merging this. Do you have any objections?

@slipher
Copy link
Member

slipher commented Feb 21, 2025

@slipher I'm fine merging this. Do you have any objections?

No

@illwieckz illwieckz force-pushed the illwieckz/nacl-minidump branch 2 times, most recently from daad1cb to 4e65aa9 Compare February 21, 2025 21:50
@illwieckz
Copy link
Member Author

illwieckz commented Feb 21, 2025

I just added this:

- if (NOT NACL_MINIDUMP)
+ if (NOT NACL_MINIDUMP AND "${NACL_TARGET}" STREQUAL "i686" AND EXISTS "${DEPS_DIR}/saigo_newlib/x86_64-nacl/lib32")

The NACL_TARGET test both tests if Saigo is used and the target is i686, PNaCl Clang compiles to pexe, not to i686, hence NACL_TARGET is unset with PNaCl Clang.

The x86_64-nacl/lib32 path is specific to the Google-built Saigo, builds from DaemonSaigoNativeClientToolkit will provide i686-nacl/lib instead and then the original code is expected to work out of the box once it is properly packaged.

@illwieckz illwieckz force-pushed the illwieckz/nacl-minidump branch from 4e65aa9 to b0d70fa Compare February 21, 2025 21:55
@slipher
Copy link
Member

slipher commented Feb 22, 2025

LGTM the new commit

@illwieckz illwieckz merged commit e91cb12 into master Mar 19, 2025
9 checks passed
@illwieckz illwieckz deleted the illwieckz/nacl-minidump branch March 19, 2025 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slipher slipher slipher left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@illwieckz @slipher @DolceTriade