disable custom page size support when using -fPIC#779
disable custom page size support when using -fPIC#779dicej wants to merge 4 commits intoWebAssembly:mainfrom
-fPIC#779Conversation
See WebAssembly#778 for details.
|
BTW, I had hoped to add a test for this, but just building a p1, p2, or p3 executable module/component with |
|
Yeah I've wanted to add tests for |
dicej
force-pushed
the
no-custom-page-sizes-for-pic
branch
4 times, most recently
from
8310b44 to
c2bc526
Compare
|
@alexcrichton I added a test that covers the change. My CMake skills aren't great, though, so I'm open to any tips you might have on making that part more idiomatic. |
This includes a new `add_waslibc_shared_test` CMake function which builds the test executable using shared libraries linked via `wasm-tools component link`. There's probably a better way to express this in CMake, but this is the only one I was able to make work.
dicej
force-pushed
the
no-custom-page-sizes-for-pic
branch
from
c2bc526 to
3ad2b39
Compare
| add_library(${test_library} SHARED ${test_file}) | ||
| target_compile_options(${test_library} PRIVATE -fPIC) | ||
| target_link_libraries(${test_library} PRIVATE libc_test_support_shared) | ||
| # Here we embed the component type custom section into `${test_library}` using | ||
| # `wasm-tools component embed`, then generate the `${test_name}` file by | ||
| # linking `${test_library}`, `liblibc_test_support_shared.so`, and `libc.so` | ||
| # together using `wasm-tools component link`. | ||
| # | ||
| # TODO: There must be a better way to express this in CMAKE. | ||
| add_custom_command( | ||
| TARGET ${test_library} | ||
| POST_BUILD | ||
| COMMAND ${wasm_tools} component embed --world wasi:cli/command "${CMAKE_CURRENT_SOURCE_DIR}/../wasi/p2/wit" $<TARGET_FILE:${test_library}> -o $<TARGET_FILE:${test_library}> | ||
| COMMAND ${wasm_tools} component link $<TARGET_FILE:${test_library}> "${SYSROOT_LIB}/libc.so" $<TARGET_FILE:libc_test_support_shared> -o ${test_name} | ||
| DEPENDS engine | ||
| ) |
There was a problem hiding this comment.
Compilation-wise it might be easier to compile an executable with clang but pass -Wl,--skip-wit-component and then afterwards manually invoke the wasm-tools component link command. That should skip the embed part here and also look more like an executable test file
There was a problem hiding this comment.
Per your later comment it sounds like it's okay to leave this as-is and just remove the TODO part, correct?
| function(add_wasilibc_shared_flags target) | ||
| add_dependencies(${target} sysroot builtins) | ||
| # Using an `INTERFACE` looks to get what we want here which is to rebuild this | ||
| # if the library changes but not actually add any arguments to the link-line. | ||
| target_link_libraries(${target} INTERFACE c) | ||
| endfunction() | ||
|
|
||
| if (NOT TARGET_TRIPLE MATCHES "-threads") | ||
| add_library(libc_test_support_shared SHARED | ||
| "${LIBC_TEST}/src/common/path.c" | ||
| "${LIBC_TEST}/src/common/print.c" | ||
| "${LIBC_TEST}/src/common/rand.c" | ||
| "${LIBC_TEST}/src/common/utf8.c" | ||
| ) | ||
| target_include_directories(libc_test_support_shared PUBLIC "${LIBC_TEST}/src/common") | ||
| add_wasilibc_shared_flags(libc_test_support_shared) | ||
| # Disable some warnings in this third-party code | ||
| target_compile_options(libc_test_support_shared PRIVATE | ||
| -fvisibility=default | ||
| -fPIC | ||
| -Wno-unused-variable | ||
| -Wno-unused-parameter | ||
| -Wno-unused-function | ||
| -Wno-sign-compare | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
To avoid duplicating the above here I might recommend poking around add_internal_library_pair and doing something similar here. Basically building two libraries, one with -fPIC one without, and then linking to the "right one". A small loop could handle defining options for both of those targets.
There was a problem hiding this comment.
I've pushed an update; let me know if you think more needs to be done with this.
| #include <stdio.h> | ||
| #include <__macro_PAGESIZE.h> | ||
|
|
||
| __attribute__((__export_name__("wasi:cli/run@0.2.0#run"))) |
There was a problem hiding this comment.
does this mean to bypass crt startup? if so, it seems like too much assumptions on malloc implementation.
There was a problem hiding this comment.
It wasn't intended to do that, no. Functions such as __wasm_call_ctors, __wasm_apply_data_relocs, etc. will still be run; that's taken care of by wasm-tools compose link.
This is just a way to build an executable component from shared libraries, since ctest wants to run executables rather than shared libraries. There might be a better way to do that, but this seems to work fine.
There was a problem hiding this comment.
ok. thank you for explanation.
|
Closing this in favor of #780 |
This PR is derived from #779 to build and run dynamically-linked tests in wasi-libc to help exercise shared-library-related support. This requires the fix from #779, for example, to get tests passing. This takes a different approach to testing, however, to build and run all tests as dynamically linked instead of only some particular tests. This avoids the need for a one-off test and additionally helps reuse all existing infrastructure for tests. This expands the test coverage more and documents a few cases where dynamic libraries don't work (long-float intrinsics and setjmp) --------- Co-authored-by: Joel Dice <joel.dice@akamai.com>
3 participants
See #778 for details.