Vector35 / binaryninja-api

Public API, examples, documentation and issues for Binary Ninja
https://binary.ninja/
MIT License
900 stars 203 forks source link

Allow for FlowGraphWidget to not have a backing view or function #4764

Open ek0 opened 10 months ago

ek0 commented 10 months ago

Description Currently, FlowGraph::Show is very limited in terms of cutomization since we don't have access to the underlying QWidget. There is a FlowGraphWidget type available that allow for more control, but it seems it requires to have a backing function. It seems there is tight coupling between the Function type and the FlowGraphWidget rendering. Some checks are performed within the FlowGraphWidget::paintEvent function which disallow custom FlowGraphWidget. More information can be found in this thread.

Is your feature request related to a problem? I am trying to develop a specific plugin to display structured information, but not disassembly. It is required to have more control over the widget for UX reasons. But so far nothing in the current public API seems to work. A simple example demonstrating the issue can be reproduced using this snippet:

void TestFlowGraph(bn::Ref<bn::BinaryView> view) {
  bn::FlowGraph* graph = new bn::FlowGraph();
  auto* node = new bn::FlowGraphNode(graph);

  bn::DisassemblyTextLine line{};
  line.tokens.push_back(bn::InstructionTextToken{InstructionToken, "test"});
  line.addr = 0;

  std::vector<bn::DisassemblyTextLine> lines{};
  lines.push_back(line);

  node->SetLines(lines);
  graph->AddNode(node);
  //graph->Show("asdf"); // This works

  auto view_context = UIContext::activeContext();

  FlowGraphWidget* widget = new FlowGraphWidget{nullptr, view, graph};
  //CustomFlowGraphWidget* widget =
  //    new CustomFlowGraphWidget{nullptr, view, graph};
  widget->setGraph(graph);
  //widget->updateToGraph(graph);
  auto* pane = new WidgetPane(widget, "asdf");
  view_context->openPane(pane); // This will display "Loading..."
  // widget->show();
  //  We leak, we don't care for now
}

The current snippet will open a window that displays Loading....

Are any alternative solutions acceptable? A sufficient option would be to have the exact same FlowGraphWidget type with all the functions related checks removed. We could call it CustomFlowGraphWidget.

xusheng6 commented 9 months ago

Are you trying to use the flow graph layout infra in BN as the backbone for a general purpose flow graph? I have definitely thought about doing something similar, but I did not figure out a good way to do it. It would be perfect if we can tweak the code to support it, since otherwise I would have to resort to a third-party graphing library like https://github.com/cneben/QuickQanava

ek0 commented 9 months ago

Hey @xusheng6 !

This is exactly what I am trying to do, but I still aim at displaying languages in the graph, so the FlowGraphWidget would be perfect here. I'm basically trying to load and render user-generated files with a custom IL to render it. The main issue I have is the coupling between the view object, the function object and the widget object that prevents me from extending the UI as much as I would like to.

Looking at the FlowGraphWidget::paintEvent we can see that strong coupling exists, as Function and Analysis related checks are performed. I attempted to subclass it and reimplement everything, but some used members in the API are private, and some implementation details are hidden. Reimplementing everything correctly would require reverse engineering the API and would be time consuming.

Subclassing the FlowGraph and FlowGraphWidget classes and stripping these dependencies to allow using them without a view (or at the very least, without a function) could be enough.

xusheng6 commented 9 months ago

@ek0 this is what I attempted to do. I wish to create something similar to Cyberchef, however, instead of layered stacking of the transformations, I wish to represent the computation in a graph, which is easier to see and edit.

ek0 commented 9 months ago

That's fair, but regarding this issue, I think it'd be good enough to decouple the current API to allow for a better extension. Or at least to open-source the current FlowGraphWidget implementation so the community can take a shot at it. I don't really need editing right now, and having a proper QWidget that is a FlowGraph that has its layout done by the core is good enough.

This is currently a blocker for a plugin I am attempting to wrap-up.

xusheng6 commented 9 months ago

That's fair, but regarding this issue, I think it'd be good enough to decouple the current API to allow for a better extension. Or at least to open-source the current FlowGraphWidget implementation so the community can take a shot at it. I don't really need editing right now, and having a proper QWidget that is a FlowGraph that has its layout done by the core is good enough.

This is currently a blocker for a plugin I am attempting to wrap-up.

thx for the info, I will think about it!