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

Fetch api unified into promises , callback and asynchify #21630

Open
wants to merge 42 commits into
base: main
Choose a base branch
from

Conversation

impact-maker
Copy link

@impact-maker impact-maker commented Mar 27, 2024

Solves #21440

@impact-maker
Copy link
Author

@sbc100 @tlively Just pushed the first draft for the Fetch API. Kindly provide me some feedback so that I can complete the api restructuring along with test suites
Also, dropped an email to both of you at mdeep9771@gmail.com and would love to hear about the direction we're heading. Your feedback could help speed things up

aheejin and others added 27 commits April 11, 2024 23:40
This is the list of files using 80-bit long double:
https://github.com/llvm/llvm-project/blob/6009708b4367171ccdbf4b5905cb6a803753fe18/compiler-rt/lib/builtins/CMakeLists.txt#L279-L294
(This file is from LLVM 17.0.6, which is currently our compiler-rt
 version)

We don't have 80-bit long doubles so it looks we can exclude them.
…ipten-core#21649)

* Merge two `SYSCALLS_REQUIRE_FILESYSTEM` blocks. NFC

* Split out syscall vararg handling into seperate functions. NFC

This means we only include these function when they are needed.

As part of a seperate change I was having issue with closure compiler
when `SYSCALLS.varargs` are used by never assigned to.
Its possible that passing these function directly to Wasm is easier
to optimize in the engine.
This updates libcxx and libcxxabi to LLVM 18.1.2. Due to
llvm/llvm-project#65534 we need to update both
at the same time.

Things to note:

- Disable hardening mode:
  LLVM 18.1.2 adds support for "Hardening Modes" to libcxx
  (https://libcxx.llvm.org/Hardening.html). There are four modes: none,
  fast, extensive and debug. Different hardening modes make different
  trade-offs between the amount of checking and runtime performance. We
  for now disable it (i.e., set it to 'none') so that we don't have any
  effects on the performance. We can consider enabling it when we ever
  get to enable the debug version of libcxx.

- Disable C++20 time zone support:
  LLVM 18.1.2 adds C++20 time zone support
  (https://libcxx.llvm.org/DesignDocs/TimeZone.html) to libcxx, which
  requires access IANA Time Zone Database. Currently it seems it only
  supports Linux:
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/src/tz.cpp#L45-L49
  So this excludes the two source files from build (which is done via
  `CMakeLists.txt` in the upstream LLVM) and sets
  `_LIBCPP_HAS_NO_TIME_ZONE_DATABASE` macro in `__config_site`. In
  future maybe we can consider implementing this in JS.

- `__cxa_init_primary_exception` support:
  llvm/llvm-project#65534 introduces
  `__cxa_init_primary_exception` and uses this in libcxx. Like several
  other methods like the below in `cxa_exception.cpp`,
  https://github.com/emscripten-core/emscripten/blob/fbdd9249e939660cb0d20f00d6bc1897c2f3905e/system/lib/libcxxabi/src/cxa_exception.cpp#L264-L269
  this new function takes a pointer to a destructor, and in Wasm
  destructor returns a `this` pointer, which requires `ifdef
  __USING_WASM_EXCEPTION__` on its signature. And that it is also used
  in libcxx means we need to guard some code in libcxx with `ifdef
  __USING_WASM_EXCEPTION__` for the signature difference, and we need to
  build libcxx with `-D__USING_WASM_EXCEPTIONS__` when Wasm EH is used.
  Also we add Emscripte EH's counterpart in
  `cxa_exception_emscripten.cpp` too.

- This version fixes long-running misbehaviors of `operator new`
  llvm/llvm-project#69498 seems to fix some
  long-running misbhaviors of `operator new`, which in emscripten we
  tried to fix in emscripten-core#11079 and emscripten-core#20154. So while resolving the conflicts, I
  effectively reverted emscripten-core#11079 and emscripten-core#20154 in
  `libcxx/src/stdlib_new_delete.cpp` and `libcxx/src/new.cpp`.

- Add default `__assertion_handler` to `libcxx/include`:
  llvm/llvm-project#77883 adds a way for vendors
  to override how we handle assertions. If a vendor doesn't want to
  override it, CMake will copy the provided [default template assertion
  handler
  file](https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/vendor/llvm/default_assertion_handler.in)
  to libcxx's generated include dir, which defaults to
  `SYSROOT/include/c++/v1`:
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L72-L73
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/CMakeLists.txt#L439
  https://github.com/llvm/llvm-project/blob/26a1d6601d727a96f4301d0d8647b5a42760ae0c/libcxx/include/CMakeLists.txt#L1024
  We don't use CMake, so this renames the provided
  `vendor/llvm/default_assertion_handler.in` to `__assert_handler` in
  our `libcxx/include` directory, which will be copied to
  `SYSROOT/include/c++/v1`.

- Disable `__libcpp_verbose_abort directly` in code:
  In emscripten-core#20707 we decided to disable `__libcpp_verbose_abort` not to incur
  the code size increase that brings (it brings in `vfprintf` and its
  family). We disabled it in adding
  `#define _LIBCPP_AVAILABILITY_HAS_NO_VERBOSE_ABORT` in
  `__config_site`. But that `_NO_` macros are gone in LLVM 18.1.2, and
  changing it to `#define _LIBCPP_AVAILABILITY_HAS_VERBOSE_ABORT 0` does
  not work because it is overridden by this line, unless we provide our
  own vendor annotations:
  https://github.com/llvm/llvm-project/blob/38f5596feda3276a8aa64fc14e074334017088ca/libcxx/include/__availability#L138
  I asked about this in
  llvm/llvm-project#87012 and
  llvm/llvm-project#71002 (comment)
  (and more comments below)
  but didn't get an actionable answer yet. So this disables
  `__libcpp_verbose_abort` in the code directly for now, hopefully
  temporarily.

Each fix is described in its own commit, except for the fixes required
to resolve conflicts when merging our changes, which I wasn't able to
commit separately.

Fixes emscripten-core#21603.
- Update the regex's according to the TODO
- Remove checks for f32->f64 legalization since that was removed from
  binaryen back in https://github.com/WebAssembly/binaryen/pull/#2052
- Allow LEGALIZE_JS_API to work even when in standalone mode (standalone
  mode is implied by this test since it uses `-o a.out.wasm`.
- `e_i64_i64` was removed since it was checking for the internal
  function which takes an i64 and this function exists, even when the
  export itself is a legalizes stub.  Since we check the truthyness or
  falsyness of `e_i64_i32` in all cases anyway I think this was
  redundant.
This setting can cause issues if used on its own without
`-sWASM_BIGINT`.  See emscripten-core#21689.
…mscripten-core#21700)

This test seems to needless import itself which leads to some strange
behaviour where new workers would load script via `importSripts` and
then also via `require`.
I had a runtime error in another PR I'm working on where one of the
imports was undefined.  Accessing this `sig` property should inside the
`typeof` check.  Also since its only really ever accessed ones (aside
from the assertion) I decided to just remove the extra variable.
…#21682)

This change doesn't fix emscripten-core#21667 but just encodes the current behavior
in a test.

As a followup we could consider resolving shared libraries dependencies
based on previous command line arguments which would solve emscripten-core#21667
without the need for adding `-L.`.
This updates compiler-rt to LLVM 18.1.2.

Things to note:

- llvm/llvm-project#73573 reformatted
  https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp,
  so our diff got somewhat bigger because adding a preprocessor like `ifdef`
  somewhere means changing the indentation of all inner directives.

- In
  https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp,
  ```diff
  - #elif SANITIZER_USE_GETAUXVAL
  -   return getauxval(AT_PAGESZ);
  ```
  These lines were somehow deleted in emscripten-core#11662 for no apparent reason. (It looks
  this is not used by us) Restored it to reduce the diff between us and the
  upstream.

- Build libsanitizer_common_rt with `-Wno-format`:
  emscripten-core@67108b2
  Some code that got newly compiled in this version has violations to
  `__attribute__ ((format))` that crashes compilation without `-Wno-format`.
  Detailed description of the problem is in
  emscripten-core@67108b2.
  The simple temporary fix of adding `-Wno-format` is basically what the
  upstream code currently does with `CMakeLists.txt`.

- `ThreadStart`-related changes in
  https://github.com/emscripten-core/emscripten/blob/main/system/lib/compiler-rt/lib/asan/asan_emscripten.cpp:
  emscripten-core@36ce3d3
  This deals with the following upstream changes:
  llvm/llvm-project@4e1b55a
  llvm/llvm-project@fd16d46
  by adding `ThreadStartParams` class as
  https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/asan/asan_win.cpp
  did.

Each fix is described in its own commit, except for the fixes required to
resolve conflicts when merging our changes, which I wasn't able to commit
separately.
…1719)

Lets see if this flakieness still persists.  I ran the test 10 times
locally and didn't see it.
And also using `!=` when we know the LHS are RHS are both string,
specifically `typeof xx` on the left and a string literal on the right.
…core#21637)

As pointed out by emscripten-core#21625, when using unique_ptr's with primitive types the
raw pointer was being passed between wasm and JS, but it was using the
underlying type (e.g. char, int, etc) to convert the value. This means the
pointer was truncated to a char at the boundary.

Embind doesn't allow pointers or references to primitive types so it doesn't
make sense to allow a unique_ptr for these types either.
dependabot bot and others added 14 commits April 11, 2024 23:40
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.24.9 to 3.24.10.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@1b1aada...4355270)

---
updated-dependencies:
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ile (emscripten-core#21721)

Its useful for pre-js files to be able to depend on these variables,
and this is specifically necessary for emscripten-core#21701 which removes the default
export of `ENVIRONMENT_IS_PTHREAD`.

Note that this also fixes an inconsistency between the regular runtime
and the minimal runtime where minimal runtime was defining some
`ENVIRONMENT_IS_XX` var before pre-js but the regular runtime was not.
Update the expected output of `test_eval_ctors_debug_output` to account for
changed debug output due to WebAssembly/binaryen#6464.
The test will now succeed before and after that upstream Binaryen change.
)

This setting affects whether the ASYNCIFY_ADD list propagates, that is,
whether it automatically leads to more things added based on the
inference. It is on by default, which is safer, but may increase code size
in some cases - to undo that, set it to 0.

Fixes emscripten-core#13150
I noticed when working on emscripten-core#21701 then these tests were not renaming
the `.js` output, which meant that if `mainScriptUrlOrBlob` was
completely ignored the tests would still pass.

These changes rename the output `.js` file such that the default
name will not longer work and the tests would fail if
`mainScriptUrlOrBlob` were ignored.
…core#21734)

Outside of `MODULARIZE` this is just a normal variable that can/should
be minified.
…1736)

This always seems to refer to full filename/url.  Perhaps in the past
it actually did hold the directory name?
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.

None yet

9 participants