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

wasi: fix serdes bugs from snapshot1 migration #31122

Closed
wants to merge 1 commit into from

Conversation

@cjihrig
Copy link
Contributor

cjihrig commented Dec 28, 2019

During the migration to WASI snapshot1, a field was removed from the subscription type. The field was removed from the code, but the bounds checking logic was not updated. This commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a uint64_t. However, the bounds checks were missed, and the code was still writing uint32_t's (to the new correct offset) instead of uint64_t's. This commit updates that logic as well.

Refs: #30980

(I've also opened cjihrig/uvwasi#69 to come up with a plan to make this process a bit smoother moving forward.)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
During the migration to WASI snapshot1, a field was removed
from the subscription type. The field was removed from the
code, but the bounds checking logic was not updated. This
commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a
uint64_t. However, the bounds checks were missed, and the code
was still writing uint32_t's (to the new correct offset) instead
of uint64_t's. This commit updates that logic as well.

Refs: #30980
@Trott
Trott approved these changes Dec 29, 2019
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link

nodejs-github-bot commented Dec 29, 2019

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Dec 31, 2019

Landed in b9f0351

@Trott Trott closed this Dec 31, 2019
Trott added a commit that referenced this pull request Dec 31, 2019
During the migration to WASI snapshot1, a field was removed
from the subscription type. The field was removed from the
code, but the bounds checking logic was not updated. This
commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a
uint64_t. However, the bounds checks were missed, and the code
was still writing uint32_t's (to the new correct offset) instead
of uint64_t's. This commit updates that logic as well.

Refs: #30980

PR-URL: #31122
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@cjihrig cjihrig deleted the cjihrig:wasi-serdes branch Dec 31, 2019
BridgeAR added a commit that referenced this pull request Jan 3, 2020
During the migration to WASI snapshot1, a field was removed
from the subscription type. The field was removed from the
code, but the bounds checking logic was not updated. This
commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a
uint64_t. However, the bounds checks were missed, and the code
was still writing uint32_t's (to the new correct offset) instead
of uint64_t's. This commit updates that logic as well.

Refs: #30980

PR-URL: #31122
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
targos added a commit that referenced this pull request Jan 14, 2020
During the migration to WASI snapshot1, a field was removed
from the subscription type. The field was removed from the
code, but the bounds checking logic was not updated. This
commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a
uint64_t. However, the bounds checks were missed, and the code
was still writing uint32_t's (to the new correct offset) instead
of uint64_t's. This commit updates that logic as well.

Refs: #30980

PR-URL: #31122
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
targos added a commit that referenced this pull request Jan 14, 2020
During the migration to WASI snapshot1, a field was removed
from the subscription type. The field was removed from the
code, but the bounds checking logic was not updated. This
commit updates that check.

Similarly, __wasi_linkcount_t changed from a uint32_t to a
uint64_t. However, the bounds checks were missed, and the code
was still writing uint32_t's (to the new correct offset) instead
of uint64_t's. This commit updates that logic as well.

Refs: #30980

PR-URL: #31122
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.