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

Add more graph widgets #2273

Merged
merged 17 commits into from
Jul 16, 2020
Merged

Add more graph widgets #2273

merged 17 commits into from
Jul 16, 2020

Conversation

karliss
Copy link
Member

@karliss karliss commented Jul 2, 2020

Your checklist for this pull request

Detailed description

Introduce generic r2 graph widget. User can select one of the r2 graph types in the list or enter a command. Since it executes more or less arbitrary r2 command it needs to be manually refreshed using a button.

Add a specialized CallGraphWidget used for global and single function callgraph. Extra features in the callgraph widget compared to generic one:

  • works with callgraph, agc can't be used in the generic widget because agcj and agCj doesn't have the same structure as rest of ag*j commands
  • addressable item context menu when clicking on a block
  • seek synchronization
  • show in Callgraph and show in Global Callgraph when in addressable item context menu for other widgets

Expose more graphviz layout engines(sfdp, neato, twopi, circo), might be useful for some of the reference graphs.

GraphGridLayout bugfix for a case with multiple top level blocks. Almost never happens for CFG but is somewhat common in callgraph and some of other r2 graphs

Fix the false dock showing causing RefreshDeferrer to work poorly. This was probably a regression caused by dock layout switching feature.

Code refactoring and other changes:

  • split some of the MemoryDockWidget into AddressableDockWidget
  • Introduce CutterGraphView for some of the common Cutter specific logic like styling, reading config values, and reaction to Config and Core events. GraphView base shouldn't contain any Cutter specific code to make it easier sharing code with x64dbg . Move some of the logic from DisassemblerGraphView to it now that there are other graphs to share it with.
  • new class SimpleTextGraphVew for graphs consisting of plaintext label and optionally address associated with each block

Test plan (required)

Screenshot from 2020-07-10 14-33-23
Screenshot from 2020-07-10 14-33-37
Screenshot from 2020-07-10 14-35-24
show_in_context_menu
Screenshot from 2020-07-10 14-35-44
Screenshot from 2020-07-10 14-35-03

Steps

  • Test show in Callgraph: right click in function widget, select show in Callgraph ✔️
  • Test show in Global Callgraph: right click in function widget, select show in Global Callgraph ✔️
  • Callgraph position synchronization: ✔️
    • Click at different functions in disassembly widget, make sure both callgraphs are updated accordingly ✔️
    • Disable synchronization and click in disassembly widget make sure callgraphs are not updated ✔️
  • Image export do the steps for both callgraphs and at least one graph in the generic r2 graph widget ✔️*
    • Direct png export, check the large bitmap warning using one of global graphs
    • Direct svg export
    • Json export
    • graphviz based image export
    • gml export
  • Copy text ✔️
  • Copy address from callgraph ✔️
  • Show xrefs in callgraph ✔️
  • Change layout in all new graphs *
  • Test all the new graphviz layouts (using medium size or small graph, some of them are slow) ✔️

Closing issues

Closes #2178
Closes #406
Closes #1776

@karliss karliss marked this pull request as ready for review July 10, 2020 11:54
@karliss karliss added this to the 1.11.0 milestone Jul 10, 2020
@karliss
Copy link
Member Author

karliss commented Jul 12, 2020

Problems found during testing:

  • sometimes switching between vertical and horizontal layout in global callgraph view gets stuck above the content.
  • default export names aren't used properly

@karliss
Copy link
Member Author

karliss commented Jul 13, 2020

Both fixed.

@xarkes
Copy link
Member

xarkes commented Jul 15, 2020

Do you have any idea of what's wrong with Travis and Appveyor?

@karliss
Copy link
Member Author

karliss commented Jul 15, 2020

Appveyor should be fixed by meson version PR that was just merged. On Travis the macOS build times out. Recent successful builds take 43-48 minutes which isn't too far from 50 minute limit. The annoying part is that ~20 minutes out of that is taken by homebrew update which we have to do because Travis isn't updating their macOS images. See https://github.com/radareorg/cutter/pull/2077 .

@xarkes
Copy link
Member

xarkes commented Jul 15, 2020

Things noticed while testing:

  • The field name of the dropdown menu of the R2 graphs widget is editable
  • There is no space at all between edges so it might generate funny graphs (see picture attached below)
  • agg seems to be useless (and that might be by design as it is named "custom") but it might be interesting to add more info or a warning or something...

image

Copy link
Member

@xarkes xarkes left a comment

Choose a reason for hiding this comment

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

Nice refactors, it looks good to me :-)

@karliss
Copy link
Member Author

karliss commented Jul 15, 2020

The field name of the dropdown menu of the R2 graphs widget is editable

That's by design you can enter a plain r2 command there. See description of #2178 . Idea is that even when we have dedicated widgets for all graphs a new r2 version might add new graph types. In such situation you can just type the command of new graph without waiting until new widget gets added to Cutter.

There is no space at all between edges so it might generate funny graphs (see picture attached below)

As the amount of connected edges increases space is reduced. In my opinion that's better than keeping fixed spacing and having the ends not connect to block and spanning across multiple screens.

agg seems to be useless (and that might be by design as it is named "custom")

There some use-cases for agg. One is commands like ".axg*" which store the resulting graph in "custom graph". So instead of running axgv and obtaining unreadable graph in console widget you can run ".axg*" and then opening agg in the r2 graph widget. Other potential usescase is for display graphs generated by user scripts. The same could be done by making a Cutter plugin which reuses some of Cutter graph classes, but the r2 command interface is somewhat simpler than Cutter API and such script could be run either in plain r2 or Cutter.

@xarkes
Copy link
Member

xarkes commented Jul 16, 2020

Ok thanks for the details :) LGTM!

@karliss karliss merged commit e5d7bd6 into rizinorg:master Jul 16, 2020
@karliss karliss deleted the r2-graphs branch August 5, 2020 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic r2 graph widget agC Global callgraph, but with cutters graph engine Add Call-Graph
2 participants