Why does this simple program trigger an alignment fault?

[debevv]

Version of emscripten/emsdk:

emcc (Emscripten gcc/clang-like replacement + linker emulating GNU ld) 3.1.24 (68a9f990429e0bcfb63b1cde68bad792554350a5)
clang version 16.0.0 (https://github.com/llvm/llvm-project 277c382760bf9575cfa2eac73d5ad1db91466d3f)
Target: wasm32-unknown-emscripten
Thread model: posix

In the recent days I'm fighting with some strange behavior while trying to implement a simple messaging system between wasm workers. After a while I managed to isolate what I think should be the root cause of all the random crashes I'm getting on WASM (and not on Linux-x64).
This simple program crashes with Aborted(alignment fault):

#include <emscripten/wasm_worker.h>

#include <vector>

class Lock {
public:
    Lock() = default;

    Lock(const Lock&) = delete;
    Lock& operator=(const Lock&) = delete;

    Lock(Lock&&) = default;
    Lock& operator=(Lock&&) = default;

    ~Lock() {
        this->release();
    }

    void acquire() {      
        emscripten_lock_waitinf_acquire(&this->mutex);
    }

    void release() {
        emscripten_lock_release(&this->mutex);
    }

private:
    emscripten_lock_t mutex = EMSCRIPTEN_LOCK_T_STATIC_INITIALIZER;
};

int main() {
    std::vector<uint8_t> v;
    v.resize(65536 * 16);

    Lock l;

    std::vector<uint8_t> v2;
    v2.resize(65536 * 16);

    return 0;
}

I'm building it with these flags:

em++ -O0 -g -std=gnu++20 -fno-exceptions -sWASM=1 -sSTRICT=1 \
-sWASM_WORKERS -sALLOW_MEMORY_GROWTH=1 -sINITIAL_MEMORY=65536000 -sUSE_SDL=0 -sUSE_PTHREADS=0 \
-sMIN_WEBGL_VERSION=2 -sMAX_WEBGL_VERSION=2 -sFORCE_FILESYSTEM -sSTACK_OVERFLOW_CHECK=2 \
-sASSERTIONS=2 -sSAFE_HEAP=1 -fsanitize=undefined alignment.cpp -o alignment.html

Full error log (screenshot because i can't paste from chromium console):

It's super weird because this does not happen in Linux builds, not even with all the various -fsanitize= flags. Moreover, removing the line with Lock l; or replacing it with emscripten_lock_t mutex = EMSCRIPTEN_LOCK_T_STATIC_INITIALIZER; (which I suppose should have the same sizeof) makes the crash go away.

Again, am I missing something here? Maye some WASM quirk I'm not taking into consideration?

[sbc100]

It seems to be the combination of -sSAFE_HEAP=1 and -fsanitize=undefined together. I'm not sure that combination get a lot of testing.

BTW I was able to narrow the command line down and reproduce under node:

$ em++ --profiling-funcs -sWASM_WORKERS -sSAFE_HEAP -fsanitize=undefined alignment.cpp
$ node --experimental-wasm-threads --experimental-wasm-bulk-memory  a.out.js
Aborted(alignment fault)
/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:94
   throw ex;
   ^

RuntimeError: Aborted(alignment fault)
    at abort (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:793:10)
    at alignfault (/usr/local/google/home/sbc/dev/wasm/emscripten/a.out.js:435:2)
    at SAFE_HEAP_STORE_i32_4_4 (<anonymous>:wasm-function[558]:0x1a943)
    at dlmalloc (<anonymous>:wasm-function[138]:0x8887)
    at operator new(unsigned long) (<anonymous>:wasm-function[147]:0xa54c)
    at void* std::__2::__libcpp_operator_new<unsigned long>(unsigned long) (<anonymous>:wasm-function[123]:0x6eb1)
    at std::__2::__libcpp_allocate(unsigned long, unsigned long) (<anonymous>:wasm-function[121]:0x6dd5)
    at std::__2::allocator<unsigned char>::allocate(unsigned long) (<anonymous>:wasm-function[117]:0x69f9)
    at std::__2::allocator_traits<std::__2::allocator<unsigned char>>::allocate(std::__2::allocator<unsigned char>&, unsigned long) (<anonymous>:wasm-function[91]:0x4e2a)
    at std::__2::__split_buffer<unsigned char, std::__2::allocator<unsigned char>&>::__split_buffer(unsigned long, unsigned long, std::__2::allocator<unsigned char>&) (<anonymous>:wasm-function[76]:0x3402)

[debevv]

Reading the documentation about SAFE_HEAP looks like the memory alignment issues are not necessarily a problem (maybe only for performance), so I don't think these are what I'm looking for here. I continued experimenting with SAFE_HEAP=2.

At the risk of making this look like a code review, I'll show you what I'm banging my head on. As I wrote, I'm trying to make a simple worker to worker messaging system. This code crashes 100% of the time, with errors depending on whether SAFE_HEAP and -fsanitize=undefined are enable or not. By the way, I keep them active all the time, since the error messages tend to be totally random without them.
Errors are always due to dynamic memory allocations or deallocations insideQueue::pop() and Queue::push(), so I believe that the root cause may be a race condition somewhere. But the code is so simple that I don't understand how I could use the emscripten_lock_t API incorrectly. Moreover, there are no crashes when built for Linux with clang.

#ifdef __EMSCRIPTEN__
#include <emscripten/wasm_worker.h>
#else
#include <functional>
#include <thread>
#include <mutex>
#endif

#include <any>
#include <chrono>
#include <deque>
#include <optional>
#include <memory>
#include <string>

using namespace std::chrono_literals;

class Lock {
public:
#ifdef __EMSCRIPTEN__
    Lock() {
        emscripten_lock_init(&this->lock);
    }
#else
    Lock() = default;
#endif

    Lock(const Lock&) = delete;
    Lock& operator=(const Lock&) = delete;

    Lock(Lock&&) = delete;
    Lock& operator=(Lock&&) = delete;

#ifdef __EMSCRIPTEN__
    ~Lock() {
        this->release();
    }
#else
    ~Lock() = default;
#endif

    void acquire() {
#ifdef __EMSCRIPTEN__
        emscripten_lock_waitinf_acquire(&this->lock);
#else
        this->mutex.lock();
#endif
    }

    void release() {
#ifdef __EMSCRIPTEN__
        emscripten_lock_release(&this->lock);
#else
        this->mutex.unlock();
#endif
    }

#ifndef __EMSCRIPTEN__
    std::timed_mutex& native() {
        return this->mutex;
    }
#endif

private:
#ifdef __EMSCRIPTEN__
    emscripten_lock_t lock = EMSCRIPTEN_LOCK_T_STATIC_INITIALIZER;
#else
    std::timed_mutex mutex;
#endif
};

template<class T>
class Queue {
public:
    explicit Queue(uint32_t size) : size(size) {}

    void push(const T& item) {
        this->lock.acquire();

        if (this->queue.size() == this->size) {
            this->lock.release();
            abort();
        }

        this->queue.push_back(item);

        this->lock.release();
    }

    std::optional<T> pop() {
        this->lock.acquire();

        if (this->queue.empty()) {
            this->lock.release();
            return {};
        }

        std::optional<T> el = this->queue.front();
        this->queue.pop_front();

        this->lock.release();

        return el;
    }

private:
    const uint32_t size;

    std::deque<T> queue;

    Lock lock;
};

class Things {
public:
    void doThings() {
        // Not printed in worker
        // printf("called on worker to do a thing\n");
    }

    std::string doThingsAndReturn(int a) {
        return std::to_string(a);
    }
};

Queue<std::function<void()>> callsQueue(100000);
Queue<std::string> dataQueue(100000);

template<class I, class F, class... Args>
auto call(std::shared_ptr<I> obj, F&& f, Args&&... args) {
    const std::function<void()> fun = [obj, f, ... args = std::forward<Args>(args)]() mutable -> void {
        auto mf = std::bind_front(f, obj);
        mf(args...);
    };

    callsQueue.push(fun);
}

void worker1() {
    auto thingsDoer = std::make_shared<Things>();
    std::string data = "abcdefghijklmnopqrstuvwxyz";

    while (true) {
        call(thingsDoer, &Things::doThings);
        call(thingsDoer, &Things::doThings);
        call(thingsDoer, &Things::doThings);
        call(thingsDoer, &Things::doThings);
        call(thingsDoer, &Things::doThings);

        dataQueue.push(data);
        dataQueue.push(data);
        dataQueue.push(data);
        dataQueue.push(data);
        dataQueue.push(data);

        srand(time(nullptr));
        auto r = rand() % 10;
#ifdef __EMSCRIPTEN__
        emscripten_wasm_worker_sleep(r * 1000000);
#else
        usleep(r * 1000);
#endif
    }
}

void worker2() {
    while (true) {
        auto f = callsQueue.pop();
        while (f) {
            (*f)();
            f = callsQueue.pop();
        }

        auto d = dataQueue.pop();
        while (d) {
            d = dataQueue.pop();
        }

        srand(time(nullptr));
        auto r = rand() % 10;
#ifdef __EMSCRIPTEN__
        emscripten_wasm_worker_sleep(r * 1000000);
#else
        usleep(r * 1000);
#endif
    }
}

void main_loop() {
    static bool initialized = false;
#ifdef __EMSCRIPTEN__
    if (!initialized) {
        auto w1 = emscripten_malloc_wasm_worker(65536 * 16);
        emscripten_wasm_worker_post_function_v(w1, &worker1);
        auto w2 = emscripten_malloc_wasm_worker(65536 * 16);
        emscripten_wasm_worker_post_function_v(w2, &worker2);

        initialized = true;
    }
#else
    static std::thread w1;
    static std::thread w2;

    if (!initialized) {
        w1 = std::thread(worker1);
        w2 = std::thread(worker2);

        initialized = true;
    }
#endif
}

int main() {
#ifdef __EMSCRIPTEN__
    emscripten_set_main_loop(main_loop, 0, 1);
    emscripten_cancel_main_loop();
#else
    while (true) {
        main_loop();
        usleep(static_cast<uint64_t>(1000000 / 60));
    }
#endif
    return 0;
}

Example of a typical error:

[mannk123]

Created a PR for this - more details in there.

[debevv]

@mannk123 I can confirm that with your PR the alignment issue is gone.
Unfortunately, I'm also sure now that it was not the cause of the crashes I'm experiencing.
@sbc100 did you have the chance to look into the code I posted?
In the meanwhile, I managed to make a more compact version of it:

#include <emscripten/wasm_worker.h>

#include <deque>
#include <optional>
#include <string>

template<class T>
class Queue {
public:
    Queue() {
        emscripten_lock_init(&this->lock);
    }

    void push(const T& item) {
        emscripten_lock_waitinf_acquire(&this->lock);
        this->queue.push_back(item);
        emscripten_lock_release(&this->lock);
    }

    std::optional<T> pop() {
        emscripten_lock_waitinf_acquire(&this->lock);

        if (this->queue.empty()) {
            emscripten_lock_release(&this->lock);
            return {};
        }

        std::optional<T> el = this->queue.front();
        this->queue.pop_front();

        emscripten_lock_release(&this->lock);

        return el;
    }

private:
    std::deque<T> queue;
    emscripten_lock_t lock = EMSCRIPTEN_LOCK_T_STATIC_INITIALIZER;
};


Queue<std::string> queue;

void worker1() {
    std::string data = "abcdefghijklmnopqrstuvwxyz";

    while (true) {
        queue.push(data);
        queue.push(data);
        queue.push(data);
        queue.push(data);
        queue.push(data);

        emscripten_wasm_worker_sleep(500 * 1000);
    }
}

void worker2() {
    while (true) {
        auto d = queue.pop();
        while (d) {
            d = queue.pop();
        }
        
        emscripten_wasm_worker_sleep(500 * 1000);
    }
}

void main_loop() {
    static bool initialized = false;
    if (!initialized) {
        auto w1 = emscripten_malloc_wasm_worker(65536 * 16);
        emscripten_wasm_worker_post_function_v(w1, &worker1);
        auto w2 = emscripten_malloc_wasm_worker(65536 * 16);
        emscripten_wasm_worker_post_function_v(w2, &worker2);
        initialized = true;
    }
}

int main() {
    emscripten_set_main_loop(main_loop, 0, 1);
    return 0;
}

[mannk123]

It appears that malloc does NOT default to being thread safe when WASM_WORKERS is enabled!

@debevv I have updated my PR to turn on lock usage in malloc when using WASM_WORKERS, from my testing the segfault in your latest example goes away after that.

[debevv]

I let it run for a while, and looks like it works now! @mannk123 you can't imagine how much I'm grateful, this problem was not only making me go crazy, but it was completely blocking too.
I hope the maintainers will merge the PR as soon as possible, since this seems rather a big problem for who's using wasm workers.

[mannk123]

@debevv Glad that it worked for you! Let's see what @sbc100 thinks of the fix, I am new to emscripten myself so may have missed/misunderstood something.

[debevv]

I'm afraid the issue should be reopened because I think I found another case of memory corruption. @mannk123 @sbc100 @juj
I noticed some strange behaviors in my application, seemingly tied to the amount of memory specified with sINITIAL_MEMORY, so I created a test program:

#include <emscripten/emscripten.h>
#include <emscripten/wasm_worker.h>

#include <vector>
#include <iostream>
#include <chrono>
#include <cstdlib>
#include <list>

std::atomic_bool done = false;

void worker() {
    const uint32_t iterations = 5;    
    const uint32_t blockSize = 1024;
    const uint32_t memSize = 64 * 1000 * blockSize;
    
    for(uint32_t i = 0; i < iterations; i++) {
        std::srand(std::chrono::steady_clock::now().time_since_epoch().count());

        std::vector<uint8_t> block;
        for(uint32_t i = 0; i < blockSize; i++) {
            block.push_back((uint8_t)(rand() % 256));
        }

        std::list<std::vector<uint8_t>> memory;
        for(uint64_t i = 0; i < memSize / blockSize; i++) {
            memory.push_back(block);
        }
        
        for(uint32_t i = 0; i < memSize / blockSize; i++) {
            memory.pop_front();
        }
    }

    done = true;
}


void main_loop() {
    static bool initialized = false;
    if (!initialized) {
        auto w = emscripten_malloc_wasm_worker(65536 * 16);
        emscripten_wasm_worker_post_function_v(w, &worker);
        initialized = true;
    }
    
   if(done) {
        std::cout << "Done" << std::endl;
        emscripten_cancel_main_loop();
    }
}


int main() {
    emscripten_set_main_loop(main_loop, 0, 1);
    return 0;
}

And I discovered that It actually crashes when the amount of allocated memory goes above the one specified with sINITIAL_MEMORY. The crash happens only when the allocations are made on the worker and sSAFE_HEAP=2 is enabled.
In this example I'm using about 64MB (memSize constant) and -sINITIAL_MEMORY=31981568. If you lower memSize under -sINITIAL_MEMORY the program runs to completion.

This is how I'm building it in order to trigger the crash:

em++ -O3 -g0 -sWASM_WORKERS=1 -sASSERTIONS=1 -sINITIAL_MEMORY=31981568 -sALLOW_MEMORY_GROWTH=1 -sSAFE_HEAP=2 memusage.cpp -o memusage.html

[juj]

Thanks for the test case. Posted PR #18170 that does fix one issue with that test case, the JS-side SAFE_HEAP checks there have a chance to race.

Also stripped down the test case to smallest scenario where the issue still reproduces: #18171 , but unfortunately did not yet figure out why that one is failing.