Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hunk_allocator: break on hunk error #1187

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

illwieckz
Copy link
Member

Extracted from:

Make sure the error produces a calltrace on GDB when it happens.

While working on #1179 I did a stupid mistake at some point, mistake that produced that error, but I had no clue which code was producing the error as the engine just caught it and exited nicely. There is high chance the allocation or the usage of the allocated memory is done in the same file than the freeing is done, or that the freeing code is obviously identified as being part of the same mechanism of other functions allocating or using the memory.

@@ -325,6 +325,7 @@ void Hunk_FreeTempMemory( void *buf )

if ( hdr->magic != (int) HUNK_MAGIC )
{
ASSERT( false );
Copy link
Member

Choose a reason for hiding this comment

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

This is not a good way to do it as we use some compiler builtins to instruct the compiler to assume that the assert condition is always true. So the compiler may remove the Sys::Error.

Maybe you should put something like if (IsDebuggerAttached) BREAKPOINT; inside of Sys::Error to solve the problem of catching errors in the debugger.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Actually we may discuss the topic of always adding a breakpoing within Sys::Error instead, but this is good enough for now.

@illwieckz illwieckz changed the title hunk_allocator: assert on hunk error hunk_allocator: break on hunk error Jun 11, 2024
@slipher
Copy link
Member

slipher commented Jun 14, 2024

Come to think of it, losing a Sys::Error trace is never a problem for me (at least when debugging the client) because Sys::Error brings up a dialog box which blocks until it is closed. As I recall it works the same on Linux right? To get the stack trace you need only pause.

As for breaking always on error, freem brought up a good example in IRC today of a Sys::Error where you would not want to break: the shutting down due to time overflow ones. But I'm also loath to add breaks to random callsites one by one.

@illwieckz
Copy link
Member Author

illwieckz commented Jun 14, 2024

95% of my gdb workflow is to simply build the game and run the game over gdb, to dump a backtrace on error and to return, all within a single command.

I only need to spend time in doing more advanced things like pausing, stepping, reading values… in the remaining 5%.

Of course when the error UI spawns, I can just hit Ctrl+C and get my backtrace, but even requiring such manual break feels annoying.

This just changes my debugging experience from “The error has already been caught, just give me the backtrace, what are you waiting for!???” to “Ah, it errored out, let's read what gdb has to say!”. 😀️

@slipher
Copy link
Member

slipher commented Jun 14, 2024

If you have a super advanced and customized debugging setup, then you can just inject a GDB script with preprogrammed breakpoints wherever you please. No need to modify the code. Trying to force a break from the code itself feels kind of wrong anyway... the debugger should be in charge of that.

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

Successfully merging this pull request may close these issues.

2 participants