-
Notifications
You must be signed in to change notification settings - Fork 236
implement rust plugin command #6583
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: dev
Are you sure you want to change the base?
Conversation
rust/src/plugin_command.rs
Outdated
}, | ||
BNGetValidPluginCommandsForLowLevelILFunction, BNRegisterPluginCommandForLowLevelILFunction, LowLevelILFunctionPluginCommand, CustomLowLevelILFunctionPluginCommand { | ||
lowLevelILFunctionCommand::lowLevelILFunctionIsValid( | ||
// TODO I don't know what Generics should be used here |
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.
RegularLowLevelILFunction
Need to document this more... but generally use that unless you are emitting LLIL or need SSA version
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.
RegularLowLevelILFunction<CoreArchitecture>
with BinaryView::default_arch
?
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.
Looks like the LLIL function in rust needs to stop trying to track the arch at construction, i.e. do what C++ does
binaryninja-api/lowlevelil.cpp
Lines 55 to 61 in 12f1e2e
Ref<Architecture> LowLevelILFunction::GetArchitecture() const | |
{ | |
Ref<Function> func = GetFunction(); | |
if (!func) | |
return nullptr; | |
return func->GetArchitecture(); | |
} |
IDK the implication of this, but otherwise we need to manually retrieve the underlying function object to get the architecture.
You might have ran into this with your PR #5318
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.
This is likely so that we get the generic A in the arch
signature here:
binaryninja-api/rust/src/low_level_il/function.rs
Lines 97 to 99 in f77dbce
pub(crate) fn arch(&self) -> &A { | |
self.arch_handle.borrow() | |
} |
So we don't have another layer of indirection though the core to get at our architecture from LLIL.
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.
Calling the default_arch obviously is not the correct move here, but you can leave it for now and add a TODO, the underlying issue is we need a way to construct the LLIL function with the function objects underlying core architecture, this is easy to "solve" with a new
function that calls
binaryninja-api/rust/src/low_level_il/function.rs
Lines 153 to 158 in f77dbce
pub fn function(&self) -> Ref<Function> { | |
unsafe { | |
let func = BNGetLowLevelILOwnerFunction(self.handle); | |
Function::ref_from_raw(func) | |
} | |
} |
And then sets the arch handle for the LLIL function:
pub(crate) arch_handle: A::Handle, |
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.
I think the right answer is a new new
function that will lookup the CoreArchitecture
. I think keeping that generic around is the right move.
EDIT: Didn't see the commit right below, ignore this message lol
No description provided.