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.

Sign up for GitHub

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: partial workaround for high address support #62

Merged
merged 2 commits into from
Apr 8, 2024
Merged

fix: partial workaround for high address support #62

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4f8517
fix: partial workaround for high address support
chrisdickinson Apr 5, 2024
03fb8cd
fix: match js nomenclature for ptr offset argument in errors
chrisdickinson Apr 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 44 additions & 14 deletions crates/core/src/globals.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,47 +442,77 @@ fn build_memory(context: &JSContextRef) -> anyhow::Result<JSValueRef> {
let data = data.as_bytes()?;
let m = extism_pdk::Memory::from_bytes(data)?;
let mut mem = HashMap::new();
let offset = JSValue::Int(m.offset() as i32);
let len = JSValue::Int(m.len() as i32);

// FLOAT NOTE(Chris): SDKs represent addresses as 64-bit offsets and extents. Some
// SDKS, like the JS SDK, store block address information in the high 32 bits. Using a
// QuickJS integer type would limit us to 32-bits, erasing that info.
//
// So instead we rely on the time-honored JavaScript tradition of storing 53 bits
// of integer data in a double-precision, 64-bit float. This gives us at least 5
// bits of the high 32-bits, which allows the JS-SDK to represent 32 allocations.
//
// Notably, QuickJS supports 64-bit integers (bigint) types, they're just not exposed
// through the wrapper library we're using. We should revisit once someone (maybe us?)
// exposes bigints through the library.
let offset = JSValue::Float(m.offset() as f64);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good and makes sense to me, but only really in the context of this PR. I think a single comment here about why we're packing the offset into a float may be helpful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent point – added a note!

let len = JSValue::Float(m.len() as f64);
mem.insert("offset".to_string(), offset);
mem.insert("len".to_string(), len);
Ok(JSValue::Object(mem))
},
)?;
let memory_find = context.wrap_callback(
|_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
let ptr = args.first().ok_or(anyhow!("Expected ptr argument"))?;
let ptr = args.first().ok_or(anyhow!("Expected offset argument"))?;
if !ptr.is_number() {
bail!("Expected a pointer");
bail!("Expected an offset");
}
let ptr = ptr.as_i32_unchecked();
let ptr = if ptr.is_repr_as_i32() {
ptr.as_i32_unchecked() as i64
} else {
ptr.as_f64_unchecked() as i64
};

let m = extism_pdk::Memory::find(ptr as u64).unwrap();
let mut mem = HashMap::new();
let offset = JSValue::Int(m.offset() as i32);
let len = JSValue::Int(m.len() as i32);

// See "FLOAT NOTE".
let offset = JSValue::Float(m.offset() as f64);
let len = JSValue::Float(m.len() as f64);
mem.insert("offset".to_string(), offset);
mem.insert("len".to_string(), len);
Ok(JSValue::Object(mem))
},
)?;
let memory_free = context.wrap_callback(
|_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
let ptr = args.first().ok_or(anyhow!("Expected ptr argument"))?;
let ptr = args.first().ok_or(anyhow!("Expected offset argument"))?;
if !ptr.is_number() {
bail!("Expected a pointer");
bail!("Expected an offset");
}
let ptr = ptr.as_i32_unchecked();
extism_pdk::Memory::find(ptr as u64).map(|x| x.free());
let ptr = if ptr.is_repr_as_i32() {
ptr.as_i32_unchecked() as i64
} else {
ptr.as_f64_unchecked() as i64
};
if let Some(x) = extism_pdk::Memory::find(ptr as u64) {
x.free();
}

Ok(JSValue::Undefined)
},
)?;
let read_bytes = context.wrap_callback(
|_ctx: &JSContextRef, _this: JSValueRef, args: &[JSValueRef]| {
let ptr = args.first().ok_or(anyhow!("Expected ptr argument"))?;
let ptr = args.first().ok_or(anyhow!("Expected offset argument"))?;
if !ptr.is_number() {
bail!("Expected a pointer");
bail!("Expected an offset");
}
let ptr = ptr.as_i32_unchecked();
let ptr = if ptr.is_repr_as_i32() {
ptr.as_i32_unchecked() as i64
} else {
ptr.as_f64_unchecked() as i64
};
let m = extism_pdk::Memory::find(ptr as u64).unwrap();
let bytes = m.to_vec();
Ok(JSValue::ArrayBuffer(bytes))
Expand Down
Loading