Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad function pointer using fmt in a side-module. #22587

Closed
jdumas opened this issue Sep 19, 2024 · 19 comments · Fixed by #22625
Closed

Bad function pointer using fmt in a side-module. #22587

jdumas opened this issue Sep 19, 2024 · 19 comments · Fixed by #22625

Comments

@jdumas
Copy link

jdumas commented Sep 19, 2024

Version of emscripten/emsdk:

I'm on macOS 14.5.

❯ emcc -v
emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.65-git
clang version 20.0.0git
Target: wasm32-unknown-emscripten
Thread model: posix
InstalledDir: /opt/homebrew/Cellar/emscripten/3.1.65/libexec/llvm/bin

Failing command line in full:

Consider the following project:

CMakeLists.txt

# ------------------------------------------------------------------------------
# CMAKE SETUP
# ------------------------------------------------------------------------------

cmake_minimum_required(VERSION 3.25)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON CACHE BOOL "Enable/Disable output of compile commands during generation.")

project("Wasm Side Module")

# ------------------------------------------------------------------------------
# OPTIONS
# ------------------------------------------------------------------------------

option(USE_EXCEPTIONS "Use Exceptions" ON)
option(USE_WASM_EXCEPTIONS "Use WASM Exceptions" OFF)

set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard version to compile with")
set(CMAKE_CXX_STANDARD_REQUIRED ON CACHE BOOL "Whether the C++ standard version is a requirement")
set(CMAKE_CXX_EXTENSIONS OFF CACHE BOOL "Whether compiler specific extensions are requested")

# ------------------------------------------------------------------------------
# DEPENDENCIES
# ------------------------------------------------------------------------------

if(NOT CMAKE_BUILD_TYPE)
    set(CMAKE_BUILD_TYPE "RelWithDebInfo" CACHE STRING "Choose the type of build" FORCE)
endif()

include(FetchContent)

# Need dependencies to get them
add_compile_options(
    -fPIC
    -flto
    -fsanitize=undefined
    -Werror=bad-function-cast
    -Werror=cast-function-type
    -fno-omit-frame-pointer
)
add_link_options(
    -fPIC
    -flto
    -fsanitize=undefined
)
if(EMSCRIPTEN)
    add_link_options(
        --profiling-funcs
        -sSTACK_OVERFLOW_CHECK=2
        -sDYNCALLS=1
        -sEXCEPTION_STACK_TRACES=1
        -sSAFE_HEAP
        -sASSERTIONS=2
    )
endif()

if(USE_EXCEPTIONS)
    add_compile_options(-fexceptions)
    add_link_options(-fexceptions)
elseif(USE_WASM_EXCEPTIONS)
    add_compile_options(-fwasm-exceptions)
    add_link_options(-fwasm-exceptions)
endif()

FetchContent_Declare(
    fmt
    GIT_REPOSITORY https://github.com/fmtlib/fmt.git
    GIT_TAG ed8f8be70d82b0a081286e262867956e565c1bf6
)
FetchContent_MakeAvailable(
    fmt
)

if(EMSCRIPTEN)

    # Side module
    add_executable(side_module side_module.cpp)
    target_link_options(side_module PRIVATE -sSIDE_MODULE=2)

    target_link_libraries(side_module PRIVATE fmt::fmt)
    set_target_properties(side_module PROPERTIES
        PREFIX ""
        SUFFIX ".wasm"
        ENABLE_EXPORTS ON
    )

    # Main module
    add_executable(main_module main_module.cpp)
    target_link_options(main_module PRIVATE
        -sMAIN_MODULE=2
        -sNO_AUTOLOAD_DYLIBS
        $<TARGET_FILE:side_module>
    )

    target_link_libraries(main_module PRIVATE side_module)
    set_target_properties(main_module PROPERTIES SUFFIX ".cjs")

endif()

main_module.cpp

#include <iostream>

#include <dlfcn.h>

int main(int argc, char** argv)
{

    void* handle = dlopen("side_module.wasm", RTLD_NOW);

    if (!handle) {
        std::cout << "failed to dlopen" << std::endl;
        return 1;
    }

    typedef void (*func)();

    func test_func = (func)dlsym(handle, "my_function");

    {
        const char* error = dlerror();
        if (error || !test_func) {
            std::cout << "Failed to load symbol err = " << error << "\n";
            return 1;
        }
    }

    std::cout << "calling test_func" << std::endl;
    test_func();

    return 0;
}

side_module.cpp

#include <emscripten.h>
#include <fmt/format.h>

#include <iostream>
#include <map>
#include <string_view>

class MyObject
{
    struct AttributeManager
    {
        int create(std::string_view name)
        {
            std::string key(name);
            auto [it, insertion_successful] = m_name_to_id.emplace(key, m_name_to_id.size());
            return it->second;
        }

        int get_id(std::string_view name) const
        {
            std::string key(name);
            auto it = m_name_to_id.find(key);
            if (it != m_name_to_id.end()) {
                return it->second;
            } else {
                return -1;
            }
        }

    protected:
        std::map<std::string, int> m_name_to_id;
    };

    AttributeManager m_manager;

public:
    int get_attribute_id(std::string_view name) const
    {
        auto ret = m_manager.get_id(name);
        if (ret == -1) {
            throw std::runtime_error(fmt::format("Attribute '{}' does not exist.", name));
        }
        return ret;
    }

    int create_attribute(std::string_view name) { return m_manager.create(name); }
};

extern "C" {

EMSCRIPTEN_KEEPALIVE
void my_function()
{
    MyObject foo;
    int x = foo.create_attribute("foo");
    int y = foo.create_attribute("bar");

    std::cout << foo.get_attribute_id("foo") << std::endl;
    std::cout << foo.get_attribute_id("bar") << std::endl;
}

}

Steps to reproduce:

Compile with the following:

emcmake cmake ..
cmake --build . -j 12
node main_module.cjs

Failing output:

❯ node main_module.cjs
calling test_func
Aborted(Assertion failed: bad function pointer type - dynCall function not found for sig 'iij')
/Users/jedumas/sandbox/wasm/build/main_module.cjs:638
  /** @suppress {checkTypes} */ var e = new WebAssembly.RuntimeError(what);
                                        ^

RuntimeError: Aborted(Assertion failed: bad function pointer type - dynCall function not found for sig 'iij')
    at abort (/Users/jedumas/sandbox/wasm/build/main_module.cjs:638:41)
    at assert (/Users/jedumas/sandbox/wasm/build/main_module.cjs:370:5)
    at dynCallLegacy (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1268:3)
    at dynCall (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1297:13)
    at /Users/jedumas/sandbox/wasm/build/main_module.cjs:1805:12
    at stubs.<computed> (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1955:20)
    at side_module.wasm.legalfunc$invoke_iij (wasm://wasm/side_module.wasm-002f3df6:wasm-function[213]:0x21ab1)
    at side_module.wasm.my_function (wasm://wasm/side_module.wasm-002f3df6:wasm-function[72]:0x7691)
    at main_module.wasm.main (wasm://wasm/main_module.wasm-00f4246e:wasm-function[398]:0x23b11)
    at callMain (/Users/jedumas/sandbox/wasm/build/main_module.cjs:6651:15)

Node.js v22.7.0
@jdumas
Copy link
Author

jdumas commented Sep 19, 2024

It seems that combining both LTO, DCE (with -sSIDE_MODULE=2) and exceptions (with -fexceptions) results in the runtime error shown above. If I remove any of those 3, the code runs fine.

I've also been trying to replace my usage of std::string_view with string-view-lite, hoping I could use a custom fmt::formatter<> with it. But so far I haven't had any luck doing that (I did specialize fmt::is_range<> to disable range detection as well). @vitaut here is a godbolt sample of what I'm trying to achieve with string-view-lite and fmt. Can you advice on what I'm missing here?

@vitaut
Copy link

vitaut commented Sep 19, 2024

@jdumas {fmt} has logic to autodetect string-like types so if your custom string view has compatible API it will be automatically formattable.

@jdumas
Copy link
Author

jdumas commented Sep 19, 2024

@vitaut is there a way to hijack that? At least for local testing to see if I can circumvent the runtime error by having fmt convert it to a full std::string, or use the fmt::streamed() approach.

@vitaut
Copy link

vitaut commented Sep 19, 2024

@sbc100
Copy link
Collaborator

sbc100 commented Sep 19, 2024

Does enabling -sWASM_BIGINT fix the issue?

How about using -fwasm-exceptions?

@jdumas
Copy link
Author

jdumas commented Sep 19, 2024

@vitaut Hmm this doesn't seem to work. Am I forgetting something?

@sbc100 I enabled -sWASM_BIGINT and the issue persists. However, even with the option enabled I cannot get a static_assert(sizeof(int) == sizeof(uint64_t)); to work.

Using -fwasm-exceptions the runtime works fine, but our product hasn't enabled it yet so I'd like to find a workaround that can work with -fexceptions.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 19, 2024

-sWASM_BIGINT doesn't change anything about the C/C++ environment. It just means that we can allow i64 values to flow to/from JavaScript rather than having binaryen split them into i32 pairs. That splitting is that cause of the legalfunc part in side_module.wasm.legalfunc$invoke_iij in the stacktrace above. I guess that was a red herring though.

@jdumas
Copy link
Author

jdumas commented Sep 19, 2024

I see. Yes the stack trace looks like this after I enable -sWASM_BIGINT:

calling test_func
Aborted(Assertion failed: bad function pointer type - dynCall function not found for sig 'iij')
/Users/jedumas/sandbox/wasm/build/main_module.cjs:642
  /** @suppress {checkTypes} */ var e = new WebAssembly.RuntimeError(what);
                                        ^

RuntimeError: Aborted(Assertion failed: bad function pointer type - dynCall function not found for sig 'iij')
    at abort (/Users/jedumas/sandbox/wasm/build/main_module.cjs:642:41)
    at assert (/Users/jedumas/sandbox/wasm/build/main_module.cjs:370:5)
    at dynCallLegacy (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1267:3)
    at dynCall (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1298:13)
    at /Users/jedumas/sandbox/wasm/build/main_module.cjs:1800:12
    at stubs.<computed> (/Users/jedumas/sandbox/wasm/build/main_module.cjs:1949:20)
    at side_module.wasm.my_function (wasm://wasm/side_module.wasm-002f281a:wasm-function[72]:0x763f)
    at main_module.wasm.main (wasm://wasm/main_module.wasm-00f3df46:wasm-function[397]:0x23682)
    at callMain (/Users/jedumas/sandbox/wasm/build/main_module.cjs:6617:15)
    at doRun (/Users/jedumas/sandbox/wasm/build/main_module.cjs:6655:23)

Node.js v22.7.0

@vitaut
Copy link

vitaut commented Sep 20, 2024

Hmm this doesn't seem to work. Am I forgetting something?

My bad. Since this trait is internal, it's only used for convenience in some places and not enough to disable string-like API detection.

@jdumas
Copy link
Author

jdumas commented Sep 20, 2024

Ah, too bad. Would it make sense to expose it as a feature in fmt? Without it I don't really see what other workaround I could use.

@vitaut
Copy link

vitaut commented Sep 20, 2024

Would it make sense to expose it as a feature in fmt?

Sure, a PR would be welcome, provided that there is a reasonable motivation - I can't say that I fully understand what is going on in this issue.

@jdumas
Copy link
Author

jdumas commented Sep 24, 2024

I can't say that I fully understand what is going on in this issue.

Yeah me neither. I'm hoping @sb100 can confirm whether it's a bug in emscripten and whether there is a possible workaround.

@sbc100
Copy link
Collaborator

sbc100 commented Sep 24, 2024

I think I see what the problem might be. Can you confirm is adding RTLD_GLOBAL to your dlopen call fixes the issue? Is so I think I know what is doing on.

@jdumas
Copy link
Author

jdumas commented Sep 24, 2024

Indeed, adding RTLD_GLOBAL to the dlopen() the program doesn't crash!

@sbc100
Copy link
Collaborator

sbc100 commented Sep 25, 2024

I'm working on a fix, but is adding RTLD_GLOBAL an acceptable workaround for now?

@jdumas
Copy link
Author

jdumas commented Sep 25, 2024

I think so? I'm not sure exactly what this does tbh. Do you have an explanation of what is happening and why RTLD_GLOBAL fixes the issue?

@sbc100
Copy link
Collaborator

sbc100 commented Sep 25, 2024

Then you use RTLD_GLOBAL the dynamic linker adds the symbols to the global symbol table. See

$mergeLibSymbols: (exports, libName) => {
. While doing this is also registers the dynCall_xxx functions that are provided by the module:
if (sym.startsWith('dynCall_') && !Module.hasOwnProperty(sym)) {
Module[sym] = exp;
}
.

If mergeLibSymbols is never called then those symbols are never registered resulting in your dynCall function not found for sig error.

@jdumas
Copy link
Author

jdumas commented Sep 25, 2024

Hmm but how do you explain that the program ran fine in Debug mode, or without LTO/DCE enabled?

In any case thanks for your help! I would have never been able to figure this out myself 😅

@sbc100
Copy link
Collaborator

sbc100 commented Sep 25, 2024

Hmm but how do you explain that the program ran fine in Debug mode, or without LTO/DCE enabled?

In any case thanks for your help! I would have never been able to figure this out myself 😅

I would imagine that perhaps in that mode the main module already had its own version of dynCall_iij so the one in the side module was redundant. Then in release mode the module itself no longer has any iij function pointers and so didn't need the dynCall_iij itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants