On Thu 2022-08-25 10:50:20, Joe Lawrence wrote: > On 7/1/22 3:48 PM, Marcos Paulo de Souza wrote: > > The shadow variable type will be used in klp_shadow_alloc/get/free > > functions instead of id/ctor/dtor parameters. As a result, all callers > > use the same callbacks consistently[*][**]. > > > > The structure will be used in the next patch that will manage the > > lifetime of shadow variables and execute garbage collection automatically. > > > > [*] From the user POV, it might have been easier to pass $id instead > > of pointer to struct klp_shadow_type. > > > > The problem is that each livepatch registers its own struct > > klp_shadow_type and defines its own @ctor/@dtor callbacks. It would > > be unclear what callback should be used. They should be compatible. > > > > This problem is gone when each livepatch explicitly uses its > > own struct klp_shadow_type pointing to its own callbacks. > > > > [**] test_klp_shadow_vars.c uses a custom @dtor to show that it was called. > > The message must be disabled when called via klp_shadow_free_all() > > because the ordering of freed variables is not well defined there. > > It has to be done using another hack after switching to > > klp_shadow_types. > > > > Is the ordering problem new to this patchset? Shadow variables are > still saved in klp_shadow_hash and I think the only change in this patch > is that we need to compare through shadow_type and not id directly. Or > does patch 4/4 change behavior here? Just curious, otherwise this patch > is pretty straightforward. The problem is old. klp_shadow_free_all() uses hash_for_each(). It iterates the hashes sorted by the hash value. The tested arrays are on stack. The address of the stack is different in every run. As a result, the hash is always different and the pointers are sorted in different order. Best Regards, Petr