emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.77k stars 3.3k forks source link

c-string parameter of js->c callback function is corrupt unless literal #16814

Open SoylentGraham opened 2 years ago

SoylentGraham commented 2 years ago

I have some CAPI functions that I'm calling from javascript (Module.ccall) and providing a function-pointer (created via Module.addFunction) for event callbacks in C++.

typedef void OnStreamMetaChanged(const char* StreamMetaJson);
extern "C" int  AllocInstance(const char* RootUrl,Manifest_OnStreamMetaChanged* OnStreamMetaChanged);

This works... with literal strings, ie const char* Hello = "OnStreamMetaChangedCallbackFunc Test";

function OnStreamMetaChangedCallback(JsonPtr)
{
    const Json = Module.UTF8ToString(JsonPtr);
    console.log(`OnStreamMetaChangedCallback -> ${JsonPtr} ${Json}`);
}

OnStreamMetaChangedCallback -> 2421 OnStreamMetaChangedCallbackFunc Test

Allocating the js function

    const CallbackSignature = `vi`; //  i = pointer = UTF8ToString() to get string
    const OnStreamChangedCallbackFunc = Module.addFunction(OnStreamMetaChangedCallback,CallbackSignature);

Usage

        const Options = {async:true};
        const ReturnType = 'number';
        const ArgTypes= ['string','number'];    //  number=pointer
        const ArgValues = [StreamUrl,OnStreamChangedCallbackFunc];
        const Instance = await Module.ccall('AllocManifestInstance',ReturnType,ArgTypes,ArgValues,Options);

If I use std::string std::vector mallloc new[] etc (ie, things that should still exist and not neccessarily on the stack) I get a (presumably later-in-the-heap) pointer but there doesn't seem to be the right data there. OnStreamChangedCallback -> 5406464 ػR OnStreamChangedCallback -> 5437144

SOMETIMES this does seem to work... (whilst malloc() std::string std::vector etc still don't)

        std::string AllocatedStringContent{"Allocated string test"};
        char* AllocatedString = new char[100];
        memset( AllocatedString, 0, 100 );
        AllocatedStringContent.copy( AllocatedString, AllocatedStringContent.length());
        OnStreamMetaChangedFunc(AllocatedString);

OnStreamChangedCallback -> 5401744 Allocated Version string test

I'll try and reduce down to a minimal example, but as I understand it, the callbacks are synchronous, so any stack or variables I'm using should still exist...

Notable flags;

Flags+=" -sALLOW_TABLE_GROWTH"
Flags+=" -O0"
#Flags+=" -sALLOW_MEMORY_GROWTH" # no difference
Flags+=" -std=c++17"

Am I supposed to be doing something C++ side to lock the heap, or correct the string addresses? Is there some alignment required? Are these possibly the wrong addresses? Are the callbacks blocking?

I couldn't find any examples that match this situation. There is half an example in the docs which use a literal string, but that's the case that works fine :)

edit: and to clarify, the C++ build works fine, but obviously the callback is more immediate

SoylentGraham commented 2 years ago

I also tried copying to a global buffer... (wrapped via a std::function lambda) no improvement

static char DumbBuffer[1024];
void OutputViaDumbBuffer(const char* String,Manifest_OnStreamMetaChanged* Callback)
{
    memset( DumbBuffer, 0, std::size(DumbBuffer) );
    memcpy( DumbBuffer, String, strlen(String) );
    Callback( &DumbBuffer[0] );
}
SoylentGraham commented 2 years ago

I have changed const Options = {async:true}; to const Options = {};

And I think the problem is fixed; I only added this as I called Module.ccall inside an async function in javascript (I have built with -sASYNCIFY just because I wanted to use emscripten_sleep() to yield to the event loop) because it was throwing an error.

Putting the Module.ccall in a non-async function seems to give me a correct callback...

Is this expected behaviour? For future reference, should I be allocating data to pass back if in an async function in a different manor? (I guess there's a whole different can of worms using asyncify)