wasi: accept int32 ptr in clock_time_get #62822
Conversation
|
Review requested:
|
8719763 to
9099d80
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
| @@ -306,6 +307,16 @@ uint32_t ConvertType(Local<Value> value) { | |||
| return value.As<Uint32>()->Value(); | |||
| } | |||
|
|
|||
| template <> | |||
There was a problem hiding this comment.
Why do you need a template argument and a function for this one liner?
There was a problem hiding this comment.
WASI function arguments are type-checked here, but since there was no int32_t template, this time_ptr type change required adding one.
guybedford
left a comment
There was a problem hiding this comment.
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.
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 asuint32_t, and adds regression tests.