Update emscripten version to 1.38.8.

[ekpyron]

Depends on argotorg/solc-js#296 (merged)

This results in:

• Travis: emscripten changed (1.38.8)
• Circleci: emscripten changed (1.38.8)

[ekpyron]

It also seems possible to use newer versions of emscripten, when restricted to asm.js. For that a list of exported symbols as used in solc-js has to be provided, though, and solc-js has to be slightly adjusted to account for different naming (e.g. Runtime.addFunction => addFunction).

A web assembly build seems possible as well, but so far I did not get the solc-js CLI to work with it (may be worth looking into, though, since it's less than half the size and seems way faster than the asm.js build).

[chriseth]

@axic can you tell what the problem was?

[ekpyron]

It may be that the problem was connected to the gnosis tests that are currently disabled, since they are broken anyways...

[axic]

#3491 was failing to compile with an updated emscripten version. Perhaps it will compile with the new boost version.

Let's push these changes onto that PR and see if it compiles. If at least it is not significantly worse than it is right now, I'd vote to merge this PR.

However, does it affect the soljson size? Is it smaller? Bigger? Unfortunately we do not have speed tests (should) to see how that differs between the two boost versions.

[ekpyron]

@axic Do you think we should fix solc-js to include the hack in the test scripts here? Should be easy to do in a "backwards"-compatible way and might be nice, if people want to build and use soljson.js locally.
Same goes for the CMAKE_EXE_LINKER_FLAGS in solidity - we could automatically add them, if a newer emscripten version is detected.

[ekpyron]

The solc-js test fails naturally, since it downloads a different version of soljson.js in which addFunction is still named soljson.Runtime.addFunction and not soljson.addFunction (which is what it is called in the soljson.js built from this PR). This would need some fix in solc-js, if we want to continue with this PR...

[codecov]

Codecov Report

Merging #4441 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #4441   +/-   ##
========================================
  Coverage    88.25%   88.25%           
========================================
  Files          347      347           
  Lines        33153    33153           
  Branches      3982     3982           
========================================
  Hits         29260    29260           
  Misses        2535     2535           
  Partials      1358     1358

Flag	Coverage Δ
#all	88.25% <ø> (ø)	⬆️
#syntax	28.24% <ø> (ø)	⬆️

[axic]

To me it seems we could also apply this after merging #4486.

[christianparpart]

apart from my 2 above comments, it's just version number changes, that do LGTM.

[ekpyron]

@axic Regarding the additional compile options:
I added STRICT=1 - that will prevent us from using deprecated compiler options, which makes sense - and our build still works.

I don't think we need or should unconditionally add VERBOSE or EMCC_DEBUG at this time - those are just meant to produce more compiler output in case of bugs.

[ekpyron]

Rebased and further simplified.

[ekpyron]

Moving to "Inbox" in 0.5.4 in the hopes that we see some progress on this. The PR is still done and just needs a review.

[axic]

I'm not sure we should have them here or in the libsolc makefile.

The libsolc makefile has the functions exported by libsolc. These are the functions also needed by solc-js.

[ekpyron]

Actually, I considered moving them, but then kept them here in the end. To me it feels more like a setting for Emscripten to make it export these functions - these are not really defined in libsolc after all... but I'd be fine with moving them over as well, if you prefer that.

[axic]

I'm not sure which is cleaner:

• having all in libsolc
• having all here
• having them split because they are for different purposes?
• (An additional possibility is to actually use emscripten's annotations in the code to mark exported fnuctions to be kept.)

In any case probably we should keep a note in both places.

[ekpyron]

I added a notice on both sides. I think it makes sense to split it this way - if e.g. libyul should export some functions, then we should specify it there and not have to duplicate the interface functions exported here.

[axic]

Rebased after #5809.

[ekpyron]

@axic I extracted the emscripten changelog between the versions (starting from the latest change, so to be read from bottom to top) and left a short comment on each of them, stating why I think it doesn't affect us or is already addressed, so I think all of this should be fine:

Emscripten Changelog between the versions 1.37.21 and 1.38.8:

• Breaking change: Simplify exception handling, disabling it by default.
  Previously it was disabled by default in -O1 and above and enabled in -O0,
  which could be confusing. You may notice this change if you need exceptions
  and only run in -O0 (since if you test in -O1 or above, you'd see you need to
  enable exceptions manually), in which case you will receive an error at
  runtime saying that exceptions are disabled by default and that you should
  build with -s DISABLE_EXCEPTION_CATCHING=0 to enable them. We compile with -O3, so no effect.
• Fix regression in 1.37.37 on configure scripts on MacOS (see [Yul] Fix registration of functions in scopes. #6456) We compile on Linux, so no effect.
• MALLOC option, allowing picking between dlmalloc (previous allocator and
  still the default) and emmalloc, a new allocator which is smaller and
  simpler.We don't use this option (might consider it, though)
• Binaryen update that should fix all known determinism bugs.Nice!
• addFunction is now supported on LLVM wasm backend, but when being used on
  the wasm backend, you need to provide an additional second argument, a Wasm
  function signature string. Each character within a signature string
  represents a type. The first character represents the return type of a
  function, and remaining characters are for parameter types.
  • 'v': void type
  • 'i': 32-bit integer type
  • 'j': 64-bit integer type (currently does not exist in JavaScript)
  • 'f': 32-bit float type
  • 'd': 64-bit float type
    For asm.js and asm2wasm you can provide the optional second argument, but it
    isn't needed. For that reason this isn't a breaking change, however,
    providing the second argument is recommended so that code is portable across
    all backends and modes.Only affects WASM backend, which we don't use
• LLVM and clang updates from upstream (5.0svn, close 5.0 release).Good!
• Breaking change: Don't export the ALLOC_* numeric constants by default. As
  with previous changes, a warning will be shown in -O0 and when ASSERTIONS
  are on if they are used.Not used by us
• Breaking change: Don't export FS methods by default. As with previous
  changes, a warning will be shown in -O0 and when ASSERTIONS are on, which
  will suggest either exporting the specific methods you need, or using
  FORCE_FILESYSTEM which will auto export all the main filesystem methods.
  Aside from using FS methods yourself, you may notice this change when using a
  file package created standalone, that is, by running the file packager
  directly and then loading it at run time (as opposed to telling emcc to
  package the files for you, in which case it would be aware of them at compile
  time); you should build with FORCE_FILESYSTEM to ensure filesystem support
  for that case.Not used by us
• Breaking change: Remove the Runtime object, and move all the useful methods
  from it to simple top-level functions. Any usage of Runtime.func should be
  changed to func.Supported by solc-js now
• Breaking change: Change NO_EXIT_RUNTIME to 1 by default. This means that by
  default we don't include code to shut down the runtime, flush stdio streams,
  run atexits, etc., which is better for code size. When ASSERTIONS is on, we
  warn at runtime if there is text buffered in the streams that should be
  flushed, or atexits are used.We already had it set to 1 explicitly
• Meta-DCE for JS+wasm: remove unused code between JS+wasm more aggressively.
  This should not break valid code, but may break code that depended on unused
  code being kept around (like using a function from outside the emitted JS
  without exporting it - only exported things are guaranteed to be kept alive
  through optimization).WASM related, doesn't affect us
• Breaking change: Similar to the getValue/setValue change from before (and
  with the same ASSERTIONS warnings to help users), do not export the
  following runtime methods by default: ccall, cwrap, allocate,
  Pointer_stringify, AsciiToString, stringToAscii, UTF8ArrayToString,
  UTF8ToString, stringToUTF8Array, stringToUTF8, lengthBytesUTF8, stackTrace,
  addOnPreRun, addOnInit, addOnPreMain, addOnExit, addOnPostRun,
  intArrayFromString, intArrayToString, writeStringToMemory,
  writeArrayToMemory, writeAsciiToMemory.All we need are exported in this PR
• Breaking change: Do not polyfill Math.{clz32, fround, imul, trunc} by
  default. A new LEGACY_VM_SUPPORT option enables support for legacy
  browsers. In ASSERTIONS mode, a warning is shown if a polyfill was needed,
  suggesting using that option.Doesn't seem to affect us, probably not used by us
• Breaking change: Do not export getValue/setValue runtime methods by default.
  You can still use them by calling them directly in code optimized with the
  main file (pre-js, post-js, js libraries; if the optimizer sees they are
  used, it preserves them), but if you try to use them on Module then you
  must export them by adding them to EXTRA_EXPORTED_RUNTIME_METHODS. In -O0
  or when ASSERTIONS is on, a run-time error message explains that, if they
  are attempted to be used incorrectly.Adjusted in this PR by adding to EXTRA_EXPORTED_RUNTIME_METHODS

[axic]

The generated size seems to be ~100k smaller (9080k vs 9230k)

[chriseth]

Do we really need bash? ;)

[ekpyron]

As a matter of fact I think we do for having well specified behavior with the arguments here :-).