don't break stack walking #308
Conversation
src/detours.cpp
Outdated
| // to register this information. This supports kernel stack walks. | ||
| // See https://learn.microsoft.com/en-us/cpp/build/exception-handling-x64 | ||
| DWORD64 imageBase; | ||
| if (NULL == RtlLookupFunctionEntry((DWORD64)pDetour, &imageBase, NULL)) { |
There was a problem hiding this comment.
In some cases, A Detour function could legitimately be a leaf function and thus have no function table entry, so having this requirement is somewhat over-strict.
Regardless, this shouldn't be just DETOURS_X64 specific - function table unwinding is also used on ARM64, for example.
There was a problem hiding this comment.
re: leaf functions
Table entries are also required for any functions that allocate stack space.
But point taken that the detour could be a very simple function.
The first version of this PR simply checked for MEM_IMAGE - and trusted the compiler to have done its job. On consideration though I changed this to the function table lookup - since I didn't want to entirely preclude the use of a JIT detour. A combination of these two approaches would seem reasonable.
re: ARM64
Agreed, but that's not something I felt comfortable writing.
Detouring a function should not break our ability to walk the call stack.
On X64, suggest that we require the Detour to reside in MEM_IMAGE so that Windows has access to the compiler generated function information necessary for unwinding. This will prevent surprises.