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

[DRWTSN32] Initial rough implementation #145

Merged
merged 7 commits into from Jan 6, 2018

Conversation

learn-more
Copy link
Member

@learn-more learn-more commented Nov 25, 2017

Purpose

This aims to add a rough sketch of how drwtsn32 could look like.
DrWtsn32 is a crash reporter.

TODO

  • Figure out why the stacktrace does not work
  • Output to a file (on the desktop?) instead of stdout.
  • Figure out how to get the correct exception, instead of the attach breakpoint.
  • Notify the user about the stacktrace dumped on the desktop.
  • Disassemble the offending function (maybe move the kdb disassembler into a library?)

@learn-more learn-more added the WIP label Nov 25, 2017
@HBelusca
Copy link
Contributor

\o/
You should also ask @encodedpr what the status of his crash-reporter is.


set_cpp(WITH_RUNTIME WITH_EXCEPTIONS WITH_STL)


Copy link
Contributor

Choose a reason for hiding this comment

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

Extra newline

@@ -0,0 +1,118 @@
#include "precomp.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing code source copyright header.

case CREATE_PROCESS_DEBUG_EVENT:
{
data.ProcessPath.resize(MAX_PATH);
DWORD len = GetModuleFileNameExA(evt.u.CreateProcessInfo.hProcess, NULL, &data.ProcessPath[0], data.ProcessPath.size());
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using UNICODE everywhere?
But then the STL stuff should be like std::wstring & co.
Unless you want to use ATL?

Copy link
Member Author

Choose a reason for hiding this comment

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

Eventually I want to print this to a file instead of stdout, and then UNICODE is not really convenient.

@encodedpr
Copy link
Contributor

Status = STATUS_NOTHING

@yagoulas
Copy link
Member

Here is the code of the logger I'm working on based on your code. I want it to be able to log a backtrace for the exceptions it may encounter so in the end the two will share a lot of code:

https://github.com/yagoulas/reactos/blob/roslogger/base/applications/roslogger/roslogger.cpp

@Mouvedia
Copy link

Please don't add an unique identifier.
cf https://www.schneier.com/blog/archives/2017/08/nsa_collects_ms.html

@HBelusca
Copy link
Contributor

@Mouvedia , you are completely out of topic. That spying thingie is when reports are sent onto the internet to MS. Here the PR is about just a mere post-mortem debugger.

@Mouvedia
Copy link

Mouvedia commented Dec 16, 2017

@HBelusca if you are planning to send the crash reports sometime in the future it will become relevant.

@ThFabba
Copy link
Member

ThFabba commented Dec 16, 2017

It's a valid concern, although not for the current implementation.
However the "unique identifier" that article talks about is the IP address. It's, uhm... pretty hard to send things on the internet without including that.
Also, end-to-end encryption mitigates this problem, as the article also mentions.

@Mouvedia
Copy link

Mouvedia commented Dec 16, 2017

However the "unique identifier" that article talks about is the IP address.

I was talking about HKEY_LOCAL_MACHINE\Software\Microsoft\Cryptography\MachineGuid.
The IP can be dynamic, it's less critical.

@gigaherz
Copy link
Member

What does the MachineGuid registry key have to do with this? This is a drwatson PR, implementing a debugging tool. That key is used exclusively by the cryptographic components.

@learn-more
Copy link
Member Author

Please keep your tin foil hat BS out of this PR, thanks.

@learn-more
Copy link
Member Author

The disassembly of the offending function will be added later, I want to go ahead and get this in.
Please review.

(After reviewing, all commits will be squashed to one)

return true;
}
break;
case 0x406d1388:
Copy link
Member

Choose a reason for hiding this comment

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

Magic value

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough :)

@learn-more learn-more merged commit 280d7a9 into reactos:master Jan 6, 2018
@learn-more learn-more deleted the drwtsn branch January 6, 2018 10:47
dfbag7 pushed a commit to dfbag7/reactos that referenced this pull request Jan 14, 2018
On application crash, drwtsn32 will attach to the application and try to get a dump, consisting of:
- List of loaded modules
- List of loaded threads
- Per thread, a stacktrace
- Per thread, a small hexdump from the stack
- Per thread, a dump of the most common registers

This dump is saved to the desktop, and the user is notified of the dump being dropped there.

CORE-14180
reactos#145
dfbag7 pushed a commit to dfbag7/reactos that referenced this pull request Jan 14, 2018
On application crash, drwtsn32 will attach to the application and try to get a dump, consisting of:
- List of loaded modules
- List of loaded threads
- Per thread, a stacktrace
- Per thread, a small hexdump from the stack
- Per thread, a dump of the most common registers

This dump is saved to the desktop, and the user is notified of the dump being dropped there.

CORE-14180
reactos#145
julenuri added a commit to julenuri/reactos that referenced this pull request Dec 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants