Inconsistent allocator usage causing crashes

[p0358]

Description

There's sentry_malloc and sentry_free functions. It is expected that they're the only ones used for memory allocation and free operations.
However, for example in this code:

sentry-native/src/backends/sentry_backend_crashpad.cpp

Lines 85 to 92 in 757a7b8

	char *mpack = sentry_value_to_msgpack(event, &mpack_size);
	sentry_value_decref(event);
	if (!mpack) {
	return;
	}
	int rv = sentry__path_write_buffer(data->event_path, mpack, mpack_size);
	sentry_free(mpack);

the usage is inconsistent. When sentry_value_to_msgpack is called, it calls mpack_writer_init_growable, that uses MPACK_MALLOC, which is defined as:

sentry-native/vendor/mpack.h

Lines 204 to 206 in 757a7b8

	#define MPACK_MALLOC malloc
	#define MPACK_REALLOC realloc
	#define MPACK_FREE free

So the pointer is allocated with MPACK_MALLOC aka malloc, but getting freed with sentry_free.

When does the problem happen

• During build
• During run-time
• When capturing a hard crash

Environment

• OS: [e.g. Windows 10, 64-bit] Windows 10, 64-bit
• Compiler: [e.g. MSVC 19] MSVC 19.31.31104.0
• CMake version and config: 3.22.22022201-MSVC_2, -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_SHARED_LIBS=OFF -DSENTRY_LIBRARY_TYPE=STATIC
  -- SENTRY_TRANSPORT=winhttp
  -- SENTRY_BACKEND=crashpad
  -- SENTRY_LIBRARY_TYPE=SHARED

Steps To Reproduce

See #606 (comment)
What I did was commenting out these functions and then implementing them in a different library, that in turn getting successfully linked.
But on runtime, when sentry_free is called in

sentry-native/src/backends/sentry_backend_crashpad.cpp

Line 92 in 757a7b8

sentry_free(mpack);

with a custom free function on a pointer it doesn't recognize, it fails.
I cannot paste the exact code, as the allocator is specific to the game engine and I'm not sure if the behavior would be the same with all allocators.

Proposed fix

It seems that replacing these macros with sentry_ functions does fix my crash.
Inserting the changes above the only import of vendor/mpack.h I could find did not help, so I simply just did those somewhere at the top of my vendor/mpack.h:

#define MPACK_MALLOC sentry_malloc
#define MPACK_REALLOC sentry_realloc
#define MPACK_FREE sentry_free

Note that sentry_realloc didn't exist and I created it in my copy as well.
But creating sentry_realloc and defining MPACK_REALLOC isn't really needed, since the mpack header will just create its own realloc if it's missing, so you could do just fine skipping that one.

[Swatinem]

Thanks for reaching out, I think using the defines to redirect to our own malloc/free is a good idea indeed.