-
Notifications
You must be signed in to change notification settings - Fork 45
Add class reconstruction support. #145
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
base: main
Are you sure you want to change the base?
Conversation
|
@cyberkaida let me know what improvements you want me to make to this rough version. |
0528da1 to
2109fc2
Compare
|
I did some cleanup on how the errors were being handled and stuff, this doesn't include code cleanup just yet, just fixing some of the issues with how it was handling errors. |
2109fc2 to
73ca9e6
Compare
73ca9e6 to
e40efb1
Compare
| } | ||
|
|
||
| Symbol functionSymbol = function.getSymbol(); | ||
| Namespace parentNamespace = functionSymbol.getParentNamespace(); |
Check warning
Code scanning / CodeQL
Dereferenced variable may be null Warning
functionSymbol
this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds class reconstruction support by implementing a new tool provider for RTTI-based class reconstruction and integrating it into the MCP server.
- Introduces ClassToolProvider with multiple tool registrations for listing, creating, and reconstructing classes via RTTI data.
- Updates integration tests with ClassToolProviderIntegrationTest and registers the new provider in the MCP server.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/test.slow/java/reva/tools/classes/ClassToolProviderIntegrationTest.java | Integration tests for the new class reconstruction functionality |
| src/main/java/reva/tools/classes/ClassToolProvider.java | New tool provider implementation with several tools for class operations, including RTTI reconstruction |
| src/main/java/reva/tools/AbstractToolProvider.java | Added a generic helper method to retrieve optional list parameters |
| src/main/java/reva/server/McpServerManager.java | Updated to register the new ClassToolProvider |
| // Execute the RTTI script | ||
| String scriptName = "RecoverClassesFromRTTIScript.java"; | ||
|
|
||
| try { |
Copilot
AI
Jun 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The executeRttiReconstruction method contains deeply nested try-catch blocks. Consider refactoring this logic (e.g., by extracting helper methods) to improve readability and maintainability.
| try { | ||
| // Create the class namespace | ||
| Namespace classNamespace = symbolTable.createClass(parentNamespace, className, ghidra.program.model.symbol.SourceType.USER_DEFINED); | ||
|
|
||
| program.endTransaction(txId, true); | ||
|
|
||
| Map<String, Object> result = createBasicClassInfo(classNamespace, 0); | ||
| result.put("message", "Successfully created class namespace: " + className); | ||
| return createJsonResult(result); | ||
|
|
||
| } catch (Exception e) { | ||
| program.endTransaction(txId, false); | ||
| throw e; |
Copilot
AI
Jun 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using a try-finally block when managing transactions to ensure that program.endTransaction is always executed, even if an exception is thrown.
| try { | |
| // Create the class namespace | |
| Namespace classNamespace = symbolTable.createClass(parentNamespace, className, ghidra.program.model.symbol.SourceType.USER_DEFINED); | |
| program.endTransaction(txId, true); | |
| Map<String, Object> result = createBasicClassInfo(classNamespace, 0); | |
| result.put("message", "Successfully created class namespace: " + className); | |
| return createJsonResult(result); | |
| } catch (Exception e) { | |
| program.endTransaction(txId, false); | |
| throw e; | |
| boolean success = false; | |
| try { | |
| // Create the class namespace | |
| Namespace classNamespace = symbolTable.createClass(parentNamespace, className, ghidra.program.model.symbol.SourceType.USER_DEFINED); | |
| success = true; | |
| Map<String, Object> result = createBasicClassInfo(classNamespace, 0); | |
| result.put("message", "Successfully created class namespace: " + className); | |
| return createJsonResult(result); | |
| } catch (Exception e) { | |
| throw e; | |
| } finally { | |
| program.endTransaction(txId, success); |
e40efb1 to
19b7aaf
Compare
- Use McpSchema.Tool.builder() pattern instead of constructor - Add title fields to all 8 class tools for better UX - Fix address formatting to use AddressUtil.formatAddress() - Update error message to match integration test expectations - Remove unused imports and add missing AddressUtil import This aligns the ClassToolProvider with the new MCP SDK patterns introduced in the main branch rebase.
19b7aaf to
79fb394
Compare
This is a PR for class reconstruction support. It's still a work in progress, but it's working. We need to clean it up probably.