shizcow / hotpatch Goto Github PK

Currently, there's several extra layers of indirection on every Patchable function call. In the past several weeks I've been experimenting with ways to remove this, and finally have a lead. I have found the function pointer approach to be more forgiving with reference lifetimes, allowing for method patching to actually work. This issue tracks progress on integration.

• Figure out how to actually do it
• Reconfigure library to handle all incoming Patchable declarations
• Update #[patchable] to generate the correct init code
• Fix and set up importing
• Update #[patch] to interface with new imports
• Ensure backwards compatibility with previous version
• Docs!

I intend to work on implementing methods when I next have time. An implementation draft can be found in #7.

I'm currently concerned with the need to import traits to use hotpatched methods. The reason can be summarized as follows:

struct Foo{}
impl Foo {
  static hotpatched_fn: Patchable;
}
// this trait will be auto-generated by a macro
trait HotpatchFoo {
  fn hotpatched_fn();
}

hotpatch_fn has a dual definition -- AFAIK this is required to get the .hotpatched_fn syntax working while still having a Patchable somewhere in the mix. Problem is that HotpatchFoo trait now needs to be imported.

Ideas around this:

• See if the static can be placed in the trait instead. I don't think this will work but would at least make a trait import only required when hotpatching. EDIT: This works.
• See if generic traits can be specialized with additional associated functions. This way, an auto-generated const-generic parameter could do all the magic, leaving only a single crate-wide import directly from hotpatch::MethodTrait required. Don't think this will work either, but it seems the most promising solution.
• impl<const HOTPATCH_ID: usize> Deref for fn::<HOTPATCH_ID>(&Foo, Args...) -> Ret. I highly doubt you can impl Deref for a method (and I don't know the syntax to get this right), but this MIGHT be possible with specialization (and would be the best way forward imo)

As soon as I find an implementation I'm happy with I'll write a todo list in this issue.

Help Wanted

Ideas on how to get around having to import traits? Inherent traits aren't in nightly yet, and workarounds I know of don't allow duplicate definitions.

This issue is meant to track the status of patching methods, as well as a public idea board. This is sectioned mainly into two achievements: associated functions and methods. NOTE methods will get their own tracking issue; this will be closed when associated functions are done.

TODO items for associated functions:

• Get a real demo working
• Integrate for everything in lib
• Fix with macros
• Module aware expansions for imports
• Test with imports
• Import macros
• Test for backwards compatibility
• Docs!
• Error checking to deny actual methods (temporary)

Associated Functions

Prior to filing this issue, a method of patchable associated functions was discovered. The final implementation syntax is as follows:

struct Foo {}

#[patchable]
impl Foo {
  fn bar() {}
}

fn main() {
  Foo::bar.hotpatch_fn(|| unimplemented!());
}

It is (roughly) implemented internally as follows:

struct Foo {}
static __hotpatch_implgen_Foo_bar: hotpatch::Patchable<(Self, i32), ()> =
    hotpatch::Patchable::__new( ||
        // macro passthrough from the initial implementation
    );
fn __hotpatch_staticwrap_0_bar() -> &'static Patchable<(Self, i32), ()> {
    &__hotpatch_implgen_Foo_bar
}
impl Foo {
    pub const bar: hotpatch::MutConst<Patchable<(Self, i32), ()>> =
        hotpatch::MutConst::new(__hotpatch_staticwrap_0_bar);
     // where MutConst is a const reference to a static variable
     // It's pretty much an associated static with extra steps (they aren't supported)
}

Problem: Because the implementation must be separated from the impl block, this cannot support a self parameter (in any form).

Problem: Self is not available. I have yet to find a workaround, but I suspect one exists using dummy consts.

Methods

Recently I discovered a way of making patchable methods. The main holdup was declaring a way of having some associated static holding a function pointer with a self parameter. And the only way to get a self parameter to work is with a method directly in an impl block.

Or at least, I thought so. Traits perfectly achieve this. Combining this with the fact that there can be multiple struct associations under the same name if they're in a trait and of different types yields a perfect solution. The working syntax is as follows:

struct Foo {}

#[patchable]
impl Foo {
  fn bar(&self) {}
}

fn main() {
  let foo = Foo::new(); // new defined elsewhere
  foo.bar(); // works
  
  // hotpatched as the following:
  Foo::bar.hotpatch_fn(|| unimplemented!());
  
  // Yet the following does NOT work for hotpatching:
  foo.bar.hotpatch_fn(|| unimplemented!());
  // This is GREAT for cleanliness
}

The macro-expanded implementation is still being figured out. However, the following is a working demo of making Foo::bar different from foo.bar:

struct Foo {
    data: i32,
}

impl Foo {
    fn new(data: i32) -> Self {
	Self{data}
    }
}

// begin macro generated content

// macro input:
// #[patchable]
// impl Foo {
//     fn bar(self) {
//         println!("hello from bar! This foo object has data: {}", self.data);
//     }
// }

impl Foo {
    #[allow(non_upper_case_globals)]
    const bar: i32 = 0;
}

trait FnTrait { // final implementation will use mangled names
    fn bar(self);
}

impl FnTrait for Foo {
    fn bar(self) {
	println!("hello from bar! This foo object has data: {}", self.data);
    }
}

// end macro generated content

// test
fn main() {
    let foo = Foo::new(100); // just gives foo some instance data
    foo.bar(); // Has access to self, and the data  -^-----------^
    println!("{:?}", Foo::bar); // Foo::bar is a const i32; totally different!
}

I really like the fact that foo.bar is actually a method, and that foo.bar.hotpatch isn't allowed. Forcing the use of Foo::bar.hotpatch cements the idea that the method is patched for all instances.

Problem: trait associated functions (without self) are not privy to the same non-competition rules. This may mean a macro may need to decide whether to use the associated function implementation or the method implementation. This feels like a rustc bug to me.

Can you see a path towards hotpatching structs as well as functions?

As mentioned in this reddit comment, if a patch implementation from an external library has unsafe code then patchable functions should require unsafe.

As this crate already exports some information alongside functions (module path, signature), I'd like to move in the direction of adding more attributes. unsafe would be one. I have yet to come up with others.

For passers by: suggestions on other attributes are highly appreciated.

Your project looks really interesting and i will definitely experiment with it. What i was wondering was can you patch methods ?

Hi. I'm facing this error on stable 1.50

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> /home/tekjar/.cargo/git/checkouts/hotpatch-6d47e0c925964dfc/702236c/hotpatch_macros/src/lib.rs:1:1
  |
1 | #![feature(proc_macro_diagnostic)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is this library nightly only?

Currently, the only unsafe portion of this crate is that trying to patch a parent function (without the unsafe force) causes a deadlock. Using the backtrace crate it may be possible to detect when this occurs and issues an error. It may be difficult to ensure that the function identities are the same, and to ensure that this does not cause a significant runtime penalty.

I have no idea if this crate works already with webassembly or not, but just in case, it might be a good idea to make it work on webassembly.