New Data Structure for Source Lines to replace SDB #880

thestr4ng3r · 2021-03-20T17:46:16Z

DO NOT SQUASH

Overview:

Both RzBinSourceRow and sdb_addrinfo are removed. Instead:
Added a new data structure RzBinSourceLineInfo for holding a large amount of addr+line+column+filename samples in a memory-efficient way with O(log n) access by address.
An RzBinSourceLineInfo can be conveniently constructed using the RzBinSourceLineInfoBuilder by pushing samples in an arbitrary order into it and the builder will take care of creating a valid RzBinSourceLineInfo in a final step.
Such an RzBinSourceLineInfo object is part of RzBinObject next to symbols, sections, etc. and can be in a bin plugin's lines function, as it is done for dex and CoreSymbolication.
CL which wasn't really using meta but just messing around with the sdb_addrinfo has been removed.

ix has been added (iX removed and integrated here):

Usage: ix   Display source file line info (from debug info)
| ix[j]   List all source line info available (this was CL before)
| ix.[j]  Show source line info for current address (this was CL. before)
| ixf[j]  Show summary of all source files used (this was iX before)

For #676, this reduces the load time for me from ~1m to ~45s.
Memory usage (from massif) after loading this binary:
Before:

After:

So we get ~180MB vs. ~540MB. Getting lower than this is probably impossible while maintaining efficient access.

XVilka · 2021-03-22T05:20:16Z

Would be nice to reuse it also for PDB too.

thestr4ng3r · 2021-03-22T09:56:23Z

Yes, it should be used for everything eventually. But actually I am not even sure if pdb right now supports line info at all.

librz/bin/dwarf.c

librz/bin/bfile.c

XVilka

I went through and it's a good quality code! 👍
While I think the API should be more abstracted from the fact that we use DWARF, PDB, or dlSYM, it's already a huge improvement and we can merge it right away, imho. Further improvements could be done as a separate issues/PRs

XVilka · 2021-03-26T04:31:39Z

I tried this branch on rizin itself build with debug information (rizin $(which rizin) then aaa; s main; Vp):

These rizin.c34rizin.c strings do not look very good. Could you please address this problem too?

thestr4ng3r · 2021-03-26T08:52:15Z

These rizin.c34rizin.c strings do not look very good. Could you please address this problem too?

Yes. The main problem here is that we don't have any tests for this (yet) as it depends on your local filesystem contents.

thestr4ng3r · 2021-03-26T09:00:42Z

While I think the API should be more abstracted from the fact that we use DWARF, PDB, or dlSYM,...

Added detailed info about going more format-agnostic here: #907 (comment)

librz/bin/dbginfo.c

thestr4ng3r · 2021-03-26T09:48:53Z

line - An unsigned integer indicating a source line number. Lines are numbered beginning at 1. The compiler may emit the value 0 in cases where an instruction cannot be attributed to any source line.

This isn't handled nicely yet, I have to fix it.

thestr4ng3r · 2021-03-26T10:51:49Z

All fixed, including the ugly rizin.c34rizin.c comments.

XVilka · 2021-03-26T11:48:15Z

Two failing tests:



[XX] db/formats/mach0/coresymbolication Xcode symbols cache new (line numbers)
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'ix
?e --
ix.@`is~viewDidLoad[2]`
ix.@0x100006430
' bins/mach0/D9A37B67-F10A-35AA-A852-ABFBBECC03AE-new.symbols
-- stdout
@@ -5,7 +5,6 @@
 0x100006404	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	27
 0x100006420	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
 0x100006430	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
-0x100006440	-	-
 0x100006440	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	-
 0x100006444	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
 0x100006454	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
@@ -30,6 +29,7 @@
 0x100006528	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.m	51
 0x10000652c	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	13
 0x10000653c	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	13
+0x100006540	-	-
 0x100006540	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	-
 0x100006550	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.m	15
 0x100006564	-	-




[XX] db/formats/mach0/coresymbolication Xcode symbols cache (line numbers)
RZ_NOPLUGINS=1 rizin -escr.utf8=0 -escr.color=0 -escr.interactive=0 -N -Qc 'ix
?e --
ix.@`is~viewDidLoad[2]`
ix.@0x100006430
' bins/mach0/D9A37B67-F10A-35AA-A852-ABFBBECC03AE.symbols
-- stdout
@@ -5,7 +5,6 @@
 0x100006404	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	27
 0x100006420	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
 0x100006430	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
-0x100006440	-	-
 0x100006440	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	-
 0x100006444	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
 0x100006454	/Users/ftamagni/src/TestRTTI/TestRTTI/AppDelegate.m	30
@@ -30,6 +29,7 @@
 0x100006528	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.m	55
 0x10000652c	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	13
 0x10000653c	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	13
+0x100006540	-	-
 0x100006540	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.h	-
 0x100006550	/Users/ftamagni/src/TestRTTI/TestRTTI/SceneDelegate.m	15
 0x100006564	-	-

thestr4ng3r · 2021-03-26T12:41:12Z

Two failing tests:

Fixed and updated unit tests to catch this.

ret2libc

Couldn't review everything :P I hope it won't break anything. I couldn't see the code coverage for this branch on codecov though. It would help to see which part are not covered at all by tests.

ret2libc · 2021-03-29T07:21:20Z

librz/include/rz_bin_dwarf.h

@@ -939,14 +947,18 @@ RZ_API void rz_bin_dwarf_loc_free(HtUP /*<offset, RzBinDwarfLocList*>*/ *loc_tab
 RZ_API void rz_bin_dwarf_debug_info_free(RzBinDwarfDebugInfo *inf);
 RZ_API void rz_bin_dwarf_debug_abbrev_free(RzBinDwarfDebugAbbrev *da);

-RZ_API RzList *rz_bin_dwarf_parse_line(RzBinFile *binfile, RZ_NULLABLE RzBinDwarfDebugInfo *info, RzBinDwarfLineInfoMask mask);
-RZ_API char *rz_bin_dwarf_line_header_get_full_file_path(RZ_NULLABLE RzBinDwarfDebugInfo *info, const RzBinDwarfLineHeader *header, ut64 file_index);
+typedef char **RzBinDwarfLineFileCache;


what does this represent? A small doxygen-doc to explain it?

Lets merge this and test a bit before the release, not to be untested. I think @thestr4ng3r could add a comment in a separate commit.

github-actions bot added the rz-test label Mar 20, 2021

thestr4ng3r force-pushed the refactor-debug-lines branch 3 times, most recently from 02d9f08 to ecd8f53 Compare March 21, 2021 20:22

XVilka added the DWARF label Mar 22, 2021

thestr4ng3r force-pushed the refactor-debug-lines branch 5 times, most recently from 8cbc4f0 to f4b2abd Compare March 23, 2021 21:23

XVilka reviewed Mar 24, 2021

View reviewed changes

librz/bin/dwarf.c Show resolved Hide resolved

thestr4ng3r force-pushed the refactor-debug-lines branch from f4b2abd to 9d3e28c Compare March 24, 2021 21:02

github-actions bot added command RzBin labels Mar 24, 2021

thestr4ng3r force-pushed the refactor-debug-lines branch 4 times, most recently from 3f0221e to 132dd38 Compare March 25, 2021 19:26

thestr4ng3r marked this pull request as ready for review March 25, 2021 20:18

thestr4ng3r requested review from kazarmy, ret2libc and wargio as code owners March 25, 2021 20:18

thestr4ng3r force-pushed the refactor-debug-lines branch 2 times, most recently from 0431cd7 to d7cbb2d Compare March 25, 2021 20:45

wargio reviewed Mar 25, 2021

View reviewed changes

librz/bin/bfile.c Outdated Show resolved Hide resolved

XVilka added this to the 0.2.0 milestone Mar 26, 2021

XVilka approved these changes Mar 26, 2021

View reviewed changes

XVilka mentioned this pull request Mar 26, 2021

Unify code of source information access for DWARF, PDB, dSYM #907

Closed

ret2libc reviewed Mar 26, 2021

View reviewed changes

librz/bin/dbginfo.c Outdated Show resolved Hide resolved

thestr4ng3r force-pushed the refactor-debug-lines branch from d7cbb2d to 1d40ebd Compare March 26, 2021 09:48

thestr4ng3r force-pushed the refactor-debug-lines branch from 1d40ebd to 9ac0ca0 Compare March 26, 2021 10:51

thestr4ng3r added 5 commits March 26, 2021 13:36

Add new Line Data Structure

46fb5cd

Make Dwarf emit RzBinSourceLineInfo

1b3869e

Refactor Rest to use RzBinSourceLineInfo

59b196d

Merge CL and iX into ix

7eb1a2b

Split bin_dwarf in Setting and Printing

f6b0f04

thestr4ng3r force-pushed the refactor-debug-lines branch from 9ac0ca0 to f6b0f04 Compare March 26, 2021 12:39

XVilka approved these changes Mar 27, 2021

View reviewed changes

thestr4ng3r requested a review from ret2libc March 27, 2021 09:29

ret2libc approved these changes Mar 29, 2021

View reviewed changes

XVilka merged commit 63891a1 into dev Mar 30, 2021

XVilka deleted the refactor-debug-lines branch March 30, 2021 05:07

This was referenced Mar 30, 2021

Add some docs for RzBinDwarfLineFileCache #936

Merged

Separate Relocs Setting and Printing #938

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Data Structure for Source Lines to replace SDB #880

New Data Structure for Source Lines to replace SDB #880

thestr4ng3r commented Mar 20, 2021 •

edited

Loading

XVilka commented Mar 22, 2021

thestr4ng3r commented Mar 22, 2021 •

edited

Loading

XVilka left a comment

XVilka commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

XVilka commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

ret2libc left a comment

ret2libc Mar 29, 2021

XVilka Mar 30, 2021

New Data Structure for Source Lines to replace SDB #880

New Data Structure for Source Lines to replace SDB #880

Conversation

thestr4ng3r commented Mar 20, 2021 • edited Loading

XVilka commented Mar 22, 2021

thestr4ng3r commented Mar 22, 2021 • edited Loading

XVilka left a comment

Choose a reason for hiding this comment

XVilka commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

XVilka commented Mar 26, 2021

thestr4ng3r commented Mar 26, 2021

ret2libc left a comment

Choose a reason for hiding this comment

ret2libc Mar 29, 2021

Choose a reason for hiding this comment

XVilka Mar 30, 2021

Choose a reason for hiding this comment

thestr4ng3r commented Mar 20, 2021 •

edited

Loading

thestr4ng3r commented Mar 22, 2021 •

edited

Loading