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

feat(heap): Make number and string allocation functions unsafe #60

Merged
merged 10 commits into from
Nov 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
41 changes: 34 additions & 7 deletions nova_vm/src/ecmascript/types/language/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ impl std::fmt::Debug for Number {
}
}

impl From<NumberIndex> for Number {
fn from(value: NumberIndex) -> Self {
Number::Number(value)
}
}

impl From<SmallInteger> for Number {
fn from(value: SmallInteger) -> Self {
Number::Integer(value)
Expand Down Expand Up @@ -60,6 +66,25 @@ impl From<f32> for Number {
}
}

const MAX_NUMBER: f64 = ((1u64 << 53) - 1) as f64;
const MIN_NUMBER: f64 = -MAX_NUMBER;

impl TryFrom<f64> for Number {
type Error = ();

fn try_from(value: f64) -> Result<Self, ()> {
if value.is_finite() && value.trunc() == value && (MIN_NUMBER..=MAX_NUMBER).contains(&value)
{
debug_assert_eq!(value as i64 as f64, value);
Ok(Number::try_from(value as i64).unwrap())
} else if value as f32 as f64 == value {
Ok(Number::Float(value as f32))
} else {
Err(())
aapoalas marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

impl TryFrom<Value> for Number {
type Error = ();
fn try_from(value: Value) -> Result<Self, Self::Error> {
Expand All @@ -76,13 +101,15 @@ impl TryFrom<Value> for Number {
}

impl Number {
pub fn new(value: Value) -> Self {
debug_assert!(matches!(
value,
Value::Number(_) | Value::Integer(_) | Value::Float(_)
));
// SAFETY: Sub-enum.
unsafe { std::mem::transmute::<Value, Number>(value) }
aapoalas marked this conversation as resolved.
Show resolved Hide resolved
pub fn from_f64(agent: &mut Agent, value: f64) -> Self {
if let Ok(value) = Number::try_from(value) {
value
} else {
// SAFETY: Number was not representable as a
// stack-allocated Number.
let id = unsafe { agent.heap.alloc_number(value) };
Number::Number(id)
}
}

pub fn nan() -> Self {
Expand Down
8 changes: 2 additions & 6 deletions nova_vm/src/ecmascript/types/language/object/property_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
},
heap::{
indexes::{StringIndex, SymbolIndex},
GetHeapData,
CreateHeapData, GetHeapData,
},
Heap, SmallInteger, SmallString,
};
Expand All @@ -28,11 +28,7 @@ pub enum PropertyKey {
impl PropertyKey {
// FIXME: This API is not necessarily in the right place.
pub fn from_str(heap: &mut Heap, str: &str) -> Self {
if let Ok(ascii_string) = SmallString::try_from(str) {
PropertyKey::SmallString(ascii_string)
} else {
PropertyKey::String(heap.alloc_string(str))
}
heap.create(str).into()
}

pub fn into_value(self) -> Value {
Expand Down
8 changes: 2 additions & 6 deletions nova_vm/src/ecmascript/types/language/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ mod data;
use super::Value;
use crate::{
ecmascript::execution::Agent,
heap::{indexes::StringIndex, GetHeapData},
heap::{indexes::StringIndex, CreateHeapData, GetHeapData},
SmallString,
};

Expand Down Expand Up @@ -59,11 +59,7 @@ impl From<String> for Value {

impl String {
pub fn from_str(agent: &mut Agent, message: &str) -> String {
if let Ok(ascii_string) = SmallString::try_from(message) {
String::SmallString(ascii_string)
} else {
String::String(agent.heap.alloc_string(message))
}
agent.heap.create(message)
aapoalas marked this conversation as resolved.
Show resolved Hide resolved
}

pub fn from_small_string(message: &'static str) -> String {
Expand Down
30 changes: 5 additions & 25 deletions nova_vm/src/ecmascript/types/language/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
ArrayBufferIndex, ArrayIndex, BigIntIndex, DateIndex, ErrorIndex, FunctionIndex,
NumberIndex, ObjectIndex, RegExpIndex, StringIndex, SymbolIndex,
},
GetHeapData,
CreateHeapData, GetHeapData,
},
Heap, SmallInteger, SmallString,
};
Expand Down Expand Up @@ -130,26 +130,11 @@ pub(crate) const REGEXP_DISCRIMINANT: u8 =

impl Value {
pub fn from_str(heap: &mut Heap, message: &str) -> Value {
if let Ok(ascii_string) = SmallString::try_from(message) {
Value::SmallString(ascii_string)
} else {
Value::String(heap.alloc_string(message))
}
heap.create(message).into()
}

pub fn from_f64(heap: &mut Heap, value: f64) -> Value {
let is_int = value.fract() == 0.0;
if is_int {
if let Ok(data) = Value::try_from(value as i64) {
return data;
}
}
if value as f32 as f64 == value {
// TODO: Verify logic
Value::Float(value as f32)
} else {
Value::Number(heap.alloc_number(value))
}
pub fn from_f64(agent: &mut Agent, value: f64) -> Value {
Number::from_f64(agent, value).into()
}

pub fn nan() -> Self {
Expand Down Expand Up @@ -314,12 +299,7 @@ impl TryFrom<&str> for Value {
impl TryFrom<f64> for Value {
type Error = ();
fn try_from(value: f64) -> Result<Self, ()> {
// TODO: verify logic
if value as f32 as f64 == value {
Ok(Value::Float(value as f32))
} else {
Err(())
}
Number::try_from(value).map(|v| v.into())
}
}

Expand Down
4 changes: 2 additions & 2 deletions nova_vm/src/engine/small_integer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ impl std::fmt::Debug for SmallInteger {
}

impl SmallInteger {
pub const MIN_BIGINT: i64 = -2i64.pow(55);
pub const MIN_BIGINT: i64 = -(2i64.pow(55));
pub const MAX_BIGINT: i64 = 2i64.pow(55) - 1;

pub const MIN_NUMBER: i64 = -2i64.pow(53) + 1;
pub const MIN_NUMBER: i64 = -(2i64.pow(53)) + 1;
pub const MAX_NUMBER: i64 = 2i64.pow(53) - 1;

#[inline]
Expand Down
39 changes: 30 additions & 9 deletions nova_vm/src/heap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,16 @@ pub trait GetHeapData<'a, T, F: 'a> {

impl CreateHeapData<f64, Number> for Heap<'_, '_> {
fn create(&mut self, data: f64) -> Number {
if let Ok(value) = Value::try_from(data) {
Number::new(value)
} else if data as f32 as f64 == data {
Number::new(Value::Float(data as f32))
// NOTE: This function cannot currently be implemented
// directly using `Number::from_f64` as it takes an Agent
// parameter that we do not have access to here.
if let Ok(value) = Number::try_from(data) {
value
} else {
let id = self.alloc_number(data);
Value::Number(id).try_into().unwrap()
// SAFETY: Number was not representable as a
// stack-allocated Number.
let id = unsafe { self.alloc_number(data) };
Number::Number(id)
}
}
}
Expand Down Expand Up @@ -153,7 +156,8 @@ impl CreateHeapData<&str, String> for Heap<'_, '_> {
if let Ok(value) = String::try_from(data) {
value
} else {
let id = self.alloc_string(data);
// SAFETY: String couldn't be represented as a SmallString.
let id = unsafe { self.alloc_string(data) };
Value::String(id).try_into().unwrap()
}
}
Expand Down Expand Up @@ -250,7 +254,18 @@ impl<'ctx, 'host> Heap<'ctx, 'host> {
.expect("RealmIdentifier matched a freed Realm")
}

pub fn alloc_string(&mut self, message: &str) -> StringIndex {
/// Allocate a string onto the Agent heap
///
/// This method will currently iterate through all heap strings to look for
/// a possible matching string and if found will return its StringIndex
/// instead of allocating a copy.
///
/// SAFETY: The string being allocated must not be representable as a
/// SmallString. All SmallStrings must be kept on the stack to ensure that
/// comparison between heap allocated strings and SmallStrings can be
/// guaranteed to never equal true.
pub unsafe fn alloc_string(&mut self, message: &str) -> StringIndex {
debug_assert!(message.len() > 7 || message.ends_with('\0'));
let wtf8 = Wtf8::from_str(message);
let found = self
.strings
Expand All @@ -270,7 +285,13 @@ impl<'ctx, 'host> Heap<'ctx, 'host> {
}
}

pub fn alloc_number(&mut self, number: f64) -> NumberIndex {
/// Allocate a 64-bit floating point number onto the Agent heap
///
/// SAFETY: The number being allocated must not be representable
/// as a SmallInteger or f32. All stack-allocated numbers must be
/// inequal to any heap-allocated number.
pub unsafe fn alloc_number(&mut self, number: f64) -> NumberIndex {
debug_assert!(number.fract() != 0.0 || number as f32 as f64 != number);
self.numbers.push(Some(number.into()));
NumberIndex::last(&self.numbers)
}
Expand Down
2 changes: 1 addition & 1 deletion nova_vm/src/heap/heap_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ pub fn heap_gc(heap: &mut Heap) {
marked.store(true, Ordering::Relaxed);
let data = heap.symbols.get(index).unwrap().as_ref().unwrap();
if let Some(string_index) = data.descriptor {
queues.push_value(Value::String(string_index));
queues.push_value(string_index.into());
}
}
});
Expand Down
34 changes: 17 additions & 17 deletions nova_vm/src/heap/math.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
use super::{
heap_constants::WellKnownSymbolIndexes,
object::{ObjectEntry, PropertyDescriptor},
Heap,
CreateHeapData, Heap,
};
use crate::ecmascript::{
execution::JsResult,
types::{PropertyKey, Value},
};

pub(super) fn initialize_math_object(heap: &mut Heap) {
let e = Value::from_f64(heap, std::f64::consts::E);
let ln10 = Value::from_f64(heap, std::f64::consts::LN_10);
let ln2 = Value::from_f64(heap, std::f64::consts::LN_2);
let log10e = Value::from_f64(heap, std::f64::consts::LOG10_E);
let log2e = Value::from_f64(heap, std::f64::consts::LOG2_E);
let pi = Value::from_f64(heap, std::f64::consts::PI);
let sqrt1_2 = Value::from_f64(heap, std::f64::consts::FRAC_1_SQRT_2);
let sqrt2 = Value::from_f64(heap, std::f64::consts::SQRT_2);
let e = heap.create(std::f64::consts::E);
let ln10 = heap.create(std::f64::consts::LN_10);
let ln2 = heap.create(std::f64::consts::LN_2);
let log10e = heap.create(std::f64::consts::LOG10_E);
let log2e = heap.create(std::f64::consts::LOG2_E);
let pi = heap.create(std::f64::consts::PI);
let sqrt1_2 = heap.create(std::f64::consts::FRAC_1_SQRT_2);
let sqrt2 = heap.create(std::f64::consts::SQRT_2);
let abs = ObjectEntry::new_prototype_function_entry(heap, "abs", 1, false);
let acos = ObjectEntry::new_prototype_function_entry(heap, "acos", 1, false);
let acosh = ObjectEntry::new_prototype_function_entry(heap, "acosh", 1, false);
Expand Down Expand Up @@ -53,14 +53,14 @@ pub(super) fn initialize_math_object(heap: &mut Heap) {
let tanh = ObjectEntry::new_prototype_function_entry(heap, "tanh", 1, false);
let trunc = ObjectEntry::new_prototype_function_entry(heap, "trunc", 1, false);
let entries = vec![
ObjectEntry::new_frozen_entry(heap, "E", e),
ObjectEntry::new_frozen_entry(heap, "LN10", ln10),
ObjectEntry::new_frozen_entry(heap, "LN2", ln2),
ObjectEntry::new_frozen_entry(heap, "LOG10E", log10e),
ObjectEntry::new_frozen_entry(heap, "LOG2E", log2e),
ObjectEntry::new_frozen_entry(heap, "PI", pi),
ObjectEntry::new_frozen_entry(heap, "SQRT1_2", sqrt1_2),
ObjectEntry::new_frozen_entry(heap, "SQRT2", sqrt2),
ObjectEntry::new_frozen_entry(heap, "E", e.into()),
ObjectEntry::new_frozen_entry(heap, "LN10", ln10.into()),
ObjectEntry::new_frozen_entry(heap, "LN2", ln2.into()),
ObjectEntry::new_frozen_entry(heap, "LOG10E", log10e.into()),
ObjectEntry::new_frozen_entry(heap, "LOG2E", log2e.into()),
ObjectEntry::new_frozen_entry(heap, "PI", pi.into()),
ObjectEntry::new_frozen_entry(heap, "SQRT1_2", sqrt1_2.into()),
ObjectEntry::new_frozen_entry(heap, "SQRT2", sqrt2.into()),
ObjectEntry::new(
PropertyKey::Symbol(WellKnownSymbolIndexes::ToStringTag.into()),
PropertyDescriptor::roxh(Value::from_str(heap, "Math")),
Expand Down
15 changes: 8 additions & 7 deletions nova_vm/src/heap/number.rs
Original file line number Diff line number Diff line change
@@ -1,40 +1,41 @@
use super::{object::ObjectEntry, Heap};
use super::{object::ObjectEntry, CreateHeapData, Heap};
use crate::{
ecmascript::{
execution::JsResult,
types::{Object, PropertyKey, Value},
types::{Number, Object, PropertyKey, Value},
},
heap::{
heap_constants::{get_constructor_index, BuiltinObjectIndexes},
FunctionHeapData, PropertyDescriptor,
},
SmallInteger,
};

pub fn initialize_number_heap(heap: &mut Heap) {
let entries = vec![
ObjectEntry::new(
PropertyKey::from_str(heap, "EPSILON"),
PropertyDescriptor::roh(Value::from_f64(heap, f64::EPSILON)),
PropertyDescriptor::roh(heap.create(f64::EPSILON).into()),
),
ObjectEntry::new_prototype_function_entry(heap, "isFinite", 1, false),
ObjectEntry::new_prototype_function_entry(heap, "isInteger", 1, false),
ObjectEntry::new_prototype_function_entry(heap, "isNan", 1, false),
ObjectEntry::new_prototype_function_entry(heap, "isSafeInteger", 1, false),
ObjectEntry::new(
PropertyKey::from_str(heap, "MAX_SAFE_INTEGER"),
PropertyDescriptor::roh(Value::from_f64(heap, 9007199254740991.0)),
PropertyDescriptor::roh(Number::try_from(SmallInteger::MAX_NUMBER).unwrap().into()),
),
ObjectEntry::new(
PropertyKey::from_str(heap, "MAX_VALUE"),
PropertyDescriptor::roh(Value::from_f64(heap, f64::MAX)),
PropertyDescriptor::roh(heap.create(f64::MAX).into()),
),
ObjectEntry::new(
PropertyKey::from_str(heap, "MIN_SAFE_INTEGER"),
PropertyDescriptor::roh(Value::from_f64(heap, -9007199254740991.0)),
PropertyDescriptor::roh(Number::try_from(SmallInteger::MIN_NUMBER).unwrap().into()),
),
ObjectEntry::new(
PropertyKey::from_str(heap, "MIN_VALUE"),
PropertyDescriptor::roh(Value::from_f64(heap, f64::MIN)),
PropertyDescriptor::roh(heap.create(f64::MIN).into()),
),
ObjectEntry::new(
PropertyKey::from_str(heap, "NaN"),
Expand Down
Loading