bad call to boot_SDL() breaks some platforms

[kernigh]

Code near src/SDL.xs line 147 calls boot_SDL() without the required arguments. This causes undefined behavior in the C compiler, and doesn't work on some platforms. When I tried SDL-2.548 from CPAN with perl 5.30.1 on OpenBSD amd64, perl -e 'require SDL' segfaulted on a NULL pointer.

What's wrong: The bad code is,

void boot_SDL();
void boot_SDL__OpenGL();

XS(boot_SDL_perl)
{

#ifndef NOSIGCATCH 
	signal(SIGSEGV, handler);
#endif
	PL_perl_destruct_level = 2;
	GET_TLS_CONTEXT
	boot_SDL();

#if defined WINDOWS || defined WIN32
  SDL_RegisterApp ("SDLPerl App", 0, GetModuleHandle (NULL));
#endif

}

The correct prototype seems to be void boot_SDL(pTHX_ CV *), so the call boot_SDL(); with no arguments is wrong. The callee boot_SDL() receives undefined values as parameters. It uses these undefined values in an XS handshake with Perl. (The handshake checks that the SDL module and Perl are compatible.) This can only work if we get lucky and the undefined values work with the handshake.

After the macro expansion, the caller boot_SDL_perl() might look roughly like this:

/* threaded perl */
void boot_SDL_perl(PerlInterpreter *my_perl, CV *cv)
{
    my_perl->Iperl_destruct_level = 2;
    parent_perl = pthread_getspecific(*Perl_Gthr_key_ptr(my_perl));
    boot_SDL();
}

/* not-threaded perl */
void boot_SDL_perl(CV *cv)
{
    (PL_curinterp)->Iperl_destruct_level = 2;
    boot_SDL();
}

In threaded perl, GET_TLS_CONTEXT might set parent_perl to some function's return value. This usage of GET_TLS_CONTEXT looks bogus, because the code has no need to parent_perl; but on some platforms, GET_TLS_CONTEXT might fix the handshake by accident. The return value from pthread_getspecific() might be in the same place as the first argument to boot_SDL(). Then the call boot_SDL() might act like boot_SDL(parent_perl, ???). This might work, because the handshake would check parent_perl and ignore the second argument.

In not-threaded perl, pTHX_ and GET_TLS_CONTEXT are nothing, so void boot_SDL(pTHX_ CV *) is just void boot_SDL(CV *). The handshake checks the CV. In OpenBSD amd64, a function's first argument goes in register %rdi (ELF ABI for AMD64, figure 3.4, page 20). If the compiler would preserve %rdi until the call to boot_SDL, then the handshake might work. My compiler, clang 8.0.1, made a tail call where boot_SDL_perl() jumped to boot_SDL(). It needed an undefined CV * for the tail call, and decided to use 0, so I got boot_SDL(NULL). Then the handshake dereferenced the NULL and segfaulted. Because of the tail, boot_SDL_perl() is not in gdb's backtrace.

Possible fix: I changed void boot_SDL(); to void boot_SDL(pTHX_ CV *); and boot_SDL(); to boot_SDL(aTHX_ cv). This worked for me. This seems less than correct, because I am reusing boot_SDL_perl's cv, but I guess that boot_SDL should have its own cv. The handshake seems to accept the wrong cv.

A more correct fix might use the BOOT: keyword described in perlxs, but I have not tried this. One would change the boot_SDL_perl() to BOOT: code, and remove the MODULE = SDL at the end of the file. Then the XS preprocessor would emit boot_SDL_perl(), not boot_SDL(), and the BOOT: would add code to boot_SDL_perl().