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

cmake: Add wasi toolchain #328

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

cmake: Add wasi toolchain #328

wants to merge 15 commits into from

Conversation

axic
Copy link
Member

@axic axic commented May 20, 2021

Part of #293.

@axic
Copy link
Member Author

axic commented May 20, 2021

With cmake -DTOOLCHAIN=wasi .. this goes pretty far, but it seems that imported ethash project currently depends on pthreads, which is not supported:

Scanning dependencies of target ethash
[ 62%] Building CXX object lib/ethash/CMakeFiles/ethash.dir/managed.cpp.o
[ 62%] Building CXX object lib/ethash/CMakeFiles/ethash.dir/ethash.cpp.o
[ 87%] Building CXX object lib/ethash/CMakeFiles/ethash.dir/progpow.cpp.o
[ 87%] Building C object lib/ethash/CMakeFiles/ethash.dir/primes.c.o
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:26:6: error: no type named 'mutex' in namespace 'std'
std::mutex shared_context_mutex;
~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:30:6: error: no type named 'mutex' in namespace 'std'
std::mutex shared_context_full_mutex;
~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:47:10: error: no member named 'lock_guard' in namespace 'std'
    std::lock_guard<std::mutex> lock{shared_context_mutex};
    ~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:47:26: error: no member named 'mutex' in namespace 'std'
    std::lock_guard<std::mutex> lock{shared_context_mutex};
                    ~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:47:33: error: use of undeclared identifier 'lock'; did you mean 'clock'?
    std::lock_guard<std::mutex> lock{shared_context_mutex};
                                ^~~~
                                clock
/Users/alex/Projects/evmone/wasi-sdk-12.0//share/wasi-sysroot/include/time.h:61:9: note: 'clock' declared here
clock_t clock (void);
        ^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:68:10: error: no member named 'lock_guard' in namespace 'std'
    std::lock_guard<std::mutex> lock{shared_context_full_mutex};
    ~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:68:26: error: no member named 'mutex' in namespace 'std'
    std::lock_guard<std::mutex> lock{shared_context_full_mutex};
                    ~~~~~^
/Users/alex/.hunter/_Base/135567a/60efa9e/32bc5aa/Build/ethash/Source/lib/ethash/managed.cpp:68:33: error: use of undeclared identifier 'lock'; did you mean 'clock'?
    std::lock_guard<std::mutex> lock{shared_context_full_mutex};
                                ^~~~
                                clock
/Users/alex/Projects/evmone/wasi-sdk-12.0//share/wasi-sysroot/include/time.h:61:9: note: 'clock' declared here
clock_t clock (void);
        ^
8 errors generated.
make[5]: *** [lib/ethash/CMakeFiles/ethash.dir/managed.cpp.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [lib/ethash/CMakeFiles/ethash.dir/all] Error 2
make[3]: *** [all] Error 2
make[2]: *** [ethash-Release-prefix/src/ethash-Release-stamp/ethash-Release-build] Error 2
make[1]: *** [CMakeFiles/ethash-Release.dir/all] Error 2
make: *** [all] Error 2

Can ethash be improved to not compile pthread-dependent parts for evmone?

@axic
Copy link
Member Author

axic commented May 20, 2021

The second problem is building evmc, dl* used by the loader is not available. This would mean we need some configurability what is built out of evmc as a dependency for evmone (excluding tests/tools).

@codecov
Copy link

codecov bot commented May 20, 2021

Codecov Report

Merging #328 (1724d3d) into master (2c1f473) will not change coverage.
The diff coverage is n/a.

❗ Current head 1724d3d differs from pull request most recent head a320c66. Consider uploading reports for the commit a320c66 to get more accurate results

@@           Coverage Diff           @@
##           master     #328   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          27       27           
  Lines        3932     3932           
=======================================
  Hits         3923     3923           
  Misses          9        9           
Flag Coverage Δ
consensus 97.73% <ø> (ø)
unittests 99.77% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@axic
Copy link
Member Author

axic commented May 20, 2021

D'oh, there's a toolchain+platform file in wasi-sdk, but that seems to be working worse for me.

@chfast
Copy link
Member

chfast commented May 21, 2021

Can ethash be improved to not compile pthread-dependent parts for evmone?

This is the problem Silkworm is also having (cc @yperbasis, @AndreaLanfranchi). We can attack the ethash library first, probably by disabling some "managed" API in wasm build.

Here we only need the ethash::keccak library so other solution is to not build full ethash library. This can be done by adding some CMake options to ethash or not using Hunter to get it.

@AndreaLanfranchi
Copy link
Contributor

Yes we experienced the same problem in Silkworm and opted to amalgamate in our source (with slight modifications) only the strict necessary from ethash library code. Here what we have now https://github.com/torquem-ch/silkworm/tree/master/ethash/ethash

@axic
Copy link
Member Author

axic commented May 22, 2021

This is the problem Silkworm is also having (cc @yperbasis, @AndreaLanfranchi). We can attack the ethash library first, probably by disabling some "managed" API in wasm build.

Here we only need the ethash::keccak library so other solution is to not build full ethash library. This can be done by adding some CMake options to ethash or not using Hunter to get it.

I think for the evmone case, we may be better off having a BUILD_ETHASH=OFF setting (which is on by default), as we only need keccak.

Not sure about the Silkworm case, @AndreaLanfranchi does silkworm also uses the ethash part for PoW?

@axic
Copy link
Member Author

axic commented May 22, 2021

But for evmc we may need to have cmake settings for building the client- vs vm-side only. On the vm-side the loader is not needed. Another dumb option is just having an ifdef in the loader for unsupported platforms resulting in abort(), but I guess that is not to your liking.

@AndreaLanfranchi
Copy link
Contributor

@axic

Not sure about the Silkworm case, @AndreaLanfranchi does silkworm also uses the ethash part for PoW?

Yes we implement the full Ethash PoW validation

@axic
Copy link
Member Author

axic commented Feb 6, 2023

Currently blocked on evmc:

[  5%] Building C object evmc/lib/instructions/CMakeFiles/instructions.dir/instruction_metrics.c.o
[ 10%] Building C object evmc/lib/instructions/CMakeFiles/instructions.dir/instruction_names.c.o
[ 21%] Building CXX object evmc/lib/tooling/CMakeFiles/tooling.dir/run.cpp.o
[ 21%] Building C object evmc/lib/loader/CMakeFiles/loader.dir/loader.c.o
/home/builder/project/evmc/lib/loader/loader.c:25:10: fatal error: 'dlfcn.h' file not found
#include <dlfcn.h>
         ^~~~~~~~~
1 error generated.

Why do we need to even build these from evmc? At this point I think mostly only the headers are used.

@chfast
Copy link
Member

chfast commented Feb 6, 2023

Why do we need to even build these from evmc? At this point I think mostly only the headers are used.

These are used for evmc testing tool but we can skip it.

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 this pull request may close these issues.

3 participants