From eeb995347eb41fa8268731b8c280d67dfb584ceb Mon Sep 17 00:00:00 2001 From: Tomasz Kalinowski Date: Mon, 11 Nov 2024 10:52:05 -0500 Subject: [PATCH] mirror struct defs; maybe fix windows issue + other allignment discrepancies --- src/reassign.c | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/reassign.c b/src/reassign.c index c67bbf68..6bd47517 100644 --- a/src/reassign.c +++ b/src/reassign.c @@ -6,6 +6,30 @@ #include #include // for NULL +// Mirror the exact structures of SEXPREC from R internals +struct proxy_sxpinfo_struct { + uint64_t bits; // guaranteed to be 64 bits +}; + +struct proxy_closxp_struct { + struct SEXPREC *formals; + struct SEXPREC *body; + struct SEXPREC *env; +}; + +struct proxy_sexprec { + // Use the same header macro pattern as R + struct proxy_sxpinfo_struct sxpinfo; + struct SEXPREC *attrib; + struct SEXPREC *gengc_next_node, *gengc_prev_node; + union { + struct proxy_closxp_struct closxp; + // We could add other union members if needed + } u; +}; + +typedef struct proxy_sexprec* proxy_sexp; + SEXP covr_reassign_function(SEXP old_fun, SEXP new_fun) { if (TYPEOF(old_fun) != CLOSXP) error("old_fun must be a function"); if (TYPEOF(new_fun) != CLOSXP) error("new_fun must be a function"); @@ -19,20 +43,19 @@ SEXP covr_reassign_function(SEXP old_fun, SEXP new_fun) { // But those functions are now "non-API". So we comply with the letter of the law and // swap the fields manually, making some hard assumptions about the underling memory // layout in the process. See also: "The Cobra Effect" (https://en.wikipedia.org/wiki/Cobra_effect). - - // Offset and size for closure-specific data within SEXPREC - const size_t closure_data_offset = 32; // 8 bytes (sxpinfo) + 24 bytes (3 pointers for attrib, gengc_next_node, gengc_prev_node) - const size_t closure_data_size = 24; // 3 pointers for formals, body, env (3 * 8 bytes) + // Rather than using memcpy() with a hard coded byte offset, + // we mirror the R internals SEXPREC struct defs here, to hopefully match the alignment + // behavior of R (e.g., on windows). // Duplicate attributes is still not "non-API", thankfully. DUPLICATE_ATTRIB(old_fun, new_fun); - // Temporary buffer to hold CLOSXP data (the 3 pointers to formals, body, env) - char temp[closure_data_size]; + proxy_sexp old = (proxy_sexp) old_fun; + proxy_sexp new = (proxy_sexp) new_fun; - memcpy(temp, (char *)old_fun + closure_data_offset, closure_data_size); - memcpy((char *)old_fun + closure_data_offset, (char *)new_fun + closure_data_offset, closure_data_size); - memcpy((char *)new_fun + closure_data_offset, temp, closure_data_size); + struct proxy_closxp_struct tmp = old->u.closxp; + old->u.closxp = new->u.closxp; + new->u.closxp = tmp; return R_NilValue; }