On Mon, Oct 21, 2019 at 03:23:01AM -0600, Anthony J. Bentley wrote:
> Hi,
>
> higan includes a cooperative multithreading library, libco, that swaps
> contexts causing the emulator to crash with MAP_STACK violations.
>
> Replacing malloc() with mmap(...MAP_STACK...) is enough to eliminate
> crashes. But this also means replacing free() with munmap(). Unlike
> free(), munmap() requires the allocated size. However, higan doesn't
> keep track of the size, and thus it can't be passed to munmap(). I
> don't think developing and maintaining local patches to keep track of
> size is practical.
>
> The end result is that the below diff prevents the MAP_STACK crash, but
> also leaks mmap()ed memory.
>
> Without this, higan is unusable. Should I just commit it anyway?
> Any other ideas?
It looks like all relevant free() calls are wrapped by a function like this:
void co_delete(cothread_t t) {
free(t);
}
Storing the allocated size in a new field of cothread_t is not an option?
E.g. the free parts would then look like this, assuming a new 'size' field
added to that struct and initialized after allocation:
void co_delete(cothread_t t) {
size_t size = t->size,
munmap(t, size);
}
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/emulators/higan/Makefile,v
> retrieving revision 1.6
> diff -u -p -r1.6 Makefile
> --- Makefile 12 Jul 2019 20:46:09 -0000 1.6
> +++ Makefile 21 Oct 2019 09:19:23 -0000
> @@ -5,7 +5,7 @@ COMMENT = multi-system Nintendo emulator
> V = 106
> DISTNAME = higan_v$V-source
> PKGNAME = higan-$V
> -REVISION = 3
> +REVISION = 4
>
> USE_WXNEEDED = Yes
>
> Index: patches/patch-libco_amd64_c
> ===================================================================
> RCS file: patches/patch-libco_amd64_c
> diff -N patches/patch-libco_amd64_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libco_amd64_c 21 Oct 2019 09:19:23 -0000
> @@ -0,0 +1,26 @@
> +$OpenBSD$
> +
> +Index: libco/amd64.c
> +--- libco/amd64.c.orig
> ++++ libco/amd64.c
> +@@ -130,7 +130,10 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + size += 512; /* allocate additional space for storage */
> + size &= ~15; /* align stack to 16-byte boundary */
> +
> +- if(handle = (cothread_t)malloc(size)) {
> ++ handle = (cothread_t)mmap(0, size, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0);
> ++ if(handle == MAP_FAILED) {
> ++ err(1, "libco mmap failed");
> ++ } else {
> + long long *p = (long long*)((char*)handle + size); /* seek to top of
> stack */
> + *--p = (long long)crash; /* crash if
> entrypoint returns */
> + *--p = (long long)entrypoint; /* start of
> function */
> +@@ -141,7 +144,7 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + }
> +
> + void co_delete(cothread_t handle) {
> +- free(handle);
> ++ //free(handle);
> + }
> +
> + void co_switch(cothread_t handle) {
> Index: patches/patch-libco_arm_c
> ===================================================================
> RCS file: patches/patch-libco_arm_c
> diff -N patches/patch-libco_arm_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libco_arm_c 21 Oct 2019 09:19:23 -0000
> @@ -0,0 +1,26 @@
> +$OpenBSD$
> +
> +Index: libco/arm.c
> +--- libco/arm.c.orig
> ++++ libco/arm.c
> +@@ -50,7 +50,10 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + size += 256;
> + size &= ~15;
> +
> +- if(handle = (unsigned long*)malloc(size)) {
> ++ handle = (unsigned long*)mmap(0, size, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0);
> ++ if(handle == MAP_FAILED) {
> ++ err(1, "libco mmap failed");
> ++ } else {
> + unsigned long* p = (unsigned long*)((unsigned char*)handle + size);
> + handle[8] = (unsigned long)p;
> + handle[9] = (unsigned long)entrypoint;
> +@@ -60,7 +63,7 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + }
> +
> + void co_delete(cothread_t handle) {
> +- free(handle);
> ++ //free(handle);
> + }
> +
> + void co_switch(cothread_t handle) {
> Index: patches/patch-libco_libco_h
> ===================================================================
> RCS file: patches/patch-libco_libco_h
> diff -N patches/patch-libco_libco_h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libco_libco_h 21 Oct 2019 09:19:23 -0000
> @@ -0,0 +1,14 @@
> +$OpenBSD$
> +
> +Index: libco/libco.h
> +--- libco/libco.h.orig
> ++++ libco/libco.h
> +@@ -7,6 +7,8 @@
> + #ifndef LIBCO_H
> + #define LIBCO_H
> +
> ++#include <err.h>
> ++
> + #ifdef __cplusplus
> + extern "C" {
> + #endif
> Index: patches/patch-libco_ppc_c
> ===================================================================
> RCS file: patches/patch-libco_ppc_c
> diff -N patches/patch-libco_ppc_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libco_ppc_c 21 Oct 2019 09:19:23 -0000
> @@ -0,0 +1,26 @@
> +$OpenBSD$
> +
> +Index: libco/ppc.c
> +--- libco/ppc.c.orig
> ++++ libco/ppc.c
> +@@ -261,7 +261,10 @@ static const uint32_t libco_ppc_code[1024] = {
> + static uint32_t* co_create_(unsigned size, uintptr_t entry) {
> + (void)entry;
> +
> +- uint32_t* t = (uint32_t*)malloc(size);
> ++ uint32_t* t = (uint32_t*)mmap(0, size, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0);
> ++ if(t == MAP_FAILED) {
> ++ err(1, "libco mmap failed");
> ++ }
> +
> + #if LIBCO_PPCDESC
> + if(t) {
> +@@ -324,7 +327,7 @@ cothread_t co_create(unsigned int size, void (*entry_)
> + }
> +
> + void co_delete(cothread_t t) {
> +- free(t);
> ++ //free(t);
> + }
> +
> + static void co_init_(void) {
> Index: patches/patch-libco_x86_c
> ===================================================================
> RCS file: patches/patch-libco_x86_c
> diff -N patches/patch-libco_x86_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libco_x86_c 21 Oct 2019 09:19:23 -0000
> @@ -0,0 +1,26 @@
> +$OpenBSD$
> +
> +Index: libco/x86.c
> +--- libco/x86.c.orig
> ++++ libco/x86.c
> +@@ -84,7 +84,10 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + size += 256; /* allocate additional space for storage */
> + size &= ~15; /* align stack to 16-byte boundary */
> +
> +- if(handle = (cothread_t)malloc(size)) {
> ++ handle = (cothread_t)mmap(0, size, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_STACK, -1, 0);
> ++ if(handle == MAP_FAILED) {
> ++ err(1, "libco mmap failed");
> ++ } else {
> + long *p = (long*)((char*)handle + size); /* seek to top of stack */
> + *--p = (long)crash; /* crash if entrypoint
> returns */
> + *--p = (long)entrypoint; /* start of function */
> +@@ -95,7 +98,7 @@ cothread_t co_create(unsigned int size, void (*entrypo
> + }
> +
> + void co_delete(cothread_t handle) {
> +- free(handle);
> ++ //free(handle);
> + }
> +
> + void co_switch(cothread_t handle) {
>
>
