[WIP] Rewrite PDB parser #1498

Basstorm · 2021-08-21T14:05:50Z

Your checklist for this pull request

I've read the guidelines for contributing to this repository
I made sure to follow the project's coding style
I've added tests that prove my fix is effective or that my feature works (if possible)
I've updated the documentation and the rizin book with the relevant information (if needed)

Detailed description

Undone:
1: Fix bugs in symbols parsing(done)
2: update test and unit test(cmd test done)
3: switch to new rz_buf_xxx API(done)
4: update doxygen comments(done)
5: use bit mask when parsing bitfields(done)

Test plan

...

Closing issues

...

XVilka

I thought the API for rz_buf_ access has been changed by @08a to separate the return value from the error.

librz/analysis/type_pdb.c

librz/bin/pdb/pdb.c

librz/bin/pdb/pdb.h

librz/bin/pdb/stream_pe.c

librz/bin/pdb/tpi.c

librz/bin/pdb/tpi.h

librz/type/base.c

librz/bin/pdb/pdb.c

librz/bin/pdb/tpi.c

XVilka · 2021-08-23T09:41:12Z

In fact, I can see there is rz_core_pdb_info() in librz/core/cbin.c. I suggest to create new file called librz/core/cpdb.c and put that function here, also rename it to rz_core_bin_pdb_info_print() and put your RZ_API rz_core_bin_pdb_*() functions as well. Like it's done to DWARF - librz/core/cdwarf.c.

librz/bin/pdb/pdb.c

librz/bin/pdb/tpi.c

librz/analysis/type_pdb.c

librz/bin/pdb/pdb.c

librz/core/cpdb.c

XVilka

Also seems too many tests marked as BROKEN. Why?

XVilka · 2021-08-24T09:23:39Z

librz/bin/pdb/pdb.c

+		ut32 blocks = count_blocks(stream_size, pdb->super_block->block_size);
+		total_blocks += blocks;
+	}
+	//				NumStreams 						StreamsSizes 				StreamsBlockMap


What does this comment mean?

Indicate the meaning of each sizeof(ut32)

XVilka · 2021-08-24T09:24:11Z

librz/bin/pdb/pdb.c


+/**
+ * \brief Parse pdb from the buffer


pdb -> PDB

XVilka · 2021-08-24T09:25:29Z

librz/bin/pdb/tpi.c

 		return lf_enum ? lf_enum->substructs : NULL;
 	}
 	default:
 		return NULL;
 	}
 }

-static char *get_type_name(void *type) {
+/**
+ * \brief Get the name of the givin type


Get the name of the type

XVilka · 2021-08-24T09:25:52Z

librz/bin/pdb/tpi.c

 		return lf_union->name.name;
 	}
 	default:
 		return NULL;
 	}
 }

-static ut64 get_type_val(void *type) {
+/**
+ * \brief Get the numric inside the givin type


Get the numeric value inside the type

XVilka · 2021-08-24T09:28:19Z

librz/core/cpdb.c

+ * \param mode Output Mode
+ * \return bool
+ */
+RZ_API bool rz_core_pdb_info(RzCore *core, const char *file, RzOutputMode mode) {


Rename it into rz_core_pdb_info_print()

Basstorm · 2021-08-24T09:44:37Z

Also seems too many tests marked as BROKEN. Why?

They are flags(were added by rz_cons_printf("f xx=0x1234"), I removed it.) or classes(We don't support yet)

wargio · 2021-08-24T09:53:42Z

Also seems too many tests marked as BROKEN. Why?

They are flags(were added by rz_cons_printf("f xx=0x1234"), I removed it.) or classes(We don't support yet)

i think this is fine, as long as the C apis does the same (actually much better if is via C)

ret2libc

Also seems too many tests marked as BROKEN. Why?

They are flags(were added by rz_cons_printf("f xx=0x1234"), I removed it.) or classes(We don't support yet)

i think this is fine, as long as the C apis does the same

If they don't show up with fi @ 0xXXXXX it means the C API does not do the same, thus I think it is not fine to completely remove them. The purpose of loading PDB is to get more information about the binary, and for example knowing where the main function is is a valuable info that we lose if we don't put a flag in the right address.

ret2libc · 2021-08-24T07:17:30Z

librz/include/rz_analysis.h

@@ -24,6 +24,7 @@
 #include <rz_type.h>
 #include <rz_arch.h>
 #include <rz_cmd.h>
+#include "../bin/pdb/pdb.h"


Why this??? I think it should not be done.

We have RZ_API void rz_parse_pdb_types(const RzTypeDB *typedb, const RzPdb *pdb); in this module.

ret2libc · 2021-08-24T07:18:35Z

librz/include/rz_pdb.h

@@ -12,25 +12,6 @@ extern "C" {

 struct RZ_PDB7_ROOT_STREAM;

-typedef struct rz_pdb_t {


is this file even necessary now? it seems there's not much inside..

Ah, I forgot this file..

XVilka · 2021-08-24T09:57:46Z

I agree that we should keep this f ... output. Even if we might remove it in the future, it's crucial for testing and backward compatibility for now.

Basstorm · 2021-08-24T11:06:58Z

Move to #1517

github-actions bot added rz-test RzBin labels Aug 21, 2021

XVilka requested changes Aug 21, 2021

View reviewed changes

Basstorm force-pushed the pdb branch from fc67346 to 3a04fa9 Compare August 22, 2021 15:41

XVilka reviewed Aug 23, 2021

View reviewed changes

librz/type/base.c Outdated Show resolved Hide resolved

Basstorm force-pushed the pdb branch from 3a04fa9 to b79f900 Compare August 23, 2021 07:58

XVilka requested changes Aug 23, 2021

View reviewed changes

wargio changed the title ~~[WIP] Totally rewrite PDB parser~~ [WIP] Rewrite PDB parser Aug 23, 2021

wargio reviewed Aug 23, 2021

View reviewed changes

librz/analysis/type_pdb.c Show resolved Hide resolved

Basstorm force-pushed the pdb branch from d8db3a7 to 54e5eb0 Compare August 23, 2021 12:02

ret2libc added this to the 0.3.0 milestone Aug 23, 2021

Basstorm force-pushed the pdb branch from 54e5eb0 to 326bbba Compare August 23, 2021 16:48

github-actions bot added the command label Aug 23, 2021

Basstorm added 2 commits August 24, 2021 11:07

Delete useless main.c

9fed05a

Totally rewrite PDB parser

5ea5c2c

XVilka requested changes Aug 24, 2021

View reviewed changes

Basstorm added 12 commits August 24, 2021 16:03

Use new rz_buf_read API

bc9457a

Fix build warnings

75a0857

We must free after parsing

172be42

Use bitmask to operate bitfield

a89f49a

Doxygen

781450a

Check for NULL

36e256f

Fix information loss on S390X

fdd73f4

Fix indentation && check for NULL

a20bc10

separate

afab30c

Revert log in base.c and use puts instead of rz_cons_print

ac873bc

Fix potential segfault

459c8d3

Do not print in idp

e83acec

Basstorm added 3 commits August 24, 2021 16:03

Fix potential overflow

2b54fed

Update test

46aa884

Fix leak and format of json output

3bfd163

Basstorm force-pushed the pdb branch from 326bbba to 3bfd163 Compare August 24, 2021 08:59

XVilka requested changes Aug 24, 2021

View reviewed changes

ret2libc reviewed Aug 24, 2021

View reviewed changes

Basstorm closed this Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Rewrite PDB parser #1498

[WIP] Rewrite PDB parser #1498

Basstorm commented Aug 21, 2021 •

edited

Loading

XVilka left a comment

XVilka commented Aug 23, 2021 •

edited

Loading

XVilka left a comment

XVilka Aug 24, 2021

Basstorm Aug 24, 2021

XVilka Aug 24, 2021

XVilka Aug 24, 2021

XVilka Aug 24, 2021

XVilka Aug 24, 2021

Basstorm commented Aug 24, 2021

wargio commented Aug 24, 2021 •

edited

Loading

ret2libc left a comment

ret2libc Aug 24, 2021

Basstorm Aug 24, 2021

ret2libc Aug 24, 2021

Basstorm Aug 24, 2021

XVilka commented Aug 24, 2021 •

edited

Loading

Basstorm commented Aug 24, 2021

		@@ -12,25 +12,6 @@ extern "C" {

		struct RZ_PDB7_ROOT_STREAM;

		typedef struct rz_pdb_t {

[WIP] Rewrite PDB parser #1498

[WIP] Rewrite PDB parser #1498

Conversation

Basstorm commented Aug 21, 2021 • edited Loading

XVilka left a comment

Choose a reason for hiding this comment

XVilka commented Aug 23, 2021 • edited Loading

XVilka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Basstorm commented Aug 24, 2021

wargio commented Aug 24, 2021 • edited Loading

ret2libc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XVilka commented Aug 24, 2021 • edited Loading

Basstorm commented Aug 24, 2021

Basstorm commented Aug 21, 2021 •

edited

Loading

XVilka commented Aug 23, 2021 •

edited

Loading

wargio commented Aug 24, 2021 •

edited

Loading

XVilka commented Aug 24, 2021 •

edited

Loading