4 ms·

It’s been more than a decade since I worked in games, but for my entire games career, any and all use of C++ exceptions was strictly disallowed due to other hor

by dahart 2y ago

It’s been more than a decade since I worked in games, but for my entire games career, any and all use of C++ exceptions was strictly disallowed due to other horror stories. I was still bit in a very similar way by someone’s C++ copy constructor trickery - a crash that only happened in a release build after playing the game for a while, with a stack corruption. Like the author, for me this was one of the hardest bugs I ever had to track down, and I ended up writing a tiny release mode debugger that logged call stacks in order to do it. Once I was able to catch the corruption (after several days of debugging during a crunch weekend), someone on my team noticed the stomp values looked like floating point numbers, and pretty quickly we figured out it was coming from the matrix class trying to be too clever with it’s reference counting IIRC. There’d been a team of around a dozen people trying to track this down during overtime, so it suddenly hit me once we fixed it that someone’s cute idea that took maybe 10 seconds to write cost several tens of thousands of dollars to fix.

ziml77 2y ago
This could still happen without exceptions though, right? The flow is more explicit without exceptions, but returning an error code up the stack would have the same effect of causing the memory that select() is referencing to possibly be used for a different purpose when it writes its result.
- mark_undoio 2y ago
  My reading is that the problem was specifically that they were injecting an exception to recover control from the C library back to their code. It seems like the select() was within its rights to have passed a stack allocated buffer to be written asynchronously by the kernel since it, presumably, knew it couldn't encounter any exceptions. But injecting one has broken that assumption. If the select() implementation had returned normally with an error or was expecting then I'd assume this wouldn't have happened.
  - ryao 2y ago
    According to the documentation, the wait is terminated by APC calls, so there is no need to trigger an exception in the APC call to return control to C++: https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select https://learn.microsoft.com/en-us/windows/win32/api/winsock2... https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobjectex https://learn.microsoft.com/en-us/windows/win32/api/synchapi... If they had implemented this to use a C++ exception to return control to C++, they should have encountered this stack corruption issue immediately upon implementing this, rather than have a customer some point in the future hit it before they did. My read of this is that they had the callback function do something and someone eventually got it to throw an exception. This is undefined behavior because there is no correct way to unwind a C stack frame. However, that is not obvious, especially if you test it since if the C function does nothing special such that it needs no special clean up, everything should be fine. However, WaitForSingleObjectEx() does something extremely special. Skipping that by unwinding the stack bit them. I filed bugs against both GCC and LLVM requesting warnings to protect people from doing this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118263 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118263 https://github.com/llvm/llvm-project/issues/121427 https://github.com/llvm/llvm-project/issues/121427
- tom_ 2y ago
  There are no error returns from an APC? The return type is void and the system expects the routine (whatever it is) to return: https://learn.microsoft.com/en-us/windows/win32/api/winnt/nc-winnt-papcfunc https://learn.microsoft.com/en-us/windows/win32/api/winnt/nc... - whichever call put the process in the alertable wait state then ends up returning early. This is a little bit like the Win32 analogue of POSIX signal handlers and EINTR, I suppose.
  - ziml77 2y ago
    Oh I see my confusion. I misunderstood how the APC fit in. Now it makes sense why specifically an exception was problematic.
intelVISA 2y ago
Clever code is always expensive, either you're paying for somebody smart to work at their cognitive peak which is less productive for them than 'simpler' code, or more likely you'll instead pay multiples more down the line for someone's hubris. I think this is the rare direction that more langs should follow Rust in that 'clever' code can be more easily quarantined for scrutiny via unsafe.
- hinkley 2y ago
  The time when people see this the clearest is during an outage. You’re in a stressful situation and it’s being made worse and longer by clever code. Do you see now why maybe clever code isn’t a good idea? Cortisol actually reduces the effectiveness of working memory. Write your code like someone looking at it is already having a bad day, because not only are odds very good they will be, but that’s the costliest time for them to be looking at it. Probability x consequences.
- deleted 2y ago
  [deleted]
hinkley 2y ago
I appreciate the notion of Grug Brained development but at the end of the day it’s just a slightly sardonic restatement of Kernighan’s Law, which is easier to get people to buy into in negotiations. Grug brain is maybe good for 1:1 interactions or over coffee with the people you vent to or are vented to.

by dahart 2y ago

ziml77 2y ago

This could still happen without exceptions though, right? The flow is more explicit without exceptions, but returning an error code up the stack would have the same effect of causing the memory that select() is referencing to possibly be used for a different purpose when it writes its result.

mark_undoio 2y ago
My reading is that the problem was specifically that they were injecting an exception to recover control from the C library back to their code. It seems like the select() was within its rights to have passed a stack allocated buffer to be written asynchronously by the kernel since it, presumably, knew it couldn't encounter any exceptions. But injecting one has broken that assumption. If the select() implementation had returned normally with an error or was expecting then I'd assume this wouldn't have happened.
- ryao 2y ago
  According to the documentation, the wait is terminated by APC calls, so there is no need to trigger an exception in the APC call to return control to C++: https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-select https://learn.microsoft.com/en-us/windows/win32/api/winsock2... https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobjectex https://learn.microsoft.com/en-us/windows/win32/api/synchapi... If they had implemented this to use a C++ exception to return control to C++, they should have encountered this stack corruption issue immediately upon implementing this, rather than have a customer some point in the future hit it before they did. My read of this is that they had the callback function do something and someone eventually got it to throw an exception. This is undefined behavior because there is no correct way to unwind a C stack frame. However, that is not obvious, especially if you test it since if the C function does nothing special such that it needs no special clean up, everything should be fine. However, WaitForSingleObjectEx() does something extremely special. Skipping that by unwinding the stack bit them. I filed bugs against both GCC and LLVM requesting warnings to protect people from doing this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118263 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118263 https://github.com/llvm/llvm-project/issues/121427 https://github.com/llvm/llvm-project/issues/121427
tom_ 2y ago
There are no error returns from an APC? The return type is void and the system expects the routine (whatever it is) to return: https://learn.microsoft.com/en-us/windows/win32/api/winnt/nc-winnt-papcfunc https://learn.microsoft.com/en-us/windows/win32/api/winnt/nc... - whichever call put the process in the alertable wait state then ends up returning early. This is a little bit like the Win32 analogue of POSIX signal handlers and EINTR, I suppose.
- ziml77 2y ago
  Oh I see my confusion. I misunderstood how the APC fit in. Now it makes sense why specifically an exception was problematic.

intelVISA 2y ago

Clever code is always expensive, either you're paying for somebody smart to work at their cognitive peak which is less productive for them than 'simpler' code, or more likely you'll instead pay multiples more down the line for someone's hubris. I think this is the rare direction that more langs should follow Rust in that 'clever' code can be more easily quarantined for scrutiny via unsafe.

hinkley 2y ago
The time when people see this the clearest is during an outage. You’re in a stressful situation and it’s being made worse and longer by clever code. Do you see now why maybe clever code isn’t a good idea? Cortisol actually reduces the effectiveness of working memory. Write your code like someone looking at it is already having a bad day, because not only are odds very good they will be, but that’s the costliest time for them to be looking at it. Probability x consequences.
deleted 2y ago
[deleted]

hinkley 2y ago

I appreciate the notion of Grug Brained development but at the end of the day it’s just a slightly sardonic restatement of Kernighan’s Law, which is easier to get people to buy into in negotiations. Grug brain is maybe good for 1:1 interactions or over coffee with the people you vent to or are vented to.