Pass emscripten linker options only when linking. #12233
Conversation
ekpyron
force-pushed
the
emscriptenCMake
branch
3 times, most recently
from
22888b8 to
ef4453a
Compare
|
The resulting |
ekpyron
force-pushed
the
emscriptenCMake
branch
from
ef4453a to
068e8cf
Compare
|
As for the set of options: those are exactly the options that a build with emscripten 2.0.33 complained about. |
ekpyron
force-pushed
the
emscriptenCMake
branch
from
068e8cf to
6f748cf
Compare
| # Abort if linking results in any undefined symbols | ||
| # Note: this is on by default in the CMake Emscripten module which we aren't using | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s ERROR_ON_UNDEFINED_SYMBOLS=1") | ||
| set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -s ERROR_ON_UNDEFINED_SYMBOLS=1") | ||
| # Disallow deprecated emscripten build options. | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s STRICT=1") |
| @@ -111,39 +111,39 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA | |||
| # http://stackoverflow.com/questions/21617158/how-to-silence-unused-command-line-argument-error-with-clang-without-disabling-i | |||
| add_compile_options(-Qunused-arguments) | |||
| elseif(EMSCRIPTEN) | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --memory-init-file 0") | |||
| set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} --memory-init-file 0") | |||
| # Leave only exported symbols as public and aggressively remove others | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fdata-sections -ffunction-sections -fvisibility=hidden") | |||
| # Optimisation level | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O3") | |||
| # Re-enable exception catching (optimisations above -O1 disable it) | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s DISABLE_EXCEPTION_CATCHING=0") | |||
There was a problem hiding this comment.
Apparently CMAKE_CXX_FLAGS are still passed to the linker as well by emscripten - but I added this and -s STRICT to both now, just to be sure.
ekpyron
force-pushed
the
emscriptenCMake
branch
from
6f748cf to
9ecfceb
Compare
|
For the record: the options are documented in https://github.com/emscripten-core/emscripten/blob/main/src/settings.js |
There was a problem hiding this comment.
Looks fine as far as I can tell. Options seem fine at a glance, though if there are any subtleties involved, I probably would not spot them without digging into the docs.
I checked the build log and there are no warnings. I also downloaded the binary and tried to use it locally with solc-js and it works so at least it's not broken in an obvious way.
ekpyron
force-pushed
the
emscriptenCMake
branch
from
9ecfceb to
e7deedb
Compare
|
Damn it, now I had to rebase due to a Changelog conflict due to the entry in #12236 (which was probably superfluous anyways) :-)... Can I trouble you guys for one more re-approval :-)? |
In preparation of #11689
Emscripten 2.0.32 will raise warnings on this which are treated as error by us (see https://github.com/emscripten-core/emscripten/blob/main/ChangeLog.md):
Will need to look at the soljson.js CI artifact to confirm that the options are correctly honoured and work using
CMAKE_EXE_LINKER_FLAGS.