search

GregTheDev / MinidumpExplorer

Inspect and capture minidump files. Includes stand alone library for reading minidump files.
MIT License
90 stars 19 forks source link

Add support for ThreadNamesStream #3

Closed GregTheDev closed 5 years ago

ssbssa commented 4 years ago

I just tried to open a crashdump with thread-names, and the Minidump Explorer crashed. I think it's because RvaOfThreadName should be RVA64 instead of RVA.

GregTheDev commented 4 years ago

Thanks for the feedback @ssbssa, will have a look and get back to you.

Edit: it looks like thread names are being stored as ascii and I had originally coded them expecting unicode. The newer minidump streams aren't documented anymore, so I have to decode them straight from the Windows SDK header file, so sometimes it's a bit of guess work. My guess is that I was running an insider build and maybe something changed along the way. Either way I'll work on a fix for you.

ssbssa commented 4 years ago

No, RvaOfThreadName points to a MINIDUMP_STRING, which is unicode. I have attached a small crashdump file where 3 of the threads have a name set.

crash-allocer32-td-2.zip

GregTheDev commented 4 years ago

Thanks for your sample, that's a great help.

I checked yours, and yup, the thread names are unicode. I also checked one of my sample crashdumps that I test on and the names there are ascii. How did you set your thread names? I used SetThreadDescription() from Kernel32.dll. The documentation says the name is definitely supposed to be unicode, so it's possible I used it incorrectly.

Thread names - ascii

Thread names - unicode

ssbssa commented 4 years ago

Yes, I also used SetThreadDescription(). It sounds to me that in your example SetThreadDescription() was wrongly used with an ascii string.

GregTheDev commented 4 years ago

Yup, my interop code for SetThreadDescription() was incorrect. Have fixed it now and things seem fine.

Try the file attached and let me know how it goes? It contains some code that's still work in progress, but it won't affect the main app yet.

MinidumpExplorer.ThreadNames.zip

ssbssa commented 4 years ago

Yes, this version works, thanks.