8000 win32 patching shouldn't use as much stack (Was: Stack overflow caused by PatchAllModules() function) · Issue #1365 · gperftools/gperftools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

win32 patching shouldn't use as much stack (Was: Stack overflow caused by PatchAllModules() function) #1365

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.

Sign up for GitHub

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

Jump to bottom
Open
p0358 opened this issue Aug 14, 2022 · 1 comment
Open

win32 patching shouldn't use as much stack (Was: Stack overflow caused by PatchAllModules() function) #1365

p0358 opened this issue Aug 14, 2022 · 1 comment
Labels
enhancement porting trivial-to-contribute windows

Comments

@p0358
Copy link
p0358 commented Aug 14, 2022 •
edited
Loading

The function PatchAllModules() located at:

gperftools/src/windows/patch_functions.cc

Line 696 in f7c6fb6

bool PatchAllModules() {

is using 65732 bytes of stack, which already triggers an MSVC warning on its own. I had increased the stack reserve size, but it did not help in my case, one user has been getting crash while a GPU driver was being loaded regardless. Doing ?? int(@$teb->NtTib.StackBase) - int(@$teb->NtTib.StackLimit) in WinDbg showed that the stack size is 61440, and the stack overflow crash trace fragment is:

libtcmalloc_minimal.dll     0x7fff69755dd7      __chkstk (chkstk.asm:109)
libtcmalloc_minimal.dll     0x7fff697426ee      `anonymous namespace'::PatchAllModules (patch_functions.cc:696)
libtcmalloc_minimal.dll     0x7fff69748e34      `anonymous namespace'::WindowsInfo::Perftools_LoadLibraryExW (patch_functions.cc:1022)

I tried moving the variables hModules (seems to be main culprit), modules and currently_loaded_modules into globals to have them located at the heap (replacing declaration with clearing instructions in case of the vector and set inside of the function), and that seems to have fixed the issue.

I would have created a PR, but I'm not familiar enough with the codebase to gauge whether these should be converted to static globals or allocated dynamically...

Edit: While default stack reserve size is over 1 MB, some reverse engineering of the Nvidia driver's DLL reveals that they supply the dwStackSize parameter to CreateThread with a value of 0x10000 (65536, so more than PatchAllModules() itself used!) for some threads. One of those happens to be a thread that loads Windows API DLLs, and as such runs into this issue.
@alk
Copy link
Contributor
alk commented Jul 3, 2023

I think allocating those arrays via tc_malloc/tc_free should work. So go ahead and contribute. If tcmalloc_minimal_unittest passes, then it works. Thanks for reporting it.

@alk alk changed the title Stack overflow caused by PatchAllModules() function win32 patching shouldn't use as much stack (Was: Stack overflow caused by PatchAllModules() function) Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement porting trivial-to-contribute windows
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@alk @p0358
0