[R-pkg-devel] Altrep header, MSVC, and STRUCT_SUBTYPES macro

[David Cortes]

Thanks for the hints.

Unfortunately, after some further testing, it seems this was just a
matter of getting lucky - using the alternative non-STRUCT_SUBTYPES 
def. of Altrep still leads to memory corruptions and crashes, just at
different points than when using the STRUCT_SUBTYPES definition. Sorry
for the trouble.

> Interesting ABI incompatibility you've found. Can you show a minimal
> example? I've tried playing with https://godbolt.org/ 

May I ask: how would you go around getting R code into Godbolt? Getting
something like the Altrep definitions and methods would involve a large
amount of R headers joined together before any package code.

> Out of curiousity - which functionalities are those ?

As I understand it (not the author of the package), this is about
integrations with frameworks for GPU computing, and the Altrep code is
used for returning the results of model fitting and serialization
functions, so unfortunately not feasible to move it to a different
package.

> it's also important to find out where exactly the
> incompatibility arises from

I unfortunately haven't been able to isolate the issue, and am not sure
how to. As far as I can tell, the altrep methods are calling the
functions which they were assigned so at least the 'set' and 'dataptr'
methods are working, but memory corruptions that crash the program
happen after calling such altrep methods, particularly when there is a
combination of 'R_UnwindProtect', C++ 'catch' that involves destructing
variables before 'R_ContinueUnwind', and then 'Rf_error'.

On Thu, 2024-05-16 at 16:25 +0300, Ivan Krylov wrote:
> В Wed, 15 May 2024 18:54:37 +0200
> David Cortes <david.cortes.rivera using gmail.com> пишет:
> 
> > The code compiles without errors under MSVC, but executing code
> > that
> > involves returning Altrep objects leads to segfaults and memory
> > corruptions, even though it works fine under other compilers.
> > 
> > I see the R Altrep header has this section:
> > #define STRUCT_SUBTYPES
> > #ifdef STRUCT_SUBTYPES
> > # define R_SEXP(x) (x).ptr
> > # define R_SUBTYPE_INIT(x) { x }
> >   typedef struct { SEXP ptr; } R_altrep_class_t;
> > #else
> > # define R_SEXP(x) ((SEXP) (x))
> > # define R_SUBTYPE_INIT(x) (void *) (x)
> >   typedef struct R_altcls *R_altrep_class_t;
> > #endif
> 
> Interesting ABI incompatibility you've found. Can you show a minimal
> example? I've tried playing with https://godbolt.org/ and passing
> around values of type R_altrep_class_t between functions, but
> couldn't
> convince "x64 msvc v19.latest" to generate different assembly no
> matter
> whether R_altrep_class_t was a pointer or a struct containing a SEXP.
> 
> > If I manually edit the R header to remove the definition of
> > 'STRUCT_SUBTYPES', leading to the second definition of
> > 'R_altrep_class_t' being used, then things work as expected when
> > the
> > package is compiled with MSVC (no segfaults and no memory
> > corruptions).
> 
> While it's hard to argue with results (I don't think it'll ever be
> broken on x86_64 Windows), this workaround relies on undefined
> behaviour and will only work as long as the ABI as understood by GCC
> passes a structure with a pointer inside exactly the same way as the
> ABI as understood by MSVC passes a bare pointer.
> 
> Isolating the MSVC-specific code as suggested by Vladimir should be
> safer, but it's also important to find out where exactly the
> incompatibility arises from. The GCC and MSVC parts still have to use
> a
> common ABI to talk to each other.
>