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

Fix arity of generated switcher continuations #105

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

dhil
Copy link
Member

@dhil dhil commented Dec 29, 2024

This patch fixes a bug with the arity of the continuation reference
generated by a switch (aka the current continuation). Its arity was
mistakenly derived from the switch-tag's codomain. The arity of the
current continuation can be obtained at the switch point by
deconstructing the type annotation on it.

Fixes #103.

@dhil dhil requested a review from rossberg December 29, 2024 19:16
Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, sorry, had missed this!

interpreter/exec/eval.ml Outdated Show resolved Hide resolved
interpreter/exec/eval.ml Outdated Show resolved Hide resolved
@dhil
Copy link
Member Author

dhil commented Jan 14, 2025

There seems to be some problem with the CI -- is this fixed upstream?

@rossberg
Copy link
Member

Btw, can you do a PR to rename Handle to Prompt?

@dhil
Copy link
Member Author

dhil commented Jan 14, 2025

Btw, can you do a PR to rename Handle to Prompt?

Yes, absolutely. Maybe better if I do that in a separate PR? Regardless, the CI fails to install OCaml -- is this a known issue elsewhere? Do we need to update the workflow definition?

@rossberg
Copy link
Member

Yes, separate PR.

The CI issue probably is just a glitch. Those happen, just manually restart the workflow.

@dhil
Copy link
Member Author

dhil commented Jan 14, 2025

Yes, separate PR.

The CI issue probably is just a glitch. Those happen, just manually restart the workflow.

I've restarted it multiple times already. Actually, it seems like the CI cannot install darcs:

  /usr/bin/sudo apt-get --yes install bubblewrap darcs g++-multilib gcc-multilib mercurial musl-tools rsync
  Reading package lists...
  Building dependency tree...
  Reading state information...
  Package darcs is not available, but is referred to by another package.
  This may mean that the package is missing, has been obsoleted, or
  is only available from another source
  E: Package 'darcs' has no installation candidate
  Notice: An error has been caught in some system package index files, so the system package index files have been re-synchronised, and the system package installation has been retried: the process '/usr/bin/sudo' failed with exit code 100
  /usr/bin/sudo apt-get update
  Get:1 file:/etc/apt/apt-mirrors.txt Mirrorlist [142 B]
  Hit:2 http://azure.archive.ubuntu.com/ubuntu noble InRelease
  Hit:6 https://packages.microsoft.com/repos/azure-cli noble InRelease
  Hit:7 https://packages.microsoft.com/ubuntu/24.04/prod noble InRelease
  Get:3 http://azure.archive.ubuntu.com/ubuntu noble-updates InRelease [126 kB]
  Hit:4 http://azure.archive.ubuntu.com/ubuntu noble-backports InRelease
  Hit:5 http://azure.archive.ubuntu.com/ubuntu noble-security InRelease
  Fetched 126 kB in 1s (241 kB/s)
  Reading package lists...
  /usr/bin/sudo apt-get --yes install bubblewrap darcs g++-multilib gcc-multilib mercurial musl-tools rsync
  Reading package lists...
  Building dependency tree...
  Reading state information...
  Package darcs is not available, but is referred to by another package.
  This may mean that the package is missing, has been obsoleted, or
  is only available from another source
  E: Package 'darcs' has no installation candidate
Error: The process '/usr/bin/sudo' failed with exit code 100

@dhil
Copy link
Member Author

dhil commented Jan 14, 2025

I think what has happened is that the version of ubuntu-latest has been bumped. Consequently, the package repositories have been changed too.

Edit: I think my analysis is correct, see ocaml/setup-ocaml#872

@dhil
Copy link
Member Author

dhil commented Jan 14, 2025

@rossberg I think we need to update the workflow to use setup-ocaml@v3 (which supposedly fixes the problem). Probably every active repo requires this change.

@rossberg
Copy link
Member

IIRC, I already upgraded upstream last year. Maybe it'd be good to resync this one soon.

@dhil dhil force-pushed the switch-contref-arity branch from 8b1c068 to 2445f30 Compare January 15, 2025 10:00
@dhil
Copy link
Member Author

dhil commented Jan 15, 2025

IIRC, I already upgraded upstream last year. Maybe it'd be good to resync this one soon.

Yes. Should I merge with WebAssembly/spec main or branch wasm-3.0? The latter seems to have been updated last time in November.

@rossberg
Copy link
Member

wasm-3.0, since that's the baseline here, isn't it? Only that has references and exceptions and all.

@dhil
Copy link
Member Author

dhil commented Jan 15, 2025

wasm-3.0, since that's the baseline here, isn't it? Only that has references and exceptions and all.

OK. I will do two more PRs. One for renaming Handle to Prompt and one to sync with wasm-3.0.

This patch fixes a bug with the arity of the continuation reference
generated by a `switch` (aka the current continuation). Its arity was
mistakenly derived from the switch-tag's codomain. The arity of the
current continuation can be obtained at the `switch` point by
deconstructing the type annotation on it.
@dhil dhil force-pushed the switch-contref-arity branch from 2445f30 to 6579ef4 Compare January 15, 2025 13:24
@dhil dhil merged commit 63959f0 into WebAssembly:main Jan 15, 2025
1 check passed
@dhil dhil deleted the switch-contref-arity branch January 15, 2025 13:35
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.

runtime crash: stack underflow when executing opcode switch
2 participants