geofft / redhook

Dynamic function call interposition / hooking (LD_PRELOAD) for Rust
BSD 2-Clause "Simplified" License
176 stars 18 forks source link

Use mod real_fn instead of impl real_fn (for cdylib) #14

Open galkinvv opened 4 years ago

galkinvv commented 4 years ago

Change real_fn to be defined in mod instead of impl, so it is exported while building as cdylib.

This allows building crates using redhook not only as dylib but as cdylib too. Tested on linux, not on mac (but no mac changes). For my (simple) use case dylib is 3.6MB while cdylib is 2.1MB.

Actually I don't know any doc details describing why impl function is not exported in cdylib, but it's looking natural for me (new to rust with c/c++ experience) that "methods" are not exported when building a library with a C-compatible interface, but free functions are exported.

galkinvv commented 4 years ago

TLDR: I want suggest another change, see changest at last link

I'm new to rust, so I'm not sure that the change is for example compatible with older compiler versions, Ive tested only rust 2018 v1.38

By the way I used redhook for a very corner case - hooking the clock_gettime on linux. It was running fine for most apps, but failed with an obscure crash when running rustc itself.

The problem was very obscure: call to dlsym (to get real function) lead to recursive call clock_gettime. the call sequence of calls was

So the top stack near overflow does NOT show that dlsym was a first recursive call initiator!

I developed a code to abort with a clear message in a case of a recursion during dlsym call instead of this complex crash. Unfortunately it's a bit too hacky to be called "fine-in-production".

https://github.com/galkinvv/redhook/commit/b048100ccd5cc3375494464ab25ab6eabf0aa13d

Should I create separate MR for it to discuss details (maybe adding tests, etc) or it is looking too hacky? (I'm discussing it here instead of a separate merge request since code changes are not auto-mergeable)