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

Initial implementation of heap parsing for Windows #14218

Merged
merged 23 commits into from Jun 5, 2019

Conversation

GustavoLCR
Copy link
Contributor

@GustavoLCR GustavoLCR commented Jun 4, 2019

Only tested on Windows 10 x64, but at least the basics should work on every version.

Also please review

@GustavoLCR GustavoLCR force-pushed the windows-heap branch 3 times, most recently from 802a17f to 7c9dc6a Compare June 4, 2019 03:40
Copy link
Collaborator

@lowlyw lowlyw left a comment

Choose a reason for hiding this comment

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

this probably needs more eyes than the amount of time i've given it, it's a ton of code 👍 . can you add some tests?

libr/core/windows_heap.c Show resolved Hide resolved
libr/core/windows_heap.c Outdated Show resolved Hide resolved
libr/core/windows_heap.c Outdated Show resolved Hide resolved
libr/core/windows_heap.c Outdated Show resolved Hide resolved
Tested only on Windows 10 1809 x64
TODO:
Clean/organize this: Order and Style
-Can this be useful in unix? does mdmp even have a heap? remote dbg session?
Copy link
Collaborator

Choose a reason for hiding this comment

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

yes! may be cool for remote debug!

@codecov-io
Copy link

codecov-io commented Jun 4, 2019

Codecov Report

Merging #14218 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14218      +/-   ##
==========================================
- Coverage   37.83%   37.83%   -0.01%     
==========================================
  Files         949      949              
  Lines      305172   305240      +68     
==========================================
+ Hits       115469   115475       +6     
- Misses     189703   189765      +62
Impacted Files Coverage Δ
libr/cons/cons.c 53.7% <ø> (ø) ⬆️
libr/core/cmd_debug.c 22.32% <ø> (ø) ⬆️
libr/debug/p/debug_native.c 52.95% <ø> (ø) ⬆️
libr/util/sys.c 52.44% <0%> (-3.3%) ⬇️
libr/cons/rgb.c 48.43% <0%> (-1.57%) ⬇️
libr/core/panels.c 0% <0%> (ø) ⬆️
libr/cons/pal.c 63.32% <0%> (+0.52%) ⬆️
libr/core/cmd_eval.c 48.09% <0%> (+1.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7366b6a...b80282e. Read the comment docs.

libr/cons/cons.c Outdated Show resolved Hide resolved
libr/core/cmd_debug.c Outdated Show resolved Hide resolved
libr/debug/p/native/w32.h Outdated Show resolved Hide resolved
@radare radare added this to the 3.6.0 milestone Jun 4, 2019
Copy link
Collaborator

@radare radare left a comment

Choose a reason for hiding this comment

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

.

libr/core/windows_heap.c Show resolved Hide resolved
libr/core/windows_heap.c Outdated Show resolved Hide resolved
libr/core/windows_heap.c Outdated Show resolved Hide resolved
libr/core/windows_heap.c Show resolved Hide resolved
libr/cons/cons.c Outdated Show resolved Hide resolved
libr/debug/p/native/maps/windows_maps.c Outdated Show resolved Hide resolved
#define WINDOWS_MAPS_H
#include <r_core.h>

R_API RList *w32_dbg_modules(RDebug *dbg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

public fcn namespacec violation

libr/debug/p/native/w32.c Outdated Show resolved Hide resolved
@@ -833,7 +617,7 @@ static RDebugPid *build_debug_pid(PROCESSENTRY32 *pe) {
return ret;
}

RList *w32_pids (int pid, RList *list) {
RList *w32_pids(int pid, RList *list) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why get a list to return it? just create it inside this funtion and remove the arg

Copy link
Collaborator

Choose a reason for hiding this comment

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

show_all_pids should be bool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why get a list to return it? just create it inside this funtion and remove the arg

I checked and the equivalent functions for the other platforms also ask for a list. I wont change them because it is out of scope for this pr

@@ -27,6 +27,45 @@ static char *getexe(const char *str) {
return argv0;
}

R_API ut32 r_sys_get_winver() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

what about making this funcction generic and make it work with uname on unix systems too instead of being windows-specific

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@radare I implemented r_sys_get_osinfo(). Can you check to see if thats what you wanted?

libr/debug/p/debug_native.c Outdated Show resolved Hide resolved
libr/debug/p/native/w32.h Outdated Show resolved Hide resolved
@XVilka XVilka added Windows Microsoft Windows platform support issues heap Parsing memory heap structures labels Jun 5, 2019
@radare
Copy link
Collaborator

radare commented Jun 5, 2019

Cool thanks! Lets wait for travis :)

@radare radare merged commit 7a10af4 into radareorg:master Jun 5, 2019
@GustavoLCR GustavoLCR deleted the windows-heap branch June 5, 2019 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
heap Parsing memory heap structures Windows Microsoft Windows platform support issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants