Closed WorksButNotTested closed 1 year ago
This is a lot of changes, and adds a lot of maintenance burden. Can you help me understand why no_std is necessary?
Hi. Thanks for the feedback. Most of the changes relate to changes to the “use” blocks at the top of files. Some of them were from tidying up (there were instances where some use statements were grouped into a single block, but others not). I don’t think there were many actual code changes.
The rust std library is quite large and in turn has a dependency on large parts of libc. Many parts of it are duplicated in the core libraries. Since Frida-rust doesn’t need the features of the standard library it unnecessarily imposes this dependency on downstream consumers.
Although I accept this won’t be a problem for many conventional rust applications, it seems sensible in any case to minimise the complexity of code injected into a target process, to both minimise code size and minimise interaction between Frida and the target.
In my case, my application requires stringent control of the address space which isn’t possible when using rusts standard library.
Perhaps the changes could be smaller and simpler if the dependency on std could be dropped all together rather than it being an optional feature?
Thanks for the explanation. It is clearer to me now.
Perhaps the changes could be smaller and simpler if the dependency on std could be dropped all together rather than it being an optional feature?
I think this is completely reasonable, actually. Instead of having an std
feature though, we can just put module map behind a feature (perhaps module_map
which internally uses std
like you mentioned)
However, my reservations don't quite stop here. I would also really like to avoid adding dependencies as part of this migration. Namely, I don't understand the purpose of no-std-compat
(perhaps we don't need it if we move to dropping std
altogether). And under no circumstances do I want forks of crates, e.g. thiserror-no-std
as a dependency. If we have to drop thiserror
, so be it.
I hope this is reasonable. I am open to feedback, though I can see the value of this feature now (and this feature, as a default).
The only issue with the ModuleMap is the dependency on Path
(to compare the filename of a path). Which isn't part of the core library. So another option could be just using string processing to do that job, but not sure whether that would affect non-unix platforms and I had no means to test and didn't want to introduce a regression.
no-std-compat
is intended to reduce the number of changes to uses
statements required to handle the switch to using types within the core libraries rather than in the std
library. If we were to migrate wholesale to nostd
then this dependency would go away. I included thiserror-no-std
to minimize the changes required to support both std
and nostd
configuations with minimal changes. It seems like it isn't used to extensively though, so it should be possible to come up with an alternative approach.
Do note that I have only considered moving frida-gum
and not frida-core
to nostd
. I've not really though about the merits of the latter since it isn't affecting what I'm trying to build. Also there is a problem I have come across (https://github.com/rust-lang/rust/issues/107845) where cargo will attempt to link against the std
library even for a nostd
crate if one of its dependencies happens to depend on std
. We should also move to version 2
of the resolver (as described in the linked issue too).
All sounds perfectly reasonable. I'm happy to go away and re-work things, just let me know what is the best direction to follow.
The only issue with the ModuleMap is the dependency on Path
For now, I think we should just feature gate it. Then we can come back later.
If we were to migrate wholesale to nostd then this dependency would go away
This makes sense, we can drop it then because everything will be core after your changes (minus a few oddities like ModuleMap). Same goes for thiserror
- it should be possible to drop it, we can just manually perform the derive
s ourselves. If you have any issues with this one let me know and I can try to help.
Do note that I have only considered moving frida-gum and not frida-core to nostd
Yup, makes sense. I don't think there is a good reason for moving frida-core to nostd, it is meant to run in hosted environments.
Are you happy if I just feature gate the parts of module map that deal with paths? It would be very useful to keep the parts which don’t in the nostd build.
Sure, that works for me.
Great. Leave it with me for a bit.
Superseded by https://github.com/frida/frida-rust/pull/85
Changes to support building
frida-rust
asno_std
. A couple of APIs are excluded from themodule_map
when built with thenostd
feature, because of their dependency onsdt::path::Path
which I couldn't find ano_std
replacement for. But it should affect the majority of use cases.