Skip to content

wasi: accept int32 ptr in clock_time_get #62822

Open
islandryu wants to merge 1 commit into
nodejs:mainfrom
islandryu:wasi-uint32-coercion
Open

wasi: accept int32 ptr in clock_time_get #62822
islandryu wants to merge 1 commit into
nodejs:mainfrom
islandryu:wasi-uint32-coercion

Conversation

@islandryu
Copy link
Copy Markdown
Member

Fixes: #62671

clock_time_get() receives time_ptr as a Wasm i32, which may appear as Int32 in JS.
This PR accepts it as int32_t, reinterprets it as uint32_t, and adds regression tests.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/wasi

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. wasi Issues and PRs related to the WebAssembly System Interface. labels Apr 19, 2026
@islandryu islandryu force-pushed the wasi-uint32-coercion branch from 8719763 to 9099d80 Compare April 19, 2026 09:47
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.68%. Comparing base (a6e9e32) to head (9099d80).
⚠️ Report is 359 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62822      +/-   ##
==========================================
+ Coverage   89.64%   89.68%   +0.03%     
==========================================
  Files         676      706      +30     
  Lines      206240   218228   +11988     
  Branches    39514    41771    +2257     
==========================================
+ Hits       184891   195707   +10816     
- Misses      13465    14415     +950     
- Partials     7884     8106     +222     
Files with missing lines Coverage Δ
src/node_wasi.cc 66.15% <100.00%> (+0.42%) ⬆️
src/node_wasi.h 0.00% <ø> (ø)

... and 241 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/node_wasi.cc
@@ -306,6 +307,16 @@ uint32_t ConvertType(Local<Value> value) {
return value.As<Uint32>()->Value();
}

template <>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you need a template argument and a function for this one liner?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

WASI function arguments are type-checked here, but since there was no int32_t template, this time_ptr type change required adding one.

@islandryu islandryu changed the title wasi: accept int32 values for uint32 args wasi: accept int32 ptr in clock_time_get Apr 26, 2026
Copy link
Copy Markdown
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Rather than just fixing this for this one function, it would be worthwhile to update CheckType<uin32_t> more generally to accept either Int32 or Uint32 and reinterpret on convert:

template <>
bool CheckType<uint32_t>(Local<Value> value) {
  return value->IsUint32() || value->IsInt32();
}
template <>
uint32_t ConvertType(Local<Value> value) {
  if (value->IsInt32()) {
    return static_cast<uint32_t>(value.As<Int32>()->Value());
  }
  return value.As<Uint32>()->Value();
}

That would solve this issue for all existing WASI Apis, not just clocks - since all other APIs would suffer from the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. wasi Issues and PRs related to the WebAssembly System Interface.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clock_time_get rejects valid WASI pointer arguments >= 0x80000000(2G)

5 participants