Skip to content

Crash Screen Updates - #888

Open
someone2639 wants to merge 126 commits into
HackerN64:develop/2.4.0from
someone2639:crash_c1
Open

Crash Screen Updates#888
someone2639 wants to merge 126 commits into
HackerN64:develop/2.4.0from
someone2639:crash_c1

Conversation

@someone2639

@someone2639 someone2639 commented Jun 16, 2025

Copy link
Copy Markdown
Collaborator
  • Implement remaining branch instructions ✅

  • Implement COP1 instructions and syscall

  • Implement .sym format and n64sym tool from libdragon ✅

  • File/line support as a result ✅

  • Change font to something with lowercase and symbols ✅

  • Move elements around to fill the whole screen ✅

  • Arbitrary text coloring ✅

  • Simple Overview page with only what's necessary ✅

  • Implement break cache eret sync (Not sure if really necessary)

Comment thread src/game/mario.c Outdated
@arthurtilly

Copy link
Copy Markdown
Collaborator

can we also remove those stupid shadow instructions that appear behind the real ones. whats up with that

@gheskett

Copy link
Copy Markdown
Collaborator

can we also remove those stupid shadow instructions that appear behind the real ones. whats up with that

Probably just a meme with crash screen transparency and scrolling. Also that asymmetrical box drawing thing has been a thing for a century (though that should probably get corrected already, why is that still this broken...)

@someone2639 someone2639 changed the title Crash Screen Disasm and Map Packer Updates Crash Screen Updates Jun 17, 2025
Comment thread Makefile Outdated
Comment thread Makefile Outdated
Comment thread src/debugger/crash_screen.c Outdated
Comment thread src/debugger/crash_screen.c Outdated
Comment thread src/debugger/disasm.c
@someone2639
someone2639 marked this pull request as ready for review June 24, 2025 01:55
@someone2639

Copy link
Copy Markdown
Collaborator Author

Ready for Review!

  • n64sym tool needs testing on other distros, since I run "/usr/bin/%s-addr2line" % $(CROSS), and I'm only 90% confident that works on everything
  • Stack Trace and symbol exporting can be disabled, though I feel like disasm is important enough to stay (that's how it was in the old crash screen); This saved at least 300KB in the rom due to not even packing the symbol table in the ROM
  • Since everything is guarded by a define, removed the if parse_map == MAP_PARSER_ADDRESS style lines and replaced them with ifdef's
  • Color support in overview soon:tm:

@gheskett gheskett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will review the shiny new denser things later (and probably gloss over the copied libdragon stuff if you can point me to which is which)

Comment thread Makefile Outdated
Comment thread src/debugger/assert.h Outdated
Comment thread src/debugger/assert.h
Comment thread src/game/debug.h
Comment thread src/game/fasttext.c Outdated
@gheskett gheskett added enhancement New feature or request cleanup Removal of useless or bloat code/features labels Jun 24, 2025
@gheskett gheskett added this to the 2.4 milestone Jun 24, 2025
@gheskett gheskett added the bug Something isn't working label Jun 24, 2025
@gheskett

gheskett commented Jun 24, 2025

Copy link
Copy Markdown
Collaborator

Also since we're touching it anyway, can we replace the HackerSM64 syscall with a proper trapping alternative? (Discuss with Falco on this probably, but even FORCE_CRASH would probably work well enough, plus that'd also remove the need of using a separate ASM file for assertions.)

@someone2639

Copy link
Copy Markdown
Collaborator Author

switching assert strategies seems like a rabbit hole since it's either a trap, break, or syscall depending on who you ask (it's even a break 0 in libultra technically)

@mpharoah

Copy link
Copy Markdown
Collaborator

The issue isn't using syscall, break, or teq $0, $0, the issue is that the crash screen is blindly trusting that memory set only in the HackerSM64 assert call is a valid pointer, and crashes if not, causing the crash screen to crash if the BREAK/SYSCALL happens from 3rd party code. Just need to validate that the pointers to the assert string have actually been set.

@someone2639

Copy link
Copy Markdown
Collaborator Author

That seems like an easy enough fix, though some time down the line (read: NOT IN THIS PR), we should come up with an ABI either here or in hackerlibultra where we can decide that e.g. break with specifically a value of 0 is a failed assert while break 6 isn't

@mpharoah

Copy link
Copy Markdown
Collaborator

Good idea. My suggestion would be SYSCALL 18995 (18995 being the 20 bits that give ASRT when base-32 encoded)

Comment thread src/debugger/stacktrace.c Outdated
@someone2639

Copy link
Copy Markdown
Collaborator Author

Ready for another lap 🏇

Comment thread tools/Makefile Outdated
@gheskett gheskett moved this from In progress to Needs Review in 2.4 (Minor Feature Update) Apr 28, 2026
@gheskett

Copy link
Copy Markdown
Collaborator

Final:tm: feature request: Add a Makefile flag that can override optimizations on everything and replace them with -O0. Additionally add an attribute for this in macros.h that can be applied to individual functions.

@gheskett

Copy link
Copy Markdown
Collaborator

Also a way to replace -g1 with -g2/-g3 since I think GDB wants that

@someone2639

Copy link
Copy Markdown
Collaborator Author

Ready for another lap 🏇

Comment thread Makefile
@github-project-automation github-project-automation Bot moved this from Needs Review to In progress in 2.4 (Minor Feature Update) Apr 30, 2026
@gheskett gheskett moved this from In progress to Needs Review in 2.4 (Minor Feature Update) May 11, 2026

@gheskett gheskett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I hate to be the bearer of bad news, but...the crash screen is still crashing on my end.

@github-project-automation github-project-automation Bot moved this from Needs Review to In progress in 2.4 (Minor Feature Update) May 11, 2026
@someone2639

Copy link
Copy Markdown
Collaborator Author

Ready for another lap 🏇

@gheskett

gheskett commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

That did it, crash screen crash is gone. Will follow up with additional testing.

@someone2639

Copy link
Copy Markdown
Collaborator Author

Ready for another lap 🏇

@arthurtilly

Copy link
Copy Markdown
Collaborator

@claude review this

Comment thread src/debugger/map_parser.h
typedef struct {
char *file;
char *func;
char func[64];

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this safe for function names 64+ bytes long? Or does map packer handle this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symt_string eventually handles the dma and only copies in the number of bytes that the buffer passed in can hold

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image it does not crash but i also couldve sworn i truncated these...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How safe are these uses of sizeof, given these are strings?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And for all similar functions/variables?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocker This MUST be resolved before the feature branch can be merged. bug Something isn't working cleanup Removal of useless or bloat code/features enhancement New feature or request

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

Crash Screen Displays Broken Crash Function

5 participants