wren icon indicating copy to clipboard operation
wren copied to clipboard

wrenCall -> foreign call causes memory corruption

Open redthing1 opened this issue 3 months ago • 9 comments

Issue

I want to wrenCall a function in a wren script, and get its return value. This function will call into a foreign function.

The foreign function needs to do some list/dictionary processing with nested data, so I need some more slots. I use wrenEnsureSlots.

However, this causes some pretty bad corruption. In valgrind I see invalid reads, and this also results in the foreign call not actually properly returning a value, and the wren stack being corrupted. The program also sometimes segfaults when trying to free the VM.

I understand that this is related to this previous issue.

My understanding is that Wren doesn't like when you wrenEnsureStack within a foreign call; however, I am not aware of any other way than wrenEnsureSlots to ensure I have enough slots to process nested data.

Is there a better way I can do this? Am I using the library incorrectly?

It seems like a fairly standard use case to use some additional slots in a foreign call implementation. But calling wrenEnsureSlots causes corruption.

Interestingly, if I don't call wrenEnsureSlots inside the foreign call, because I called it before wrenCall, the wren stack doesn't get corrupted and does return the bool, but valgrind of course reports a lot of miscellaneous memory corruption.

Overview of my code

The overview of the scripts is as follows:

builtin.wren:

class Test {
  foreign static cool_func_impl(name, platforms, hashes)

  static cool_func(meta) {
    var meta_name = meta.name
    var meta_platforms = meta.platforms
    var meta_hashes = meta.hashes

    return cool_func_impl(meta_name, meta_platforms, meta_hashes)
  }
}

test.wren:

import "builtin" for Test

System.print("foreign call test")

class ProgramMeta {
    construct new() { }

    name { "foreign call test" }
    platforms { ["windows", "linux", "macos"] }
    hashes {{
        "windows" : [
            "hash1",
            "hash2"
        ],
        "linux" : [
            "hash1",
            "hash2"
        ],
        "macos" : [
            "hash1",
            "hash2"
        ]
    }}
}

class Script {
    static run() {
        System.print("script is now running")

        return Test.cool_func(ProgramMeta.new())
    }
}

Implementation of the foreign call:

#define WREN_NEEDED_SLOTS 12

// ...

static void fcall_cool_func_impl(WrenVM* vm) {
  // read the arguments to make a CoolMeta struct
  // if we can parse successfully, return true

  // ensure that we have plenty of slots
  wrenEnsureSlots(vm, WREN_NEEDED_SLOTS);
  size_t slot_temp = 10;

  // get name
  size_t arg_name_slot = 1;
  assert_msg(wrenGetSlotType(vm, arg_name_slot) == WREN_TYPE_STRING, "name must be a string");
  const char* name = wrenGetSlotString(vm, arg_name_slot);

  // get platforms
  size_t arg_platforms_slot = 2;
  assert_msg(wrenGetSlotType(vm, arg_platforms_slot) == WREN_TYPE_LIST, "platforms must be a list");
  WrenHandle* platforms_list = wrenGetSlotHandle(vm, arg_platforms_slot);
  std::vector<std::string> platforms;
  size_t platforms_count = wrenGetListCount(vm, arg_platforms_slot);
  for (size_t i = 0; i < platforms_count; i++) {
    size_t platform_list_el_slot = slot_temp;
    wrenGetListElement(vm, arg_platforms_slot, i, platform_list_el_slot);
    assert_msg(wrenGetSlotType(vm, platform_list_el_slot) == WREN_TYPE_STRING, "platform must be a string");
    const char* platform = wrenGetSlotString(vm, platform_list_el_slot);
    platforms.push_back(platform);
  }

  // return true
  wrenSetSlotBool(vm, 0, true);
}

My program then does the following:

  // now, we want to call Script.run()

  wrenEnsureSlots(vm, WREN_NEEDED_SLOTS);

  // get handle to Script class
  wrenGetVariable(vm, source_module, "Script", 0);
  WrenHandle* script_class = wrenGetSlotHandle(vm, 0);
  // make call handle to run method
  WrenHandle* run_method = wrenMakeCallHandle(vm, "run()");

  // invoke Script.run()
  wrenSetSlotHandle(vm, 0, script_class);
  printf("wren: call Script.run()\n");
  wrenCall(vm, run_method);

  // get the boolean return value
  if (wrenGetSlotType(vm, 0) != WREN_TYPE_BOOL) {
    printf("wren: Script.run() must return a boolean, but it returned %d\n", wrenGetSlotType(vm, 0));
    assert_msg(false, "Script.run() returned invalid type");
  }
  bool run_result = wrenGetSlotBool(vm, 0);
  printf("wren: Script.run() returned %s\n", run_result ? "true" : "false");

Reproduction of Issue

I have made a full reproduction of the issue here: https://github.com/redthing1/wrenvm_tests/tree/f3985888d39168bd69b281083ff6c3a77dd9ec3d/foreigncall

Instructions to build the repro

install prerequisites:

  • c/c++ compiler
  • meson
  • ninja

get submodules (wren is a submodule):

git submodule update --init --recursive

build a demo, such as ./foreigncall:

cd ./foreigncall
meson setup build
ninja -C build
./build/foreigncall_test

redthing1 avatar Apr 04 '24 03:04 redthing1